GNOME Bugzilla – Bug 684062
object.get/set_property("X") ignore the introspection data
Last modified: 2013-02-22 03:33:57 UTC
It seems that it uses a completely different codepath from object.props.X .. In code term, PyGProps_getattro/pygobject_get_property/pygobject_get_properties should be unified as well as PyGProps_setattro/pygobject_set_property/pygobject_set_properties
Linked to: https://bugzilla.gnome.org/show_bug.cgi?id=675726 As the two different code paths were observed here as well (with reference counting differences).
Yes, agreed. This would be a nice cleanup.
Created attachment 224555 [details] [review] pygobject: Don't leak GObjectClass reference
Created attachment 224556 [details] [review] gobject: Got through introspection on set_property/set_properties
I couldn't figure out a good way to unify the code as they have slightly different semantics, get/set_property only refer to GObject properties while the obj.props object could also have python properties.
Comment on attachment 224555 [details] [review] pygobject: Don't leak GObjectClass reference The GObjectClass reference leak is a nice fix for the 3.4.1 stable branch as well, so I committed that. Thanks!
ping about the other patch ?
Comment on attachment 224556 [details] [review] gobject: Got through introspection on set_property/set_properties Note, when applying the patch to current trunk the test case applies to the wrong position. You need to move it to test_property_subclass_c(), I think. But even then, the patch causes a segfault: test_object_with_property_and_do_get_property_vfunc_raises (test_properties.TestInstallProperties) ... ok test_same_name_property_definitions_raises (test_properties.TestInstallProperties) ... ok test_subclass_with_decorator_gets_gproperties_dict (test_properties.TestInstallProperties) ... ok test_subclass_without_properties_is_not_modified (test_properties.TestInstallProperties) ... ok test_custom_getter (test_properties.TestProperty) ... /bin/bash: line 4: 9346 Segmentation fault (core dumped)
Created attachment 233480 [details] [review] gobject: Got through introspection on set_property/set_properties Updated the patch to the current master. The tests should now pass.
Comment on attachment 233480 [details] [review] gobject: Got through introspection on set_property/set_properties Thank you! I committed that with a slightly revised changelog.
Hi Olivier, Was there something specific this bug fixed? As it basically adds the same leak observed for "Object.props.<prop>" to both get_property and set_property. See bug 675726. I like to either back this out or add "GValue.unset" to both pygi_set_property and pygi_get_property to ensure we don't leak. But I honestly don't understand to purpose of introspection data for property access yet, so any information would helpful. There is also related info in bug 692747. Thanks!
The purpose of introspection data is if you have a property which returns a GList of GObjectBlob.. You need to annotate it, otherwise pygi can't know what type it is.
Do you have an example of this so I can make sure it doesn't break if I add unsets to the GValues?
Looking at farstream/gst/fsrtpconference/fs-rtp-session.c:FsRtpSession as an example, I think PyGObject is leaking the GList being given back from the "codecs" property. There is a lack of either a call GValue.unset or _pygi_argument_release after conversion from a GValue to a GIArgument and marshaled with _pygi_argument_to_object: http://git.gnome.org/browse/pygobject/tree/gi/pygi-property.c#n252 If a GValue.unset is used it should be conditional based on ownership transfer, so perhaps _pygi_argument_release should be used instead because it will do a better job at handling the three different transfer types. This would fix the leak I described here but we still need to fix bug 692747 for general object access to be fixed.