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 465631 - g_type_default_interface_ref() does not ensure working g_object_interface_list_properties()
g_type_default_interface_ref() does not ensure working g_object_interface_lis...
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gobject
2.12.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 473705 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-08-11 09:22 UTC by Yeti
Modified: 2018-05-24 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test case (1.12 KB, text/plain)
2007-08-11 09:26 UTC, Yeti
  Details
test case (prop installation) (2.66 KB, text/plain)
2007-08-11 22:49 UTC, Yeti
  Details
proposed patch (2.13 KB, patch)
2007-08-16 17:53 UTC, Yeti
none Details | Review
patch: gobject: Always create pspec_pool before it is used (1.54 KB, patch)
2014-02-12 08:49 UTC, Kjell Ahlstedt
reviewed Details | Review
patch: gobject: Always create pspec_pool before it is used (1.81 KB, patch)
2014-02-12 15:26 UTC, Kjell Ahlstedt
none Details | Review

Description Yeti 2007-08-11 09:22:16 UTC
g_object_interface_list_properties() requires class initialization, this means g_type_default_interface_ref(type).

However, it is not sufficient.  If the interface is inspected before any instantiatable (probably, I'm not sure what the exact condition is) class is initialized, it breaks.
Comment 1 Yeti 2007-08-11 09:26:39 UTC
Created attachment 93478 [details]
test case

Inspects properties in GTK_TYPE_TREE_MODEL interface and GTK_TYPE_IM_CONTEXT abstract classd (neither has any).  Compile and link with Gtk+ as usual.

If the inspection is performed in order GTK_TYPE_TREE_MODEL, GTK_TYPE_IM_CONTEXT (the default), program prints:

(process:23477): GLib-GObject-CRITICAL **: g_param_spec_pool_list: assertion `pool != NULL' failed
Inspecting GtkTreeModel
0 properties:
Inspecting GtkIMContext
0 properties:

If the program is run with argument `invert', it inspect the classes in the reverse order and prints:

Inspecting GtkIMContext
0 properties:
Inspecting GtkTreeModel
0 properties:

It seems the initialization of an unrelated class caused some global initialization which is missing in the first case.
Comment 2 Yeti 2007-08-11 22:26:47 UTC
The cause is here that pspec_pool (which is simply asserted to exist) is created in g_object_do_class_init().  And g_object_do_class_init() is not called until a GObject-derived class is initialized, GInterface initialization is not enough.
Comment 3 Yeti 2007-08-11 22:49:10 UTC
Created attachment 93519 [details]
test case (prop installation)

Another test case for g_object_interface_install_property().

That is, calling g_type_default_interface_ref() on an interface type that installs properties, before GObject class is initialized, results in the same errors due to NULL pspec_pool.
Comment 4 Yeti 2007-08-16 17:53:54 UTC
Created attachment 93800 [details] [review]
proposed patch

Add a function ensure_pspec_pool() called from functions using pspec_pool that seem to make sense to call before g_object_class_init() and from g_object_class_init().

Please review.
Comment 5 Matthias Clasen 2007-12-23 04:05:20 UTC
Might be more elegant to use g_once_init_enter/leave. Tim ?
Comment 6 Matthias Clasen 2008-11-29 03:20:12 UTC
Patch looks fine to me. Ok to commit, Tim ?
Comment 7 Matthias Clasen 2008-11-29 03:23:14 UTC
*** Bug 473705 has been marked as a duplicate of this bug. ***
Comment 8 Tim Janik 2008-12-01 18:19:45 UTC
(In reply to comment #4)
> Created an attachment (id=93800) [edit]
> proposed patch
> 
> Add a function ensure_pspec_pool() called from functions using pspec_pool that
> seem to make sense to call before g_object_class_init() and from
> g_object_class_init().
> 
> Please review.

The patch can't be properly read without applying (the changed functions aren't aparent), please produce patches with diff -u -p in the future.
In general, I'm not quite fond of modifying a part of the class initialization logic and introducing additionall thread-safety logic. I'd rather see the existing infrastructure be used, i.e. force class creation/initialization for those functions that rely on it existing by adding the following where needed:
if (!some_initialization_field)
  g_type_class_unref (g_type_class_ref (G_TYPE_OBJECT));
This relies on GObjectClass being static, which is ok, since all of the initialization and structure bookkeeping in gobject.c is done that way.
Comment 9 Kjell Ahlstedt 2014-02-12 08:49:45 UTC
Created attachment 268887 [details] [review]
patch: gobject: Always create pspec_pool before it is used

Is this patch better? Even good enough?

This bug may affect programs that call functions in the file
https://git.gnome.org/browse/glibmm/tree/tools/extra_defs_gen/generate_extra_defs.cc
Comment 10 Allison Karlitskaya (desrt) 2014-02-12 12:18:22 UTC
Review of attachment 268887 [details] [review]:

::: gobject/gobject.c
@@ +731,3 @@
 
+  if (!pspec_pool)
+    g_type_class_unref (g_type_class_ref (G_TYPE_OBJECT));

Can you explain the logic of what you're trying to do here?
Comment 11 Kjell Ahlstedt 2014-02-12 15:26:51 UTC
Created attachment 268922 [details] [review]
patch: gobject: Always create pspec_pool before it is used

How about this patch then?

I have copied Tim Janik's recommended solution in comment 8. I don't know if
this is the best way to ensure that pspec_pool is always created before it's
used.
Comment 12 Kjell Ahlstedt 2014-02-18 09:26:58 UTC
Should I use G_UNLIKELY?
  if (G_UNLIKELY(!pspec_pool))
instead of
  if (!pspec_pool)

I can add that, but first I'd like to know if the comments I added to the
patch are clear enough.
Comment 13 Kjell Ahlstedt 2014-03-26 12:02:38 UTC
What can I do to have the patch in comment 11 reviewed?

This bug affects a program that is used when building glibmm and gtkmm.
There is a workaround, but I'd prefer to have the bug fixed.
Comment 14 Emmanuele Bassi (:ebassi) 2014-03-26 12:10:32 UTC
(In reply to comment #13)
> What can I do to have the patch in comment 11 reviewed?
> 
> This bug affects a program that is used when building glibmm and gtkmm.
> There is a workaround, but I'd prefer to have the bug fixed.

thanks for the patch.

the commit message is, quite frankly, not really good. you should describe what the bug is trying to fix, with some reference to the glibmm and gtkmm failures in question.

as much as GObject is not drastically changing, things were shuffled around a lot since 2008, when Tim proposed the patch.

also, I'm fairly certain that Ryan is trying to improve the situation with GParamSpecPool.

could you please explain in greater detail what is the problem that glibmm and gtkmm are experiencing?

thanks!
Comment 15 Kjell Ahlstedt 2014-03-26 15:15:01 UTC
(In reply to comment #14)
> the commit message is, quite frankly, not really good.

I misunderstood comment 10. I didn't realize that it was the commit message
that must be improved.

> also, I'm fairly certain that Ryan is trying to improve the situation with
> GParamSpecPool.

In that case, I suppose I'd better wait for Ryan's improvements. That's
probably better than trying to improve my own patch. I'm not deeply familiar
with the inner workings of GObject.
Ryan's bug 666625 describes almost the same problem as this bug.

> could you please explain in greater detail what is the problem that glibmm
> and gtkmm are experiencing?

The programs in https://git.gnome.org/browse/glibmm/tree/tools/extra_defs_gen
use g_object_class_list_properties() and g_object_interface_list_properties()
to get information about all properties of classes and interfaces.
g_object_interface_list_properties() fails if it is called before
g_object_class_list_properties(). A workaround is to ask for the properties
of at least one class before you ask for the properties of an interface.
That's always possible, but it might not be the most natural thing to do.

The programs write text files, which are then used when the source code of the
C++ bindings are generated. If you say "there are better ways of doing that",
then you're probably right. But it's how it's done now. There are such
programs in many *mm modules, like glibmm, gtkmm, atkmm, goocanvasmm, pangomm,
cluttermm, etc.
Comment 16 GNOME Infrastructure Team 2018-05-24 11:04:59 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/glib/issues/100.