GNOME Bugzilla – Bug 703662
Marshaling a list of GValues causes segfaults
Last modified: 2013-10-08 01:20:07 UTC
Since 3.8.3 the following segfaults: ############### from gi.repository import Gtk, GObject model = Gtk.ListStore(GObject.TYPE_PYOBJECT) value = GObject.Value(GObject.TYPE_PYOBJECT) value.set_boxed(object()) model.insert_with_valuesv(0, [0], [value]) value.set_boxed(object()) model.insert_with_valuesv(0, [0], [value]) ############### I guess related to https://bugzilla.gnome.org/show_bug.cgi?id=701058
Verified. Do you know if this ever worked in prior versions of PyGObject, i.e. 3.4 ?
It has worked since 3.2
Ok, correction. The above doesn't work with 3.2 and 3.4 (3.6 untested) and works with 3.8. It works in my code for older versions... no idea why.. I was using it because it reduces the insert time by 30%.
That makes more sense :) Also note that there was never a 3.6 release because PyGObject skipped from 3.4 to 3.8 to match GNOME versioning. When reading the code it looks like this never worked except in 3.8 where it working is a side effect of the commit which broke the GValue passed by reference: https://git.gnome.org/browse/pygobject/commit/?id=9e47afe459df942d9f My gut reaction is we can't safely do much for 3.8 except back out the fix for bug 701058. I can fiddle around with a hack, but this part of the code looks like a minefield. To fix this correctly we will need to re-work the cachers to contain more information (which I think is out of scope for the 3.8 branch). Specifically the problem is the marshaling cleanup for GValue grabs the incoming Python arg type from the invoke state. Meaning for arrays, the type check "py_object_type != G_TYPE_VALUE" will always succeed because it is checking the array type, not the item type, and prematurely free the GValue which is owned by Python: https://git.gnome.org/browse/pygobject/tree/gi/pygi-marshal-cleanup.c?id=3.8.0#n262 This is called by the "from_py_cleanup" variable here: https://git.gnome.org/browse/pygobject/tree/gi/pygi-marshal-from-py.c?id=3.8.0#n1011 A hack might be accomplished by doing a similar type check found in the cleanup code but in the array marshaling code, but this time really using the Python array item for the test, not the array, and conditionally call "from_py_cleanup". I hate to add more hacks here but this might just work. A better solution would be to add a new parameter to marshaling cleanup functions. This would contain the Python object of what was marshaled. So in the normal case it would just be passed the Python function arg. But in the recursive array case, it would be the item in the array. All this needs more eyes looking at it though.
Thanks for looking into this. I would say leave the 3.8 branch as is.. Does this mean, reusing GValues is generally save except in arrays and the following is valid? ############## from gi.repository import Gtk, GObject model = Gtk.ListStore(GObject.TYPE_PYOBJECT) value = GObject.Value(GObject.TYPE_PYOBJECT) value.set_boxed(object()) model.set_value(model.insert(0), 0, value) value.set_boxed(object()) model.set_value(model.insert(0), 0, value) ############
Never mind.. that's even slower than recreating new GValues.
(In reply to comment #5) > Does this mean, reusing GValues is generally save except in arrays and the > following is valid? Yes, that should be safe, the bug is only with arrays containing GValue's. As a side note, I recall GValue's holding a boxed had some issues but might be fixed now, see bug 688081#c2 Another option could be to pass an array of Python values directly to insert_with_valuesv as it will automatically marshal them into GValue's: model.insert_with_valuesv(0, [0], [object()]) This is the case where the cleanup code described in comment #4 is actually working correctly. This is also faster because the execution path is all C for creating the GValue and setting the boxed. In [4]: %timeit model.insert_with_valuesv(0, [0], [object()]) 100000 loops, best of 3: 16.8 us per loop In [3]: %timeit model.insert_with_valuesv(0, [0], [GObject.Value(GObject.TYPE_PYOBJECT, object())]) 10000 loops, best of 3: 45.5 us per loop Actually it is a bit unclear to me why set_value and like functions are overridden in Python at all. It seems like there is already code in C for auto coercing Python values for functions which take GValue arguments. Contributions to figuring this out are very welcome :)
Simply passing the python value would for example fail in case of integers: >> model = Gtk.ListStore(GObject.TYPE_PYOBJECT) >> model.insert_with_valuesv(0, [0], [2**70]) RuntimeError: Item 0: PyObject conversion to GValue failed >> model.insert_with_valuesv(0, [0], [42]) Unable to convert from gint to PyObject
I've added a patch to bug 693402 which fixes this.