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 343278 - g_object_notify() not threadsafe (and also all callers)
g_object_notify() not threadsafe (and also all callers)
Status: RESOLVED INCOMPLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.x
Other Linux
: Normal major
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 166020
Blocks:
 
 
Reported: 2006-05-29 08:45 UTC by Jindrich Makovicka
Modified: 2008-05-06 12:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
race condition fix (6.72 KB, patch)
2006-05-29 08:47 UTC, Jindrich Makovicka
reviewed Details | Review

Description Jindrich Makovicka 2006-05-29 08:45:55 UTC
Several GStreamer core functions supposed to be MT safe call g_object_notify() without holding a lock on the object. This allows a race condition in g_object_notify_queue_thaw(), if another thread adds a notification while the notification queue is processed. In that case, g_object_notify_queue_thaw() tries to store too much data into a preallocated array. This results in a segfault.

The attached patch moves the g_object_notify() calls to the locked section, and modifies gst_object_dispatch_properties_changed() so it does not attempt to lock the object to avoid the deadlock.

After applying these changes, we encountered neither the segfault, nor a deadlock so far.
Comment 1 Jindrich Makovicka 2006-05-29 08:47:34 UTC
Created attachment 66405 [details] [review]
race condition fix
Comment 2 Wim Taymans 2006-05-29 08:59:54 UTC
what glib version are you using, this is supposed to be solved in glib. Also we don't want to take the object lock while emiting the notify as it can create all sorts of nasty deadlocks.
Comment 3 Jindrich Makovicka 2006-05-29 09:28:23 UTC
We use glib 2.10.2. AFAICS this is still an issue with CVS glib.

the problem is in this sequence in gobject.c:g_object_notify :
   
      g_object_notify_queue_add (object, nqueue, pspec);
      g_object_notify_queue_thaw (object, nqueue);

When one thread calls queue_add while other one executes queue_thaw, the program crashes. The freeze/add/thaw sequence would need some locking to be safe.
Comment 4 Wim Taymans 2006-07-03 14:28:50 UTC
holding the object lock while emiting the notify is not acceptable. I never saw a crash with notify, although I understand there is a theoretical problem. Can you provide a testcase so that we can verify any solution?

I remember we had a class wide lock for this stuff in the gst_object_dispatch_properties_changed() function. It would be nice to see if readding that lock would solve the issue as well. it was removed in the following patch: http://webcvs.freedesktop.org/gstreamer/gstreamer/gst/gstobject.c?r1=1.113&r2=1.114
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2007-08-05 15:02:38 UTC
Jindrich, can you try the change Wim asked in comment #4?
Comment 6 Jindrich Makovicka 2007-08-06 10:37:44 UTC
As far as I can remember, the code the patch refers to was still in the GStreamer release we used. Anyway, a few weeks after this report, we refrained from using GStreamer in this particular application due to performance and stability issues we were unable to resolve.
Comment 7 Sebastian Dröge (slomo) 2008-05-06 12:50:54 UTC
Closing this bug report as no further information has been provided. Please feel free to reopen this bug if you can provide the information asked for.
Thanks!