GNOME Bugzilla – Bug 529723
Crash caused by double free in gst_bus_dispose
Last modified: 2009-01-24 19:42:16 UTC
This crash was caused by a possible double free of a message between : gst_bin_dispose and bin_replace_message Since the handler is still active and that there is no lock at the moment of the call to bin_remove_message : gst_bin_replace_message can be called at the same time and they can both acquire a pointer to the same message and try to unref it Fixed by locking the bin_remove_messages call The backtrace of the crash looked like : 9 0x037d1b12 in _int_free () from /lib/tls/libc.so.6
+ Trace 196027
Created attachment 109827 [details] [review] patch to fix the crash This patch also fixes a possible leak by calling the remove_messages after we remove the sync_handler
how did this happen? were you unreffing the bin while some elements inside it were still PLAYING? normally you have to set the bin to NULL before unreffing to make sure that nothing is interfering with the dispose.
I commited this but I still want to know how you got the error. * gst/gstbin.c: (gst_bin_class_init), (gst_bin_init), (gst_bin_dispose): Use the GLib stuff to create a private structure. Add some locking around some dispose methods to make them a little safer, see #529723. Patch by: Antoine Tremblay <hexa00 at gmail dot com>
I'm pretty sure I always set my bins to null before unreffing them.. But then again I might have a weird case somewhere when I do not... I'll keep this in mind and update the bug if I come across anything.. Sorry that I can't be more definitive at this time...
I don't really see why taking the object's lock should ever be needed in a dispose function under normal circumstances. This just seems wrong and unneeded to me, at best covering up issues in other places.
Wim, has your question in comment #3 been answered in comment #4? If so, I'd like the to be reopened again. It's been set to NEEDINFO for quite some time now. If not, I'd ask the reporter to give a more precise answer.
If there's a race condition, it shouldn't be too hard to write a test case or unit test which reproduces it. Hence NEEDINFO.
Sorry but I won't have the time do this for a while... I never saw the crash after that patch though... but I agree it would be nice to make sure how it happened ..But I'm just too busy right now...
(In reply to comment #7) > If there's a race condition, it shouldn't be too hard to write a test case or > unit test which reproduces it. Hence NEEDINFO. > If you say, that a test case must be written, I'd say to set this bug to NEW, or better: Assign it to someone, who does it. I'm closing this one as INCOMPLETE as per comment #8. Feel free to reopen, if there are any news.