[glib] Make constructor-based resource registration malloc free



commit b79cfda49c59b348c9c713f5f04fd9ec982b6c9d
Author: Alexander Larsson <alexl redhat com>
Date:   Tue Jan 31 10:51:44 2012 +0100

    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 d4dc81e..0a9dfa9 100644
--- a/gio/glib-compile-resources.c
+++ b/gio/glib-compile-resources.c
@@ -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);
diff --git a/gio/gresource.c b/gio/gresource.c
index 4a93a2f..61a46e6 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
+ * <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);
+}
diff --git a/gio/gresource.h b/gio/gresource.h
index f1a8c8e..26b9a75 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__ */



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]