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
This commit is contained in:
Haakon Sporsheim 2014-11-12 11:30:51 +01:00 committed by Sebastian Dröge
parent d600fc48f8
commit 6c079367f1
2 changed files with 76 additions and 18 deletions

View file

@ -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);

View file

@ -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;
}