Make constructor-based resource registration malloc free
authorAlexander Larsson <alexl@redhat.com>
Tue, 31 Jan 2012 09:51:44 +0000 (10:51 +0100)
committerAlexander Larsson <alexl@redhat.com>
Tue, 31 Jan 2012 09:51:44 +0000 (10:51 +0100)
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
gio/gresource.c
gio/gresource.h

index d4dc81e04901ccc373bca054b2e2836d2b5501d4..0a9dfa9cff17421b3a036808e61604fc890bde70 100644 (file)
@@ -697,7 +697,7 @@ main (int argc, char **argv)
               "\n"
               "#include <gio/gio.h>\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);
index 4a93a2f44216370d78e127d3042ce57372c1605e..61a46e6c79658a7bdb6cb47e9e52dc49b91856da 100644 (file)
@@ -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
+ * <link linkend="glib-compile-resources">glib-compile-resources</link> 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
+ * <link linkend="glib-compile-resources">glib-compile-resources</link> 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
+ * <link linkend="glib-compile-resources">glib-compile-resources</link> 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);
+}
index f1a8c8ee22b7c3617738840744a4487fc06bb7c4..26b9a7581983bf24a6f1d5e7ba051013472a3233 100644 (file)
@@ -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__ */