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 722562 - Gtk.Container.do_forall gets invalid instance
Gtk.Container.do_forall gets invalid instance
Status: RESOLVED OBSOLETE
Product: pygobject
Classification: Bindings
Component: general
3.10.x
Other Linux
: Normal major
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2014-01-19 20:14 UTC by Christoph Reiter (lazka)
Modified: 2018-01-10 20:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Ignore closure callbacks when Python is not initialized (1.16 KB, patch)
2014-08-28 07:01 UTC, Simon Feltman
committed Details | Review

Description Christoph Reiter (lazka) 2014-01-19 20:14:53 UTC
##############

from gi.repository import Gtk
import traceback

class Bug(Gtk.Container):
    def __init__(self):
        self.children = []
        Gtk.Container.__init__(self)
        self.set_has_window(False)

    def do_forall(self, *args):
        try:
            print(self)
            print(self.children)
        except:
            traceback.print_exc()

Bug().destroy()

##############

<Bug object at 0x226eeb0 (__main__+Bug at 0x2519270)>
[]
<Bug object at 0x227e730 (__main__+Bug at 0x2519270)>
Traceback (most recent call last):
  • File "a.py", line 13 in do_forall
    print(self.children)
AttributeError: 'Bug' object has no attribute 'children'

#########
Comment 1 Simon Feltman 2014-01-20 03:35:44 UTC
An workaround would be to add "children" as a class variable and set the instance variable after the call to the base class __init__.

####
class Bug(Gtk.Container):
    children = tuple()
    def __init__(self):
        Gtk.Container.__init__(self)
        self.children = []
####

The problem with setting children as an instance variable prior to calling the base class __init__ is PyGObject works by creating an empty PyObject wrapper in __new__ and creating the wrapped GObject in the base __init__. When you set instance variables it switches the GObject into "toggle ref" mode which ties the lifetime of the PyObject wrapper instance to that of the GObject (needed to keep the instance variables around). The problem is there is no GObject at the time self.children is assigned and toggle ref mode is not initiated, so the PyObject wrapper and its pre-init instance variables will be lost (and blankley re-created for the do_forall vfunc). Under normal circumstances (cases where a wrapper instance does not contain instances variables), the wrapper is created and destroyed as it is marshaled between GObject and Python.

https://git.gnome.org/browse/pygobject/tree/gi/_gobject/pygobject.c?id=3.11.4#n2215

Fixes for this are a) always use toggle refs, or  b) change __new__ to always create the underlying GObject. The (b) solution would be ideal but it would break existing overrides and people sub-classing and expecting the ability to pass construct-only properties assignments into base __init__ calls. Solution (a) might cause memory regression in cases where lots of objects are created from Python. Both of these solutions need further research though.
Comment 2 Christoph Reiter (lazka) 2014-01-20 07:51:50 UTC
Hm, I'm not sure I fully understand.

* Should assigning it after init make a difference then? I see the same behavior.
* Why is it only recreated in the last call, see the following example:

######################

from gi.repository import Gtk
import traceback

class Bug(Gtk.Container):
    children = tuple()

    def __init__(self):
        Gtk.Container.__init__(self)
        self.set_has_window(False)

    def do_forall(self, *args):
        try:
            print(self)
            print(self.children)
        except:
            traceback.print_exc()

a = Bug()
a.destroy()
a.destroy()
a.destroy()

############

<Bug object at 0x1842eb0 (__main__+Bug at 0x1aee270)>
[]
<Bug object at 0x1842eb0 (__main__+Bug at 0x1aee270)>
[]
<Bug object at 0x1842eb0 (__main__+Bug at 0x1aee270)>
[]
<Bug object at 0x7fbf7539a050 (__main__+Bug at 0x1aee270)>
()

#############

And even weirder, in the following case all values in globals() are None if accessed from do_forall; seems like do_forall gets called during interpreter shutdown?:

######

from gi.repository import Gtk
import traceback

class Bug(Gtk.Container):
    def __init__(self):
        Gtk.Container.__init__(self)
        self.children = []
        self.set_has_window(False)

    def do_forall(self, *args):
        try:
            print(self)
            print(self.children)
        except:
            traceback.print_exc()

a = Bug()
b = [a]

###########

<Bug object at 0x7fa2ff2e0050 (__main__+Bug at 0x2bc4270)>
Traceback (most recent call last):
  • File "a.py", line 15 in do_forall
    traceback.print_exc()
AttributeError: 'NoneType' object has no attribute 'print_exc'
sys.excepthook is missing
Traceback (most recent call last):
  • File "a.py", line 15 in do_forall
    traceback.print_exc()
AttributeError: 'NoneType' object has no attribute 'print_exc'
sys.excepthook is missing
lost sys.stderr
zsh: segmentation fault  python a.py
Comment 3 Simon Feltman 2014-01-20 23:37:14 UTC
(In reply to comment #2)
> Hm, I'm not sure I fully understand.
> 
> * Should assigning it after init make a difference then? I see the same
> behavior.
> * Why is it only recreated in the last call, see the following example:

My last analysis was inaccurate. It turns out setting children before or after the base class init doesn't matter because a toggle ref is setup both when the instance dictionary is created and when the wrapper is registered:
https://git.gnome.org/browse/pygobject/tree/gi/_gobject/pygobject.c?id=3.11.4#n681

The problem looks to be more along the lines of do_forall getting called after the Python wrapper (created by the script) is deleted. This can be observed a bit clearer by adding __del__ with a print which shows it is called multiple times. So PyGObject is re-creating a new wrapper (without calling the Python wrappers __init__) just for the purpose of calling do_forall during the GObject finalize. This is likely caused by the wrapper qdata being cleared prior to the toggle ref removal:
https://git.gnome.org/browse/pygobject/tree/gi/_gobject/pygobject.c?id=3.11.4#n1194


The call order looks something like the following:
-- Python wrapper is created
-- GObject is created in base class __init__ call (pygobject_init)
   -- qdata is set on the GObject holding the Python wrapper
   -- toggle ref is set if there are instance variables
-- do_forall is called at will and correctly passed the wrapper as 'self'
-- Python wrapper is deleted when refcnt reaches 0 (pygobject_clear)
   -- qdata of GObject holding Python wrapper is cleared
   -- toggle ref is removed from the GObject
      -- GObject->destroy is called triggering gtk_container_destroy
         -- do_forall vfunc is called and there is no wrapper in the qdata
            this causes a new wrapper to be created for the 'self' arg

See also:
https://git.gnome.org/browse/gtk+/tree/gtk/gtkcontainer.c?id=3.11.4#n1380

I'm not sure if there is a solution to this beyond not clearing the qdata which seems like it might cause other problems. The problematic do_forall called from GObject->destroy can be worked around using a class variable. This doesn't necessarily seem bad since anything held in the original wrappers children will be cleaned up by Python (last lines of pygobject_clear).

The segfault seems to be related to the above but with some ordering differences, as you mention the interpreter is already gone. I don't fully understand this but we could make sure Python is initialized before attempting to call into vfuncs if needed.
Comment 4 Christoph Reiter (lazka) 2014-01-21 08:52:54 UTC
Thanks for looking into it, makes sense now.

With my limited understanding:

* What about INCREF if a vfunc is added and an extra DECREF in the toggle_ref callback if it's the last and the vfuncs were added (using some flag)?
* Or more general, hold a ref to the pyobject in each closure?
Comment 5 Simon Feltman 2014-01-24 23:19:13 UTC
(In reply to comment #4)
> Thanks for looking into it, makes sense now.
> 
> With my limited understanding:
> 
> * What about INCREF if a vfunc is added and an extra DECREF in the toggle_ref
> callback if it's the last and the vfuncs were added (using some flag)?
> * Or more general, hold a ref to the pyobject in each closure?

The vfunc closures don't hold per-instance data because they are essentially unbound methods. The instance is bound when the vfunc is called by pulling the wrapper from the GObject qdata, which in the problematic do_forall no longer exists. Generally adding an extra INCREF to any wrapper instance which implements vfuncs seems like it would result in the wrapper never going away. And probably the GObject too, because pygobject_clear would never get called as long as there are Python refs to the wrapper, then we never unref/toggle the GObject. I might also be misunderstanding your proposal though?
Comment 6 Simon Feltman 2014-08-28 07:01:16 UTC
This fixes the segfault noted in the latter part of comment #2 along with
the problem described in gjs bug 729611 which pygi also suffers from.

The following fix has been pushed:
16f8f68 Ignore closure callbacks when Python is not initialized
Comment 7 Simon Feltman 2014-08-28 07:01:21 UTC
Created attachment 284661 [details] [review]
Ignore closure callbacks when Python is not initialized

Add an immediate return in ffi closures if Python is not initialized.
This fixes rare events when which lead to a segfault when a process
is exiting.
Comment 8 Jente Hidskes 2017-08-08 15:55:53 UTC
What is the status of this? I'm seeing the same behavior as reported by lazka:

Traceback (most recent call last):
  • File "/home/jente/code/src/github.com/libratbag/piper/piper/mousemap.py", line 180 in do_forall
    if callback is not None and self._children is not None:
AttributeError: 'MouseMap' object has no attribute '_children'

Code: https://github.com/libratbag/piper/blob/master/piper/mousemap.py#L180
Comment 9 GNOME Infrastructure Team 2018-01-10 20:37:03 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pygobject/issues/64.