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 686149 - fix class initialisation
fix class initialisation
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Martin Pitt
Python bindings maintainers
Depends on:
Blocks: 687659
 
 
Reported: 2012-10-15 12:47 UTC by Allison Karlitskaya (desrt)
Modified: 2016-03-09 17:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Move property and signal creation into _class_init() (11.73 KB, patch)
2012-11-03 16:40 UTC, Martin Pitt
committed Details | Review

Description Allison Karlitskaya (desrt) 2012-10-15 12:47:23 UTC
pygobject has some unusual practices for class initialisation.  Take a look at pygobject/gobjectmodule.c in pyg_type_register().

There is a rather long comment in that function.  Here are some highlights:

     * Note: Interfaces to be implemented are searched twice.  First
     * we register interfaces that are already implemented by a parent
     * type.  The second time, the remaining interfaces are
     * registered, i.e. the ones that are not implemented by a parent
     * type.  In between these two loops, properties and signals are
     * registered.  It has to be done this way, in two steps,
     * otherwise glib will complain.

     * This looks like a GLib quirk, but no bug has been filed
     * upstream.

In practice this ends up looking like:

  type = g_type_register_static(...);
  class = g_type_class_ref (type);
  add_some_interfaces (type);
  add_some_properties (class);
  add_some_signals (type);
  add_some_more_interfaces (type);

Note in particular that interfaces are added after g_type_class_ref() is called.  g_type_class_ref() must be called to get the GObjectClass so that properties can be added which (as by the comment above) is necessarily done before adding some of the interfaces.

This complicated approach is caused by the fact that pygobject does type initialisation in a way that no other (known) user of gobject does.  Most users (effectively) do this:

  type = g_type_register_static(...);
  g_type_add_interface_static(type, ...);
  return type;

... wait for someone to call g_type_class_ref(type) ...

_class_init()
{
  g_signal_new(...);
  g_object_class_install_property(...);
  ...
}

When done in this order there are no 'quirks' to work around.


A goal in GObject is to prevent interfaces from being added to types after g_type_class_ref() has been called on the type.  Removing the possibility of the interfaces that a class implements from changing at runtime will allow us to reduce the complexity of the code for dispatching interface calls and improve the performance.

I considered making the rule "you cannot add interfaces after a class has been instantiated at least once" but this turns out to be somewhat more complicated.  A new state flag ("has ever been instantiated") would have to be added to the type info structure in order for us to check this and doing so would be slightly non-trivial.  g_type_class_ref() already changes a lot of state so it's easy to see if it's ever been called -- and it's a logical choice because it is *usually* called only as part of type instantiation or by subclasses (another point at which it is too late to add interfaces, really).
Comment 1 Martin Pitt 2012-11-03 16:36:51 UTC
Thanks for this detailled description! From that it was rather straightforward to make the necessary changes, I spent most of the time on sorting out some data type, GLib warning, and test issues.
Comment 2 Martin Pitt 2012-11-03 16:40:14 UTC
Created attachment 227969 [details] [review]
Move property and signal creation into _class_init()

This patch changes class initialization to the order you suggested, which is indeed a lot more straightforward and avoids the double interface initialization hack.

I tested this on Python 3.3.0 and 2.7.3, and the whole test suite passes with both.

I'd appreciate a second pair of eyes on that for a sanity check, as well as your confirmation that this now does what you mean.

Thanks!
Comment 3 Allison Karlitskaya (desrt) 2012-11-05 16:12:15 UTC
Applying the GLib patch in bug 687659 causes the pygobject testsuite to fail like so:

(process:22791): GLib-GObject-WARNING **: attempting to add an interface (TestInterface) to class (test_interface+MyObject) after class_init
Trace/breakpoint trap

After your patch it's working again.
Comment 4 Martin Pitt 2012-11-05 16:14:04 UTC
I confirm comment 3, I tried the same thing in jhbuild. Pushed.