From 6c079367f12ad5ded5e73e9b0a20d03ac1402f29 Mon Sep 17 00:00:00 2001
From: Haakon Sporsheim <haakon.sporsheim@gmail.com>
Date: Wed, 12 Nov 2014 11:30:51 +0100
Subject: [PATCH] task: Fix pause/stop race condition

If a task thread is calling pause on it self and the
controlling/"main" thread stops the task, it could end in a race
where gst_task_func loops and then checks for paused after the
controlling thread just changed the task state to stopped.
Hence the task would actually call func again even though it was
both paused and stopped.

https://bugzilla.gnome.org/show_bug.cgi?id=740001
---
 gst/gsttask.c             | 35 ++++++++++++++--------------
 tests/check/gst/gsttask.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/gst/gsttask.c b/gst/gsttask.c
index b562f6c..60c0140 100644
--- a/gst/gsttask.c
+++ b/gst/gsttask.c
@@ -292,31 +292,30 @@ gst_task_func (GstTask * task)
   gst_task_configure_name (task);
 
   while (G_LIKELY (GET_TASK_STATE (task) != GST_TASK_STOPPED)) {
-    if (G_UNLIKELY (GET_TASK_STATE (task) == GST_TASK_PAUSED)) {
+    GST_OBJECT_LOCK (task);
+    while (G_UNLIKELY (GST_TASK_STATE (task) == GST_TASK_PAUSED)) {
+      g_rec_mutex_unlock (lock);
+
+      GST_TASK_SIGNAL (task);
+      GST_INFO_OBJECT (task, "Task going to paused");
+      GST_TASK_WAIT (task);
+      GST_INFO_OBJECT (task, "Task resume from paused");
+      GST_OBJECT_UNLOCK (task);
+      /* locking order.. */
+      g_rec_mutex_lock (lock);
       GST_OBJECT_LOCK (task);
-      while (G_UNLIKELY (GST_TASK_STATE (task) == GST_TASK_PAUSED)) {
-        g_rec_mutex_unlock (lock);
+    }
 
-        GST_TASK_SIGNAL (task);
-        GST_INFO_OBJECT (task, "Task going to paused");
-        GST_TASK_WAIT (task);
-        GST_INFO_OBJECT (task, "Task resume from paused");
-        GST_OBJECT_UNLOCK (task);
-        /* locking order.. */
-        g_rec_mutex_lock (lock);
-
-        GST_OBJECT_LOCK (task);
-        if (G_UNLIKELY (GET_TASK_STATE (task) == GST_TASK_STOPPED)) {
-          GST_OBJECT_UNLOCK (task);
-          goto done;
-        }
-      }
+    if (G_UNLIKELY (GET_TASK_STATE (task) == GST_TASK_STOPPED)) {
+      GST_OBJECT_UNLOCK (task);
+      break;
+    } else {
       GST_OBJECT_UNLOCK (task);
     }
 
     task->func (task->user_data);
   }
-done:
+
   g_rec_mutex_unlock (lock);
 
   GST_OBJECT_LOCK (task);
diff --git a/tests/check/gst/gsttask.c b/tests/check/gst/gsttask.c
index 986c403..db79558 100644
--- a/tests/check/gst/gsttask.c
+++ b/tests/check/gst/gsttask.c
@@ -26,6 +26,64 @@ static GCond task_cond;
 
 static GRecMutex task_mutex;
 
+#define TEST_RACE_ITERATIONS 1000
+
+static void
+task_signal_pause_func (void *data)
+{
+  GstTask **t = data;
+
+  g_mutex_lock (&task_lock);
+  GST_DEBUG ("signal");
+  g_cond_signal (&task_cond);
+
+  gst_task_pause (*t);
+  g_mutex_unlock (&task_lock);
+}
+
+GST_START_TEST (test_pause_stop_race)
+{
+  guint it = TEST_RACE_ITERATIONS;
+  GstTask *t;
+  gboolean ret;
+
+  t = gst_task_new (task_signal_pause_func, &t, NULL);
+  fail_if (t == NULL);
+
+  g_rec_mutex_init (&task_mutex);
+  gst_task_set_lock (t, &task_mutex);
+
+  g_cond_init (&task_cond);
+  g_mutex_init (&task_lock);
+
+  while (it-- > 0) {
+    g_mutex_lock (&task_lock);
+    GST_DEBUG ("starting");
+    ret = gst_task_start (t);
+    fail_unless (ret == TRUE);
+    /* wait for it to spin up */
+    GST_DEBUG ("waiting");
+    g_cond_wait (&task_cond, &task_lock);
+    GST_DEBUG ("done waiting");
+    g_mutex_unlock (&task_lock);
+
+    GST_DEBUG ("starting");
+    ret = gst_task_stop (t);
+    fail_unless (ret == TRUE);
+
+    GST_DEBUG ("joining");
+    ret = gst_task_join (t);
+    fail_unless (ret == TRUE);
+  }
+
+  g_cond_clear (&task_cond);
+  g_mutex_clear (&task_lock);
+
+  gst_object_unref (t);
+}
+
+GST_END_TEST;
+
 static void
 task_func2 (void *data)
 {
@@ -203,6 +261,7 @@ gst_task_suite (void)
   tcase_add_test (tc_chain, test_lock);
   tcase_add_test (tc_chain, test_lock_start);
   tcase_add_test (tc_chain, test_join);
+  tcase_add_test (tc_chain, test_pause_stop_race);
 
   return s;
 }
-- 
2.7.4