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 675581 - Disallow creating objects for classes without public constructors
Disallow creating objects for classes without public constructors
Status: RESOLVED OBSOLETE
Product: pygobject
Classification: Bindings
Component: gobject
unspecified
Other Linux
: Normal minor
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 647731 687792 702211 736626 743291 (view as bug list)
Depends on:
Blocks: 708060
 
 
Reported: 2012-05-07 00:58 UTC by Simon Feltman
Modified: 2018-01-10 20:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add ObjectInfo.get_abstract method (1.55 KB, patch)
2012-09-17 00:49 UTC, Simon Feltman
reviewed Details | Review
Add ObjectInfo.get_abstract method (2.35 KB, patch)
2012-09-17 10:06 UTC, Simon Feltman
committed Details | Review

Description Simon Feltman 2012-05-07 00:58:18 UTC
Playing around with the glib property bindings and this core dump came up:

>>> from gi.repository import GObject
>>> GObject.Binding()
** GLib-GObject:ERROR:gbinding.c:545:g_binding_constructed: assertion failed: (binding->source != NULL)

Aborted (core dumped)
Comment 1 Martin Pitt 2012-05-07 01:39:03 UTC
That's not pygobject's fault really; you are not supposed to create a "blank" Binding object, the API does not have a public constructor. You should use the .bind_property() methods. They don't work right now, but that's tracked in bug 675582.

Thanks!
Comment 2 Simon Feltman 2012-05-07 08:50:06 UTC
Hi Martin,

I wasn't necessarily expecting it to be a usable API. I logged this with the idea that python should never crash. Rather than crashing it should raise an exception, something along the lines of: "Binding objects are not directly creatable, use GObject.bind_property."

A couple ideas:

* Override GObject.Binding and raise the exception in its __init__.
* Remove the Binding class from being directly accessible.
* Add a general solution for introspection generated objects which do not have constructors? to raise have an __init__ which raises.
Comment 3 Martin Pitt 2012-05-07 16:10:27 UTC
The third proposal sounds best to me. It's neither scalable nor robust to try and keep a static list of objects in all libraries there which are not directly instantiable.
Comment 4 Simon Feltman 2012-05-07 23:18:28 UTC
So after some digging around I found that Binding objects are generically creatable through g_object_new and its variations (or seem to have a public constructor).  g_object_bind_property_full actually uses this internally for the creation of the object. However, there are required construction parameters. Direct usage of the class within python works if you pass it the right parameters.

from gi.repository import GObject, Gtk
a = Gtk.Adjustment(upper=100)
b = Gtk.Adjustment(upper=100)
bb = GObject.Binding(source=a, source_property='value',
                     target=b, target_property='value',
                     flags=GObject.BindingFlags.BIDIRECTIONAL)
a.props.value = 33
assert b.props.value == 33


While I still think having the exposed "bind_property" methods on GObject would be the right way to go, we can fix the current intent of this ticket by checking G_TYPE_FLAG_INSTANTIATABLE on classes before attempting creation. Basically using a "!G_TYPE_IS_INSTANTIATABLE" check in gi/_gobject/pygobject.c:pygobject_init similar to what is being done with G_TYPE_IS_ABSTRACT. But this doesn't solve the crashing Binding object with incorrect parameters.

I've logged a ticket for the glib/gobject to support "required" constructor parameters which is what should be used for the Binding objects parameters along with G_PARAM_CONSTRUCT_ONLY which is already used. If this is ever implemented, then pygobject can either react to failed creation of object or explicitly check required construction params in pygobject.c:pygobject_prepare_construct_properties. See: https://bugzilla.gnome.org/show_bug.cgi?id=675648
Comment 5 Emmanuele Bassi (:ebassi) 2012-05-08 00:45:19 UTC
what you see here is not a random crash: it's a sanity check on the state of the object post-construction. a GBinding instance has no purpose - and no sense whatsoever - if it has no source and target; and given the semantics of memory management implied by the property binding API, source and target can only be set at construction time. any other scenario is undefined, in the same sense as the C spec uses the term "undefined".

I somewhat agree that a C-level assertion is not nice, but the intent of the API ought to be clear by the fact that there is no public constructor: the only way to get a GBinding is through the g_object_bind_property() family of functions.

removing G_TYPE_IS_INSTANTIATABLE is also out of the question: internally, we do use g_object_new(), for obvious reasons, and if the is-instantiatable flag is not available, g_type_create_instance() will fail.

in general, I'd opine that all construct-only properties are also implicitly required for a constructor - to the extent that if they are not passed, GObject will use their default value. in this case it's not only required to have them, it's required that they have a non-default value, for the obvious reason that I cannot assign a default source and target instances, or source and target property names.

in any case, this all stems from the fact that you're trying to apply generic semantics to an object that is very much *not* generic. my suggestion is to follow the intent of the API, instead; document the class as to be created only by using bind_property() and friends, and that if you really want to create a GBinding through direct construction (which is heavily discouraged) you have to pass all the properties.
Comment 6 Simon Feltman 2012-05-08 01:28:18 UTC
Thanks for the info Emmanuele. The main purpose of all this was an attempt at doing little bit better of a job with the seemingly innocuous GObject.Binding class visible in the python namespace that will segfault and exit python if you don't know what your doing. And do so in a generic way if possible. I will attempt a patch which tries to be little more informative than the segfault by overriding the Binding objects __init__ and raising an exception.
Comment 7 Simon Feltman 2012-05-09 03:41:15 UTC
For clarification, does having "no public constructor" mean there is no method on the objects info which reports "is_constructor" to be true? I ran the following test and a large amount of objects show up as not having constructors in this regard:

import gi
from gi.repository import GObject, Gtk, Gdk, Gio
repo = gi.gi.Repository.get_default()
modules = dict(GObject=GObject, Gtk=Gtk, Gdk=Gdk, Gio=Gio)

print 'Objects without Constructors:'
for modname, mod in modules.viewitems():
    for name in dir(mod):
        info = repo.find_by_name(modname, name)
        if isinstance(info, (gi.types.ObjectInfo, gi.types.StructInfo)):
            if not any(meth.is_constructor() for meth in info.get_methods()):
                print '%s.%s' % (modname, name)


It doesn't seem like a generic solution using this kind of check is what we want because right away I see things I think need to be creatable in python (Gtk.MessageDialog, Gdk.Color, Gdk.Point, ...)
Comment 8 Emmanuele Bassi (:ebassi) 2012-05-09 07:42:45 UTC
(In reply to comment #7)
> For clarification, does having "no public constructor" mean there is no method
> on the objects info which reports "is_constructor" to be true?

it's usually a good indication that these classes can only be instantiated in controlled circumstances, yes.

> I ran the
> following test and a large amount of objects show up as not having constructors
> in this regard:
> 
> import gi
> from gi.repository import GObject, Gtk, Gdk, Gio
> repo = gi.gi.Repository.get_default()
> modules = dict(GObject=GObject, Gtk=Gtk, Gdk=Gdk, Gio=Gio)
> 
> print 'Objects without Constructors:'
> for modname, mod in modules.viewitems():
>     for name in dir(mod):
>         info = repo.find_by_name(modname, name)
>         if isinstance(info, (gi.types.ObjectInfo, gi.types.StructInfo)):
>             if not any(meth.is_constructor() for meth in info.get_methods()):
>                 print '%s.%s' % (modname, name)
> 
> 
> It doesn't seem like a generic solution using this kind of check is what we
> want because right away I see things I think need to be creatable in python
> (Gtk.MessageDialog, Gdk.Color, Gdk.Point, ...)

GdkColor and GdkPoint are not objects - they are boxed types, and usually used on the stack from a C API perspective; I'd apply a heuristic on constructors only on object types.

GtkMessageDialog has a constructor - though it's sadly skipped because of its use of variadic arguments; I suspect this applies to various positives in your search.
Comment 9 Martin Pitt 2012-07-24 14:15:13 UTC
*** Bug 647731 has been marked as a duplicate of this bug. ***
Comment 10 Simon Feltman 2012-09-17 00:49:31 UTC
Created attachment 224468 [details] [review]
Add ObjectInfo.get_abstract method

Adds exposure of g_object_info_get_abstract to python for
helping with analysis of non-constructable objects from
within python.
Comment 11 Martin Pitt 2012-09-17 09:28:20 UTC
Comment on attachment 224468 [details] [review]
Add ObjectInfo.get_abstract method

This looks fine, thanks! Can you please add at least a minimal test case that ensures that real constructable objects such as GIMarshallingTests or GMainLoop are not abstract?
Comment 12 Simon Feltman 2012-09-17 10:06:51 UTC
Created attachment 224487 [details] [review]
Add ObjectInfo.get_abstract method

Adds exposure of g_object_info_get_abstract to python for
helping with analysis of non-constructable objects from
within python.
Comment 13 Simon Feltman 2012-09-17 10:08:05 UTC
Latest patch adds some tests using GObject.GObject for the False case and GObject.TypeModule for the True case.
Comment 14 Martin Pitt 2012-09-17 10:17:55 UTC
Comment on attachment 224487 [details] [review]
Add ObjectInfo.get_abstract method

+        self.assertTrue(not info.get_abstract())

Instead of this, please use

    self.assertFalse(info.get_abstract())

which is less confusing. Otherwise, please push, with an "[API add]" prefix in the short description.

Thanks!
Comment 15 Martin Pitt 2012-11-07 08:01:36 UTC
*** Bug 687792 has been marked as a duplicate of this bug. ***
Comment 16 Simon Feltman 2012-11-20 09:43:17 UTC
Some initial analysis: https://live.gnome.org/PyGObject/Analysis/Bug675581
Comment 17 Simon Feltman 2012-11-20 21:41:40 UTC
Based on the findings in: https://live.gnome.org/PyGObject/Analysis/Bug675581

It does not seem feasible to use the lack of functions marked as constructors as a way to disallow generic creation of a class from python. It seems like we would be creating a hard rule based on something that is an implicit convention. Is it safe to say C constructors are just a convenience API for C? However, my understanding of this is also minimal.

Moving forward I think the classes should be explicitly marked with a flag or annotation which states: disallow generic creation
This way the knowledge can be moved closer to the source and re-used by all gi bindings.
Comment 18 Martin Pitt 2013-06-14 13:52:32 UTC
*** Bug 702211 has been marked as a duplicate of this bug. ***
Comment 19 Emmanuele Bassi (:ebassi) 2013-06-14 14:17:24 UTC
(In reply to comment #17)
> Is it safe to say C constructors are just a convenience API for C?

yes, they are considered C convenience API.

> Moving forward I think the classes should be explicitly marked with a flag or
> annotation which states: disallow generic creation
> This way the knowledge can be moved closer to the source and re-used by all gi
> bindings.

an introspection annotation would be fine, though it would require adding that information to the typelib as well as the GIR data, and I'm not entirely sure that it can be done compatibly.
Comment 20 Simon Feltman 2014-09-14 05:08:31 UTC
*** Bug 736626 has been marked as a duplicate of this bug. ***
Comment 21 Simon Feltman 2015-01-22 04:24:50 UTC
*** Bug 743291 has been marked as a duplicate of this bug. ***
Comment 22 Olav Vitters 2015-02-18 14:55:39 UTC
*** Bug 744690 has been marked as a duplicate of this bug. ***
Comment 23 GNOME Infrastructure Team 2018-01-10 20:15:51 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/26.