From: Benjamin Otte <otte@gnome.org>
Date: Mon, 21 Apr 2003 18:09:29 +0000 (+0000)
Subject: assorted fixes:
X-Git-Tag: BRANCH-ERROR-ROOT~280
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7f5dc1299bc7454521845212401d1feb519ad696;p=platform%2Fupstream%2Fgstreamer.git

assorted fixes:

Original commit message from CVS:
assorted fixes:
- fix for #111146 (works now with lots of warnings (or maybe even without - the fun of races))
- more useful debugging output
- restructuring of when to release the lock
- emitting SHUTDOWN when holding lock so we're not destroyed while signalling
- probably something else I don't remember
---

diff --git a/gst/gstthread.c b/gst/gstthread.c
index 6e664e3..5be84df 100644
--- a/gst/gstthread.c
+++ b/gst/gstthread.c
@@ -337,9 +337,17 @@ static void
 gst_thread_catch (GstThread *thread)
 {
   gboolean wait;
-  g_mutex_lock (thread->lock);
-  if (thread != gst_thread_get_current()) {
+  if (thread == gst_thread_get_current()) {
+    /* we're trying to catch ourself */
+    if (!GST_FLAG_IS_SET (thread, GST_THREAD_MUTEX_LOCKED)) {
+      g_mutex_lock (thread->lock);
+      GST_FLAG_SET (thread, GST_THREAD_MUTEX_LOCKED);
+    }
+    GST_DEBUG (GST_CAT_THREAD, "%s is catching itself", GST_ELEMENT_NAME (thread));
+    GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING);
+  } else {
     /* another thread is trying to catch us */
+    g_mutex_lock (thread->lock);
     wait = !GST_FLAG_IS_SET (thread, GST_THREAD_STATE_SPINNING);
     while (!wait) {
       GTimeVal tv;
@@ -358,16 +366,20 @@ gst_thread_catch (GstThread *thread)
 static void
 gst_thread_release (GstThread *thread)
 {
-  g_cond_signal (thread->cond);
-  g_mutex_unlock (thread->lock);
+  if (thread != gst_thread_get_current()) {
+    g_cond_signal (thread->cond);
+    g_mutex_unlock (thread->lock);
+  }
 }
 static GstElementStateReturn
 gst_thread_change_state (GstElement *element)
 {
   GstThread *thread;
   GstElementStateReturn ret;
+  gint transition;
 
   g_return_val_if_fail (GST_IS_THREAD (element), GST_STATE_FAILURE);
+  transition = GST_STATE_TRANSITION (element);
 
   thread = GST_THREAD (element);
 
@@ -377,7 +389,14 @@ gst_thread_change_state (GstElement *element)
 
   gst_thread_catch (thread);
 
-  switch (GST_STATE_TRANSITION (element)) {
+  /* FIXME: (or GStreamers ideas about "threading"): the element variables are
+     commonly accessed by multiple threads at the same time (see bug #111146
+     for an example) */
+  if (transition != GST_STATE_TRANSITION (element)) {
+    g_warning ("inconsistent state information, fix threading please");
+  }
+
+  switch (transition) {
     case GST_STATE_NULL_TO_READY:
       /* create the thread */
       GST_FLAG_UNSET (thread, GST_THREAD_STATE_REAPING);
@@ -474,7 +493,11 @@ static void
 gst_thread_child_state_change (GstBin *bin, GstElementState oldstate, 
 			       GstElementState newstate, GstElement *element)
 {
-  parent_class->child_state_change (bin, oldstate, newstate, element);
+  GST_DEBUG (GST_CAT_THREAD, "%s (from thread %s) child %s changed state from %s to %s\n",
+              GST_ELEMENT_NAME (bin), GST_ELEMENT_NAME (gst_thread_get_current()), GST_ELEMENT_NAME (element), 
+              gst_element_state_get_name (oldstate), gst_element_state_get_name (newstate));
+  if (parent_class->child_state_change)
+    parent_class->child_state_change (bin, oldstate, newstate, element);
   /* We'll wake up the main thread now. Note that we can't lock the thread here, 
      because we might be called from inside gst_thread_change_state when holding
      the lock. But this doesn't cause any problems. */
@@ -514,7 +537,11 @@ gst_thread_main_loop (void *arg)
       while (status && GST_FLAG_IS_SET (thread, GST_THREAD_STATE_SPINNING)) {
         g_mutex_unlock (thread->lock);
         status = gst_bin_iterate (GST_BIN (thread));
-        g_mutex_lock (thread->lock);
+        if (GST_FLAG_IS_SET (thread, GST_THREAD_MUTEX_LOCKED)) {
+	  GST_FLAG_UNSET (thread, GST_THREAD_MUTEX_LOCKED);
+	} else {
+          g_mutex_lock (thread->lock);
+	}
       }
       GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING);
     }
@@ -529,6 +556,9 @@ gst_thread_main_loop (void *arg)
    * stack into the threads stack space */
   gst_scheduler_reset (GST_ELEMENT_SCHED (thread));
 
+  /* must do that before releasing the lock - we might get disposed before being done */
+  g_signal_emit (G_OBJECT (thread), gst_thread_signals[SHUTDOWN], 0);
+
   /* unlock and signal - we are out */
   g_cond_signal (thread->cond);
   g_mutex_unlock (thread->lock);
@@ -536,8 +566,6 @@ gst_thread_main_loop (void *arg)
   GST_INFO (GST_CAT_THREAD, "gstthread: thread \"%s\" is stopped",
 	    GST_ELEMENT_NAME (thread));
 
-  g_signal_emit (G_OBJECT (thread), gst_thread_signals[SHUTDOWN], 0);
-
   return NULL;
 }
 
diff --git a/gst/gstthread.h b/gst/gstthread.h
index 8758f0e..432bd4e 100644
--- a/gst/gstthread.h
+++ b/gst/gstthread.h
@@ -36,9 +36,11 @@ extern GstElementDetails gst_thread_details;
 
 
 typedef enum {
-  GST_THREAD_STATE_STARTED	= GST_BIN_FLAG_LAST,
-  GST_THREAD_STATE_SPINNING,
+  GST_THREAD_STATE_SPINNING	= GST_BIN_FLAG_LAST,
   GST_THREAD_STATE_REAPING,
+  /* when iterating with mutex locked (special cases)
+     may only be set by thread itself */
+  GST_THREAD_MUTEX_LOCKED,
 
   /* padding */
   GST_THREAD_FLAG_LAST 		= GST_BIN_FLAG_LAST + 4