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 731501 - Python GC destroys PyGObject with external references
Python GC destroys PyGObject with external references
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: gobject
3.13.x
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 638267 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-06-11 06:40 UTC by Benjamin Gilbert
Modified: 2017-11-05 12:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Do not destroy object with external references (1.13 KB, patch)
2017-09-28 06:46 UTC, Cédric Bellegarde
none Details | Review
pygobject-object: Fix Python GC collecting a ref cycle too early (3.15 KB, patch)
2017-10-26 15:43 UTC, Christoph Reiter (lazka)
committed Details | Review

Description Benjamin Gilbert 2014-06-11 06:40:32 UTC
Reproducer:

===
import gc
from gi.repository import Gio, GLib, Gtk

def on_add(container, widget):
    print container
    print 'refcount', container.__grefcount__

def setup_frame(loop):
    frame = Gtk.Frame()
    frame.connect_object('add', on_add, frame)

    # This is a dummy object to avoid keeping a Python reference to the
    # frame (the only reference will be held from C).
    x = Gio.FileInfo()
    x.set_attribute_object('frame', frame)

    GLib.idle_add(lambda:
x.get_attribute_object('frame').add(Gtk.Label(label='foo')))
    GLib.idle_add(lambda: loop.quit())

loop = GLib.MainLoop()
setup_frame(loop)
gc.collect()
loop.run()
===

Result:
===
<Frame object at 0x7f329ab28cd0 (uninitialized at 0x0)>
refcount
Traceback (most recent call last):
  • File "t.py", line 6 in on_add
    print 'refcount', container.__grefcount__
TypeError: GObject instance is not yet created
===

This is exactly bug 546802, and is fixed by the patch in that bug.  The problem exists in pygobject 2.28.6 and 3.13.1. (See <https://bugzilla.gnome.org/546802#c28>.)  Please feel free to close this bug and reopen that one; I don't have access to do it myself.
Comment 1 Benjamin Gilbert 2014-06-11 18:47:09 UTC
To be clear: bug 546802 has been closed RESOLVED FIXED, but the bug has not been fixed.
Comment 2 Cédric Bellegarde 2017-09-28 06:46:22 UTC
Created attachment 360576 [details] [review]
Do not destroy object with external references

This patch backported from https://bugzilla.gnome.org/show_bug.cgi?id=546802#c13 fixes the issue.

Another report related to this issue:
https://bugs.webkit.org/show_bug.cgi?id=177526
Comment 3 Cédric Bellegarde 2017-09-28 07:49:23 UTC
Another related issue:
https://github.com/aperezdc/webkit2gtk-python-webextension-example/issues/4
Comment 4 Cédric Bellegarde 2017-09-28 08:00:17 UTC
Another related issue:
https://github.com/gnumdk/lollypop/issues/993
Comment 5 Christoph Reiter (lazka) 2017-10-26 09:55:19 UTC
Smaller reproducer:

import gc
from gi.repository import Gio

def on_add(store, *args):
    print(store)

store = Gio.ListStore()
store.connect_object('items-changed', on_add, store)

x = Gio.FileInfo()
x.set_attribute_object('store', store)
del store
gc.collect()

x.get_attribute_object('store').append(Gio.FileInfo())
Comment 6 Christoph Reiter (lazka) 2017-10-26 13:08:38 UTC
* We have a wrapper referencing the GObject, which references the closure data,
  which in turn contains the wrapper.
* When we delete the reference to the wrapper, the remaining one held by us
  is the reference from the closure data.
* Python calls tp_traverse() and sees that the wrapper is in a ref cycle with
  itself and tries to break things up using tp_clear()
* Since the closure is handled by the GObject and not the wrapper, and the
  GObject is still alive, the breaking up fails and we end up with a half
  broken instance.

The underlying problem seems to be that things traversed in tp_traverse() need
to be "owned" by the object and unrefed by tp_clear(). This is only the case
when the underlying GObject is only owned be the wrapper and thus clearing the
wrapper will unref the GObject and invalidate the closure.

So, given that, the patch looks good.
Comment 7 Christoph Reiter (lazka) 2017-10-26 15:43:38 UTC
Created attachment 362351 [details] [review]
pygobject-object: Fix Python GC collecting a ref cycle too early

PyGObject traverses its closures in tp_traverse, but the lifetime of the closures
is tied to the lifetime of the GObject and not the wrapper. This confuses
the Python GC when it sees a ref cycle and tries to break it up with tp_clear.
Since tp_clear will not invalidate the closure and only invalidate the Python
wrapper the closure callback gets called with the now cleared/invalid object.

Instead let the GC only check the Python objects referenced by the closure when tp_clear
would actually free them and as a result break the cycle. This is only the case when
the wrapped object would be freed by tp_clear which is when its reference count is at 1.

Original patch by Gustavo Carneiro:
    https://bugzilla.gnome.org/show_bug.cgi?id=546802#c5
Comment 8 Christoph Reiter (lazka) 2017-10-27 19:27:20 UTC
*** Bug 638267 has been marked as a duplicate of this bug. ***
Comment 9 Christoph Reiter (lazka) 2017-11-05 07:54:36 UTC
@Cédric: Since I see you are trying to get debian/ubuntu to backport/upgrade: If you have a reproducible case where this happens the trick mentioned in https://bugzilla.gnome.org/show_bug.cgi?id=546802#c5 by just accessing "__dict__" on the affected object should be a valid workaround.
Comment 10 Cédric Bellegarde 2017-11-05 10:39:09 UTC
It does not happen in my program but in WebKit2GTK.
Comment 11 Christoph Reiter (lazka) 2017-11-05 12:02:49 UTC
Yes, might be, but doing something like "webpage.__dict__" before calling "webpage.connect" might still work around the issue.