GNOME Bugzilla – Bug 465631
g_type_default_interface_ref() does not ensure working g_object_interface_list_properties()
Last modified: 2018-05-24 11:04:59 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.
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.
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.
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.
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.
Might be more elegant to use g_once_init_enter/leave. Tim ?
Patch looks fine to me. Ok to commit, Tim ?
*** Bug 473705 has been marked as a duplicate of this bug. ***
(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.
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
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?
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.
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.
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.
(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!
(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.
-- 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.