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 644067 - Try not to sink objects returned by C functions.
Try not to sink objects returned by C functions.
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks: 639597 642922
 
 
Reported: 2011-03-07 01:11 UTC by Steve Frécinaux
Modified: 2011-03-07 18:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Try not to sink objects returned by C functions. (3.16 KB, patch)
2011-03-07 01:11 UTC, Steve Frécinaux
committed Details | Review

Description Steve Frécinaux 2011-03-07 01:11:32 UTC
When creating a wrapper around an object to call a python function, we
try to avoid calling pygobject_sink() when the object was not created by
pygobject or the static bindings because otherwise we can end up leaking
one reference for GInitiallyUnowned subclasses if the object was already
owned by someone else.
Comment 1 Steve Frécinaux 2011-03-07 01:11:35 UTC
Created attachment 182657 [details] [review]
Try not to sink objects returned by C functions.

When creating a wrapper around an object to call a python function, we
try to avoid calling pygobject_sink() when the object was not created by
pygobject or the static bindings because otherwise we can end up leaking
one reference for GInitiallyUnowned subclasses if the object was already
owned by someone else.
Comment 2 Steve Frécinaux 2011-03-07 01:13:54 UTC
One should test this patch with the static bindings to see if it breaks stuff.
Comment 3 johnp 2011-03-07 16:01:47 UTC
This looks too invasive for the next release which I plan to make today.  How bad is the leak?
Comment 4 Steve Frécinaux 2011-03-07 17:27:40 UTC
This patch actually solves that test from libpeas:

http://git.gnome.org/browse/libpeas/tree/tests/libpeas/extension-python.c#n62
http://git.gnome.org/browse/libpeas/tree/tests/libpeas/plugins/extension-python/extension-python.py#n33

The issue is the following one: in the test above, we are simulating the case where an object owned by C code (a GtkWindow simulacre) is wrapped into python to be set as a property for a python object.

Since we are inconditionaly calling pygobject_sink once for each wrapped GInitiallyUnowned subclass, we are actually adding one extra reference to the C objects that have to go into python, in this case the GtkWindow simulacre. The net result is that, since we have one extra reference, the object never gets freed.

Basically, without this patch, the test from line 89 fails with "3 == 2", and when the extension is unreffed, the test from line 100 also fails with "2 == 1".

It shouldn't affect any object created from python. The only affected objects are the ones which are wrapped because they are passed as argument to functions or constructors first.
Comment 5 Steve Frécinaux 2011-03-07 17:30:03 UTC
Also, the net result in bug 639597 is that all the C objects that are passed to python through libpeas leak one reference and are never freed. This affects EoG for instance which relies on the finalization of its window to terminate.
Comment 6 Steve Frécinaux 2011-03-07 18:03:05 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.