GNOME Bugzilla – Bug 601587
MiniObject race condition
Last modified: 2009-11-13 10:46:55 UTC
There's a race condition in MiniObject's gst_mini_object_free() when finalize() increments the reference count (to prevent destruction), puts the buffer in a shared pool, and before gst_mini_object_free() executes the g_atomic_int_get() some other thread may pick up the buffer from the pool and quickly unref it, which would make both of them try to call g_type_free_instance() on the same instance.
Created attachment 147494 [details] [review] Testcase that reliably reproduces the race
Created attachment 147495 [details] [review] Fix that makes the test pass
Not too happy with the test-case, it was written in a hurry. First thing that comes to mind is s/reref/recycle/g. Anyway, would be great if some of you could try running the test and verify that it reproduces the race for you, and that applying the fix does indeed make the test pass.
I should probably add that reproducing this in a real-world application is either impossible or very hard. It may only happen if the buffer alloc and dealloc happen in different threads. So for example: "videotestsrc ! xvimagesink" is safe whereas: "videotestsrc ! queue ! xvimagesink" is not The reason we discovered this is that I implemented a buffer pool in ClutterGstVideoSink and things were pretty mellow for a while -- until a queue was later introduced before the sink. I guess our use-case is an acid-test though, as there's three threads involved in our case; 1) Upstream buffer producer (H264 decoder) 2) queue's srcpad task 3) Clutter's GMainLoop We've been testing with the attached fix and so far the "mysterious" crashes that we were seeing seem to have gone away.
commit 73f2d464b7e7bc94ea0fd6159483a925743ef9c9 Author: Ole André Vadla Ravnås <ole.andre.ravnas@tandberg.com> Date: Fri Nov 13 11:42:02 2009 +0100 miniobject: avoid race when recycling buffers Avoid a race where a miniobject is recycled and quickly freed, which causes the g_type_free_instance() to be called on the same object twice. Ref the object before calling the finalize method and check if we still need to free it afterward. Also add a unit test for this case. Fixes #601587