After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740001 - task: race condition when pausing and stopping
task: race condition when pausing and stopping
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal major
: 1.4.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-12 10:39 UTC by Haakon Sporsheim (ieei)
Modified: 2014-11-19 16:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test/Example and fix (4.02 KB, patch)
2014-11-12 10:39 UTC, Haakon Sporsheim (ieei)
committed Details | Review

Description Haakon Sporsheim (ieei) 2014-11-12 10:39:04 UTC
Created attachment 290499 [details] [review]
Test/Example and fix

The GstTask thread entry point looks like this:

static void
gst_task_func (GstTask * 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);
        if (G_UNLIKELY (GET_TASK_STATE (task) == GST_TASK_STOPPED)) {
          GST_OBJECT_UNLOCK (task);
          goto done;
        }
      }
      GST_OBJECT_UNLOCK (task);
    }

    task->func (task->user_data);
  }
done:
  g_rec_mutex_unlock (lock);

  GST_OBJECT_LOCK (task);
  task->thread = NULL;
...
}

From what I can see, the while (!stopped) and if (paused) is not guarded by the object lock. A task func that look like this:
my_task_func ()
{
  ...
  gst_pad_pause_task ();
}

will once in a while cause the taks func to reenter, because another thread may change the task state to stopped between while (!stopped) and if (paused).


I've attached a patch which adds a test that must be run like this for a couple of minutes (or hours, depending on SMP/threading):
GST_CHECKS=test_pause_stop_race make -C tests/check gst/gsttask.forever

Theres also a fix in the patch. Without the fix the deadlock will normally appear after a couple of million runs, in a 1-2minutes on my system. With the fix I've been running the test for a couple of hours without deadlocking.
Comment 1 Sebastian Dröge (slomo) 2014-11-12 11:03:46 UTC
commit 6c079367f12ad5ded5e73e9b0a20d03ac1402f29
Author: Haakon Sporsheim <haakon.sporsheim@gmail.com>
Date:   Wed Nov 12 11:30:51 2014 +0100

    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