GNOME Bugzilla – Bug 657403
Crashes when instantiating custom widgets through GObject.new()
Last modified: 2011-09-26 07:56:18 UTC
Created attachment 194787 [details] reproducer When defining custom GTK widgets in Python, creating them through GtkBuilder works fine when using pygobject 2.28, but crashes when using 2.90: $ python custom-widgets.py
+ Trace 228222
As you can see, the "GData *d" pointer has an invalid value 0xaaaaaaaaaaaaaaa8. I attached a minimal reproducer for this. I bisected this to http://git.gnome.org/browse/pygobject/commit/?id=7bc4122897 , the infamous refcount fix when calling introspected constructors. As this got reverted in the 2-28 branch for maintaining ABI compatibility with static bindings, the crash does not happen there, and the test program succeeds. This still applies to current master, I just need to revert the refcount fix: (master) pygobject $ git show 7bc4122897d9d05172a2bd5b56bded87e2afaec4 | patch -Rp1 (master) pygobject $ make (master) pygobject $ PYTHONPATH=. python ~/custom-widgets.py got button from builder: <MyButton object at 0x7fd2dbc2f460 (MyButton at 0x120d360)> Traceback (most recent call last): File "/home/martin/custom-widgets.py", line 21, in <module> assert btn1.__grefcount__ == 1 AssertionError This now correctly created the MyButton object, but has a wrong refcount (it's 2) as expected. Now, this might be a ref count bug in Gtk itself, but is it possible that we mis-treat objects that were not created by pygobject itself, but by GTK internally through GtkBuilder?
Created attachment 194791 [details] simpler reproducer without GtkBuilder This an even simpler reproducer. It doesn't use GtkBuilder, just GObject.new(MyButton). This still crashes in pretty much the same way, and works when reverting above refcount patch.
Ok, I tracked this down to pygobject__g_instance_init where GObject.new passes in the newly created object with a gobject ref count of 1. We then wrap it in a python object call init on it and Py_DECREF(wrapper) and kaboom. if (wrapper == NULL) { /* this looks like a python object created through * g_object_new -> we have no python wrapper, so create it * now */ This kaboom codepath looks like it only runs when gobject_new is called so removing the decref for the wrapper should work. Why would we destroy a newly created wrapper? Attaching patch and committing because I want to make a release. This issue also breaks glade so we need it.
Created attachment 196441 [details] [review] don't destroy just created wrapper when object is created via g_object_new
Argg this is not completely correct as make check reveals that in some instances we do need to decref the wrapper. I'm not sure why some wrappers get correct number of refs and some don't
The wrapper is supposed to get an additional (python) reference in pygobject_new_full(). This is why it is unreffed in g_instance_init(). At creation time, a reference is kept by the Python GC so we should avoid keeping our own reference around, at the risk of leaking the object. The real bug here is probably that somehow the GC kicks early, kills the object and screws us in the meantime. Maybe we should keep an extra reference around during the setup of the object and unref it as late as possible: the issue with custom widgets is that the wrapper also overrides class methods so it's quite a mess at the instantiation time. I had a look at it (that's why I asked to drop the _construct() methods) but I had no time to finish my walkthrough yet :(
Created attachment 196570 [details] [review] Fix refcount bug by not creating python wrapper during gobject init stage * This only applys to python subclasses of GObject which are instantiated using GObject.new * Because we were creating the wrapper when the gobject is initialized and then again calling pygobject_new_full the wrapper would get ref'ed twice. * we could not simply Py_DECREF the wrapper due to the fact that non-subclassed objects (e.g. GObject.Object) instantiated via new do not run the same initialization code and would not have the extra ref * solution was to simply not create the wrapper during initialization because if it doesn't exist when pygobject_new_full is called it gets created and registered there
Created attachment 196572 [details] [review] Fix refcount bug by not creating python wrapper during gobject init stage * This only applys to python subclasses of GObject which are instantiated using GObject.new * Because we were creating the wrapper when the gobject is initialized and then again calling pygobject_new_full the wrapper would get ref'ed twice. * we could not simply Py_DECREF the wrapper due to the fact that non-subclassed objects (e.g. GObject.Object) instantiated via new do not run the same initialization code and would not have the extra ref * solution was to simply not create the wrapper during initialization because if it doesn't exist when pygobject_new_full is called it gets created and registered there * move the call to __init__ into pyg_object_new
Comment on attachment 196570 [details] [review] Fix refcount bug by not creating python wrapper during gobject init stage needed to move the call to tp_init so that __init__ is properly called
These patches were applied in 2.90.4, and I confirm that they fix the bug, thanks! Closing.
This was reverted in master, so reopening. It also seems to have caused some trouble with custom GTK widgets (https://launchpad.net/bugs/856669).
Argh, sorry for the noise. The original fix was reverted, but it seems that 311a4f80 actually fixed that properly now, so both test cases now work just fine. Thanks a lot!