GNOME Bugzilla – Bug 661629
[0.11] static_caps_get is racy
Last modified: 2011-10-17 12:04:01 UTC
Created attachment 198912 [details] [review] Possible fix In 0.10 and 0.11, gst_static_caps_get() contains this comment: we construct the caps on the stack, then copy over the struct into our real caps, refcount last. We do this because we must leave the refcount of the result caps to 0 so that other threads don't run away with the caps while we are constructing it. However in 0.11, the surrounding code was changed in such a way that the statement in the comment no longer applies. The new code calls gst_caps_init() on the (static) caps, which inits the miniobject refcount to 1. This creates a race (the one which is exactly documented in the comment above). The solution is to initialize the new object structure with a ref count of 0 as before. gst_caps_init() is a function local to gstcaps.c; it could be changed to set a ref count of 0 instead of 1. This is actually done by gst_mini_object_init(), so perhaps a new (private) function _gst_mini_object_init0() is needed. The code block can then close again with a call to ref the new caps from 0 to 1, which is really important because we need to keep the CPU from reordering here (which the atomic increment memory barrier will ensure). Attaching possible fix (but maybe Wim has a more elegant/different idea).
Actually this functions also leaks the pointer array (priv) because of the dual gst_caps_init. So the fix is just to get rid of the second one anyways, which fixes both problems in a much simpler way.
Straight forward fix then: commit 221836f452264e97e99ddec9cd59d9e853c7d52e caps: fix race condition and memory leak in gst_static_caps_get This was leaking the PtrArray from caps->priv, as set up by the other call to gst_caps_init. Also, the thread safety issue presented in the comment above was not taken care of anymore. We now zero the refcount again when publishing the structure.