GNOME Bugzilla – Bug 756451
Register GTypes via static constructors
Last modified: 2018-05-24 18:17:08 UTC
Its common to run into issues where types are not registered yet, but you still want to access them by name. For example, you have type names in .ui files. In order to fix this we sprinkle the code all over the place with g_type_ensure() calls, but even this is often not enough. One possible way to solve this is to use a static constructor to register the get_type() function in G_DEFINE_TYPE, so that all types are initialized early.
Created attachment 313131 [details] [review] gio: Always ensure extension points are registred before implementing them This was done in most places, but not everywhere.
Created attachment 313132 [details] [review] gtype: Register GTypes at type type system initialization A common problem in dynamic use of gtypes is that they need to have been initialized by calling the get_type function, or g_type_from_name() will fail. This happens e.g. when a .ui file references an uncommon type like GFileIcon. We fix this by using static constructors in G_DEFINE_TYPE to register a callback to the get_type function which will be called either directly if the type system is initialized, or otherwise queued for call after we've initialized the type system. This requires static constructors though, and will not support the pragma version, so pre VS 2008 is totally out.
The above patches implement this, but it relies on non-#pragma constructors. This works fine on gcc and clang, and used to work on visual studio 2008 and later, however it recently regressed in VS 2015 (see bug #752837). So, relying on this is a new requirement on the compiler/linker. Another issue with this is that the get_type calls happen much earlier than before, which may cause regressions. In general the extra code DEFINE_TYPE is quite safe (typically ADD_PRIVATE, IMPLEMENT_INTERFACE, and IMPLEMENT_EXTENSION_POINT), but it *may* contain something that initializes external dependencies earlier than before. For example, GTask initilizes the thread pool in the get_type call for unknown reasons, which causes us to always allocate an extra eventfd for the watch. This could possibly affect things via fd number changes. However, i don't see why this initialization could not be moved to class_init instead.
Review of attachment 313132 [details] [review]: ::: gobject/gtype.h @@ +1991,3 @@ + g_type_register_at_init (&type_name##_static_register_data); \ +} \ +G_DEFINE_CONSTRUCTOR(type_name##_register_at_init) This is not going to work, because gconstructor.h is not an installed header.
Can we just register the name -> function mapping? In a lot of cases all we'd need is a tiny constructor per shared library that did: g_type_mapping_register ("libostree-1.so.1", "Ostree", "ostree"); Basically having the C library say what's already required by introspection. Then GType could know g_type_from_name ("OstreeSysroot" -> dlopen("libostree-1.so.1") and ostree_sysroot_get_type () ? Maybe we wouldn't even need to make it dynamic, it's just static data we could stick in the ELF header. Just iterate over all shared libraries in the current process?
Review of attachment 313132 [details] [review]: ::: gobject/gtype.h @@ +1991,3 @@ + g_type_register_at_init (&type_name##_static_register_data); \ +} \ +G_DEFINE_CONSTRUCTOR(type_name##_register_at_init) Sure, this whole thing presumes we'll make non-pragma based of constructors a hard dependency of glib. And if we do that, gconstructor.h can be installed.
Colin: Where do we call that register_mapping format do you mean? G_DEFINE_TYPE macro expands to something that runs when you call the get_type func, which is to late. So, you still have to use constructors.
My first suggestion wasn't avoiding constructors, but it was avoiding the sudden introduction of gtype code running at constructor time. The scan-ELF-objects approach wouldn't need constructors, but it would be ELF specific.
Hi, (The following refers to a Release, optimized Visual Studio build; Debug builds should not be affected by the issue in bug 752837) (In reply to Alexander Larsson from comment #3) > The above patches implement this, but it relies on non-#pragma constructors. > This works fine on gcc and clang, and used to work on visual studio 2008 and > later, however it recently regressed in VS 2015 (see bug #752837). So, > relying on this is a new requirement on the compiler/linker. Well, actually the "regression" is due to a change in the default settings of the project files. In Visual Studio 2013 and earlier, the default link-time code generation mode was plain /LTCG, but it was changed to /LTCG:incremental in Visual Studio 2015, which would cause the constructors-using items to be optimized out by the linker, if they are not referred directly elsewhere in the main-line code flow (such as in the case of using code generated glib-compile-resources with --manual-register). The test program by itself would not pass on Windows as the GIO DLL is not loaded (as there was no reference to a GIO item, so the GIO DLL is not loaded), but by just using some random unrelated item in GIO to force the dependency on the GIO DLL, the test would pass in release builds given that: -The GIO DLL was linked with /opt:noref, rather than /opt:ref -The GIO DLL was linked with /LTCG, not /LTCG:incremental Otherwise the test fails, which means the test is still valid after we force the code to link to the GIO DLL via some other random unrelated GIO function. Hope this clears up the issue a bit-I will update the projects to explicitly (not implicitly) use /LTCG quite soon as a result. With blessings.
Colin: my initial patch instead did lazy initialization of all types when g_type_from_name or g_type_get_children was called. There was a lot of issues with this though, like unexpected reentrancy and unexpected side effects at random times. I think we could make that safe if we only init types when named in g_type_from_name, but it can't work for g_type_get_children, can it? Actually it may if we store the parent type name too.
Colin: Also, there is no need to use dlopen/dlsym. We just store the name + the get_type_func.
Independent of the whole constructor issue, the first patch is a pure bugfix. Does it look ok to push?
A couple of comments here. First: I don't want to register a bunch of extra types at ctor time. This would be a performance and memory regression. Second: if we are going to "officialise" an ABI for getting a GType from a type name then we should get around to introducing get_GObject_type() as an alternative to g_object_get_type(). Bug 746156 has some ideas.
Third: if we're going to do this then we should make it really official and introduce an actual API in libgobject for this. It seems like we could just add this to g_type_from_name() but I'm not sure we want to add a side-effect to something that used to be a straight no-side-effects lookup.
-- 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/1091.