From b79cfda49c59b348c9c713f5f04fd9ec982b6c9d Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 31 Jan 2012 10:51:44 +0100 Subject: [PATCH] Make constructor-based resource registration malloc free We need to do this because constructors run before main() and thus before any call to g_mem_set_vtable, making it impossible to use that function if constructors call g_malloc. We do this by making the constructors just register the static data for lazy registration, doing the lazy registration when using the global resource set. --- gio/glib-compile-resources.c | 80 +++++++-------- gio/gresource.c | 189 ++++++++++++++++++++++++++++++++--- gio/gresource.h | 14 +++ 3 files changed, 223 insertions(+), 60 deletions(-) diff --git a/gio/glib-compile-resources.c b/gio/glib-compile-resources.c index d4dc81e04..0a9dfa9cf 100644 --- a/gio/glib-compile-resources.c +++ b/gio/glib-compile-resources.c @@ -697,7 +697,7 @@ main (int argc, char **argv) "\n" "#include \n" "\n" - "extern GResource *%s_resource;\n", + "extern GResource *%s_get_resource (void);\n", c_name, c_name, c_name); if (manual_register) @@ -719,7 +719,6 @@ main (int argc, char **argv) guint8 *data; gsize data_size; gsize i; - char *static_str; if (!g_file_get_contents (binary_target, (char **)&data, &data_size, NULL)) @@ -760,13 +759,36 @@ main (int argc, char **argv) fprintf (file, "} };\n"); + fprintf (file, + "\n" + "static GStaticResource static_resource = { %s_resource_data.data, sizeof (%s_resource_data.data) };\n" + "extern GResource *%s_get_resource (void);\n" + "GResource *%s_get_resource (void)\n" + "{\n" + " return g_static_resource_get_resource (&static_resource);\n" + "}\n", + c_name, c_name, c_name, c_name); + + if (manual_register) { - static_str = ""; + fprintf (file, + "\n" + "extern void %s_unregister_resource (void);\n" + "void %s_unregister_resource (void)\n" + "{\n" + " g_static_resource_fini (&static_resource);\n" + "}\n" + "\n" + "extern void %s_register_resource (void);\n" + "void %s_register_resource (void)\n" + "{\n" + " g_static_resource_init (&static_resource);\n" + "}\n", + c_name, c_name, c_name, c_name); } else { - static_str = "static "; fprintf (file, "%s", gconstructor_code); fprintf (file, "\n" @@ -784,48 +806,18 @@ main (int argc, char **argv) "#else\n" "#warning \"Constructor not supported on this compiler, linking in resources will not work\"\n" "#endif\n" - "\n"); + "\n" + "static void resource_constructor (void)\n" + "{\n" + " g_static_resource_init (&static_resource);\n" + "}\n" + "\n" + "static void resource_destructor (void)\n" + "{\n" + " g_static_resource_fini (&static_resource);\n" + "}\n"); } - fprintf (file, - "\n" - "GResource *%s_resource = NULL;\n" - "\n" - "%svoid %s_unregister_resource (void)\n" - "{\n" - " if (%s_resource)\n" - " {\n" - " g_resources_unregister (%s_resource);\n" - " g_resource_unref (%s_resource);\n" - " %s_resource = NULL;\n" - " }\n" - "}\n" - "\n" - "%svoid %s_register_resource (void)\n" - "{\n" - " if (%s_resource == NULL)\n" - " {\n" - " GBytes *bytes = g_bytes_new_static (%s_resource_data.data, sizeof (%s_resource_data.data));\n" - " %s_resource = g_resource_new_from_data (bytes, NULL);\n" - " if (%s_resource)\n" - " g_resources_register (%s_resource);\n" - " g_bytes_unref (bytes);\n" - " }\n" - "}\n", c_name, static_str, c_name, c_name, c_name, c_name, c_name, static_str, c_name, c_name, c_name, c_name, c_name, c_name, c_name); - - if (!manual_register) - fprintf (file, - "\n" - "static void resource_constructor (void)\n" - "{\n" - " %s_register_resource ();\n" - "}\n" - "\n" - "static void resource_destructor (void)\n" - "{\n" - " %s_unregister_resource ();\n" - "}\n", c_name, c_name); - fclose (file); g_free (data); diff --git a/gio/gresource.c b/gio/gresource.c index 4a93a2f44..61a46e6c7 100644 --- a/gio/gresource.c +++ b/gio/gresource.c @@ -38,6 +38,8 @@ struct _GResource GvdbTable *table; }; +static void register_lazy_static_resources (); + G_DEFINE_BOXED_TYPE (GResource, g_resource, g_resource_ref, g_resource_unref) /** @@ -541,6 +543,32 @@ g_resource_enumerate_children (GResource *resource, static GRWLock resources_lock; static GList *registered_resources; +/* This is updated atomically, so we can append to it and check for NULL outside the + lock, but all other accesses are done under the write lock */ +static GStaticResource *lazy_register_resources; + +static void +g_resources_register_unlocked (GResource *resource) +{ + registered_resources = g_list_prepend (registered_resources, + g_resource_ref (resource)); +} + +static void +g_resources_unregister_unlocked (GResource *resource) +{ + if (g_list_find (registered_resources, resource) == NULL) + { + g_warning ("Tried to remove not registred resource"); + } + else + { + registered_resources = g_list_remove (registered_resources, + resource); + g_resource_unref (resource); + } +} + /** * g_resources_register: * @resource: A #GResource. @@ -555,10 +583,7 @@ void g_resources_register (GResource *resource) { g_rw_lock_writer_lock (&resources_lock); - - registered_resources = g_list_prepend (registered_resources, - g_resource_ref (resource)); - + g_resources_register_unlocked (resource); g_rw_lock_writer_unlock (&resources_lock); } @@ -574,18 +599,7 @@ void g_resources_unregister (GResource *resource) { g_rw_lock_writer_lock (&resources_lock); - - if (g_list_find (registered_resources, resource) == NULL) - { - g_warning ("Tried to remove not registred resource"); - } - else - { - registered_resources = g_list_remove (registered_resources, - resource); - g_resource_unref (resource); - } - + g_resources_unregister_unlocked (resource); g_rw_lock_writer_unlock (&resources_lock); } @@ -615,6 +629,8 @@ g_resources_open_stream (const char *path, GList *l; GInputStream *stream; + register_lazy_static_resources (); + g_rw_lock_reader_lock (&resources_lock); for (l = registered_resources; l != NULL; l = l->next) @@ -682,6 +698,8 @@ g_resources_lookup_data (const char *path, GList *l; GBytes *data; + register_lazy_static_resources (); + g_rw_lock_reader_lock (&resources_lock); for (l = registered_resources; l != NULL; l = l->next) @@ -741,6 +759,8 @@ g_resources_enumerate_children (const char *path, char **children; int i; + register_lazy_static_resources (); + g_rw_lock_reader_lock (&resources_lock); for (l = registered_resources; l != NULL; l = l->next) @@ -819,6 +839,8 @@ g_resources_get_info (const char *path, GList *l; gboolean r_res; + register_lazy_static_resources (); + g_rw_lock_reader_lock (&resources_lock); for (l = registered_resources; l != NULL; l = l->next) @@ -850,3 +872,138 @@ g_resources_get_info (const char *path, return res; } + +/* This code is to handle registration of resources very early, from a constructor. + * At that point we'd like to do minimal work, to avoid ordering issues. For instance, + * we're not allowed to use g_malloc, as the user need to be able to call g_mem_set_vtable + * before the first call to g_malloc. + * + * So, what we do at construction time is that we just register a static structure on + * a list of resources that need to be initialized, and then later, when doing any lookups + * in the global list of registered resources, or when getting a reference to the + * lazily initialized resource we lazily create and register all the GResources on + * the lazy list. + * + * To avoid having to use locks in the constructor, and having to grab the writer lock + * when checking the lazy registering list we update lazy_register_resources in + * a lock-less fashion (atomic prepend-only, atomic replace with NULL). However, all + * operations except: + * * check if there are any resources to lazily initialize + * * Add a static resource to the lazy init list + * Do use the full writer lock for protection. + */ + +static void +register_lazy_static_resources_unlocked () +{ + GStaticResource *list; + + do + list = lazy_register_resources; + while (!g_atomic_pointer_compare_and_exchange (&lazy_register_resources, list, NULL)); + + while (list != NULL) + { + GBytes *bytes = g_bytes_new_static (list->data, list->data_len); + GResource *resource = g_resource_new_from_data (bytes, NULL); + if (resource) + { + g_resources_register_unlocked (resource); + g_atomic_pointer_set (&list->resource, resource); + } + g_bytes_unref (bytes); + + list = list->next; + } +} + +static void +register_lazy_static_resources () +{ + if (g_atomic_pointer_get (&lazy_register_resources) == NULL) + return; + + g_rw_lock_writer_lock (&resources_lock); + register_lazy_static_resources_unlocked (); + g_rw_lock_writer_unlock (&resources_lock); +} + +/** + * g_static_resource_init: + * @static_resource: pointer to a static #GStaticResource. + * + * Initializes a GResource from static data using a + * GStaticResource. + * + * This is normally used by code generated by + * glib-compile-resources and is + * not typically used by other code. + * + * Since: 2.32 + **/ +void +g_static_resource_init (GStaticResource *static_resource) +{ + gpointer next; + + do + { + next = lazy_register_resources; + static_resource->next = next; + } + while (!g_atomic_pointer_compare_and_exchange (&lazy_register_resources, next, static_resource)); +} + +/** + * g_static_resource_fini: + * @static_resource: pointer to a static #GStaticResource. + * + * Finalized a GResource initialized by g_static_resource_init (). + * + * This is normally used by code generated by + * glib-compile-resources and is + * not typically used by other code. + * + * Since: 2.32 + **/ +void +g_static_resource_fini (GStaticResource *static_resource) +{ + GResource *resource; + + g_rw_lock_writer_lock (&resources_lock); + + register_lazy_static_resources_unlocked (); + + resource = g_atomic_pointer_get (&static_resource->resource); + if (resource) + { + g_atomic_pointer_set (&static_resource->resource, NULL); + g_resources_unregister_unlocked (resource); + g_resource_unref (resource); + } + + g_rw_lock_writer_unlock (&resources_lock); +} + +/** + * g_static_resource_get_resource: + * @static_resource: pointer to a static #GStaticResource. + * + * Gets the GResource that was registred by a call to g_static_resource_init (). + * + * This is normally used by code generated by + * glib-compile-resources and is + * not typically used by other code. + * + * Return value: (transfer none): a #GResource. + * + * Since: 2.32 + **/ +GResource * +g_static_resource_get_resource (GStaticResource *static_resource) +{ + register_lazy_static_resources (); + + return g_atomic_pointer_get (&static_resource->resource); +} diff --git a/gio/gresource.h b/gio/gresource.h index f1a8c8ee2..26b9a7581 100644 --- a/gio/gresource.h +++ b/gio/gresource.h @@ -49,6 +49,16 @@ G_BEGIN_DECLS #define G_RESOURCE_ERROR (g_resource_error_quark ()) GQuark g_resource_error_quark (void); +typedef struct _GStaticResource GStaticResource; + +struct _GStaticResource { + const guint8 *data; + gsize data_len; + GResource *resource; + GStaticResource *next; + gpointer padding; +}; + GType g_resource_get_type (void) G_GNUC_CONST; GResource * g_resource_new_from_data (GBytes *data, GError **error); @@ -93,6 +103,10 @@ gboolean g_resources_get_info (const char *path, GError **error); +void g_static_resource_init (GStaticResource *static_resource); +void g_static_resource_fini (GStaticResource *static_resource); +GResource *g_static_resource_get_resource (GStaticResource *static_resource); + G_END_DECLS #endif /* __G_RESOURCE_H__ */