GNOME Bugzilla – Bug 357432
[libs] GstController needs to keep controlled objects alive
Last modified: 2006-10-11 05:38:08 UTC
The pointer to the controlled object that controllers store is not backed by a reference, thus the object can be destructed without the controller being notified. It's clear that one would not want to create a controller for an object and then would drop all references to the object before uncontrolling/disposing the controller, which is why this bug has no negative side effects in C code so far. In bindings to higher level languages (with garbage collectors) however, one is not necessarily in control over the exact order in which objects as destroyed (or one does not care). Attaching a patch that turns this little python session >>> import pygst; pygst.require ("0.10"); import gst, gc >>> gst.Controller (gst.element_factory_make ("volume"), "volume") <gst.Controller object (GstController) at 0xb76855f4> >>> gc.collect () 1 >>> gc.collect () __main__:1: Warning: instance with invalid (NULL) class pointer __main__:1: Warning: g_signal_handler_disconnect: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed __main__:1: Warning: g_object_set_qdata: assertion `G_IS_OBJECT (object)' failed 1 >>> gc.collect () 0 ...into this... >>> import pygst; pygst.require ("0.10"); import gst, gc >>> gst.Controller (gst.element_factory_make ("volume"), "volume") <gst.Controller object (GstController) at 0xb75d59dc> >>> gc.collect () 0 >>> gc.collect () 1 >>> gc.collect () 1 >>> gc.collect () 0
Created attachment 73308 [details] [review] Proposed fix
Created attachment 74068 [details] [review] Updated, more complete fix. Updated patch that applies cleanly again and does not collide with recent changes. Be more elegant by just writing self->object = g_object_ref (object) to obtain the reference. Now that the controller properly references its controlled object, it becomes more important to properly free resources: Moved most cleanup from the finalize to the newly added dispose handler. Functions don't need an extra guard check to determine if dispose has been run because they all end up locking the mutex (which is still freed in finalize) and then try to find a property on the list (which is set to NULL in dispose, along with releasing the controlled object reference).
Actually this could be also fixed in another way, by making GstController gracefully disable its functionality if the controlled object is finalized (by being notified of that using a weak reference). Any thoughts on what approach is better suited here?
I prefer an additional ref, going to apply your patch.
Patch by: Ren�� Stadler <mail at renestadler dot de> * libs/gst/controller/gstcontroller.c: (gst_controller_new_valist), (gst_controller_new_list), (_gst_controller_dispose), (_gst_controller_finalize), (_gst_controller_class_init): Take ref to controlled object so that it cannot disappear. Fixes #357432.
Reopening because right below the fixed gst_controller_new_valist is gst_controller_new_list, a function with nearly identical code (still broken). Overlooked that all the time, didn't expect such a duplication of course. So how much cracked is the controller library?!
Oh damn, I got that mixed up with the another controller bug... sorry for the spam.