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 357432 - [libs] GstController needs to keep controlled objects alive
[libs] GstController needs to keep controlled objects alive
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 0.10.11
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks: 357185
 
 
Reported: 2006-09-24 10:44 UTC by René Stadler
Modified: 2006-10-11 05:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed fix (1.01 KB, patch)
2006-09-24 10:46 UTC, René Stadler
none Details | Review
Updated, more complete fix. (3.01 KB, patch)
2006-10-05 17:37 UTC, René Stadler
committed Details | Review

Description René Stadler 2006-09-24 10:44:56 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
Comment 1 René Stadler 2006-09-24 10:46:21 UTC
Created attachment 73308 [details] [review]
Proposed fix
Comment 2 René Stadler 2006-10-05 17:37:22 UTC
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).
Comment 3 René Stadler 2006-10-06 17:21:20 UTC
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?
Comment 4 Wim Taymans 2006-10-10 13:40:37 UTC
I prefer an additional ref, going to apply your patch.
Comment 5 Wim Taymans 2006-10-10 14:13:02 UTC
        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.
Comment 6 René Stadler 2006-10-11 05:35:38 UTC
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?!
Comment 7 René Stadler 2006-10-11 05:38:08 UTC
Oh damn, I got that mixed up with the another controller bug... sorry for the spam.