gobject thread safety

[Matthias Clasen <mclasen redhat com>]

Tim, congratulations to the new job. 
Does that mean you get paid now to review gobject patches ? 
That would be awesome :-)

I'd like to make one last attempt to get some motion on the 
gobject thread safety bugs,

http://bugzilla.gnome.org/show_bug.cgi?id=65041
http://bugzilla.gnome.org/show_bug.cgi?id=64764

Can you remind me what you think is wrong with the patch
for 65041 ? It does not introduce new api and simply uses
GOnce for the get_type functions inside gobject. I have 
attached it below. Also, if you could elaborate a little
more on why you think the patch for 64764 is "a joke",
that might help to move things forward... 

Thanks,

Matthias


Index: gobject/gboxed.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gboxed.c,v
retrieving revision 1.20
diff -u -p -r1.20 gboxed.c
--- gobject/gboxed.c	16 Sep 2004 18:52:40 -0000	1.20
+++ gobject/gboxed.c	10 Mar 2005 04:08:44 -0000
@@ -24,6 +24,7 @@
 #include	"gvaluearray.h"
 #include	"gclosure.h"
 #include	"gvaluecollector.h"
+#include	"gthread.h"
 
 #include <string.h>
 
@@ -135,63 +136,98 @@ g_boxed_type_init (void) 
   g_assert (type == G_TYPE_BOXED);
 }
 
+static void
+closure_register_type (GType *type_id)
+{
+  *type_id = g_boxed_type_register_static ("GClosure",
+					   (GBoxedCopyFunc) g_closure_ref,
+					   (GBoxedFreeFunc) g_closure_unref);
+}
+
 GType
 g_closure_get_type (void)
 {
-  static GType type_id = 0;
+  static GType type_id;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) closure_register_type, &type_id);
 
-  if (!type_id)
-    type_id = g_boxed_type_register_static ("GClosure",
-					    (GBoxedCopyFunc) g_closure_ref,
-					    (GBoxedFreeFunc) g_closure_unref);
   return type_id;
 }
 
+static void
+value_register_type (GType *type_id)
+{
+  *type_id = g_boxed_type_register_static ("GValue",
+					   value_copy,
+					   value_free);
+}
+
 GType
 g_value_get_type (void)
 {
-  static GType type_id = 0;
+  static GType type_id;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) value_register_type, &type_id);
 
-  if (!type_id)
-    type_id = g_boxed_type_register_static ("GValue",
-					    value_copy,
-					    value_free);
   return type_id;
 }
 
+static void
+value_array_register_type (GType *type_id)
+{
+  *type_id = g_boxed_type_register_static ("GValueArray",
+					   (GBoxedCopyFunc) g_value_array_copy,
+					   (GBoxedFreeFunc) g_value_array_free);
+}
+
 GType
 g_value_array_get_type (void)
 {
-  static GType type_id = 0;
+  static GType type_id;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) value_array_register_type, &type_id);
 
-  if (!type_id)
-    type_id = g_boxed_type_register_static ("GValueArray",
-					    (GBoxedCopyFunc) g_value_array_copy,
-					    (GBoxedFreeFunc) g_value_array_free);
   return type_id;
 }
 
+static void
+strv_register_type (GType *type_id)
+{
+  *type_id = g_boxed_type_register_static ("GStrv",
+					   (GBoxedCopyFunc) g_strdupv,
+					   (GBoxedFreeFunc) g_strfreev);
+}
+
 GType
 g_strv_get_type (void)
 {
-  static GType type_id = 0;
+  static GType type_id;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) strv_register_type, &type_id);
 
-  if (!type_id)
-    type_id = g_boxed_type_register_static ("GStrv",
-					    (GBoxedCopyFunc) g_strdupv,
-					    (GBoxedFreeFunc) g_strfreev);
   return type_id;
 }
 
+static void
+gstring_register_type (GType *type_id)
+{
+  *type_id = g_boxed_type_register_static ("GString",	/* the naming is a bit odd, but GString is obviously not G_TYPE_STRING */
+					   gstring_copy,
+					   gstring_free);
+}
+
 GType
 g_gstring_get_type (void)
 {
-  static GType type_id = 0;
+  static GType type_id;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) gstring_register_type, &type_id);
 
-  if (!type_id)
-    type_id = g_boxed_type_register_static ("GString",	/* the naming is a bit odd, but GString is obviously not G_TYPE_STRING */
-					    gstring_copy,
-					    gstring_free);
   return type_id;
 }
 
Index: gobject/gsourceclosure.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gsourceclosure.c,v
retrieving revision 1.9
diff -u -p -r1.9 gsourceclosure.c
--- gobject/gsourceclosure.c	8 Nov 2004 18:49:54 -0000	1.9
+++ gobject/gsourceclosure.c	10 Mar 2005 04:08:44 -0000
@@ -24,43 +24,56 @@
 #include "gmarshal.h"
 #include "gvalue.h"
 #include "gvaluetypes.h"
+#include "gthread.h"
+
+static void
+io_channel_register_type (GType *type)
+{
+  *type = g_boxed_type_register_static ("GIOChannel",
+					(GBoxedCopyFunc) g_io_channel_ref,
+					(GBoxedFreeFunc) g_io_channel_unref);
+}
 
 GType
 g_io_channel_get_type (void)
 {
-  static GType our_type = 0;
+  static GType our_type;
+  static GOnce once = G_ONCE_INIT;
   
-  if (our_type == 0)
-    our_type = g_boxed_type_register_static ("GIOChannel",
-					     (GBoxedCopyFunc) g_io_channel_ref,
-					     (GBoxedFreeFunc) g_io_channel_unref);
+  g_once (&once, (GThreadFunc) io_channel_register_type, &our_type);
 
   return our_type;
 }
 
+static void
+io_condition_register_type (GType *type)
+{
+  static const GFlagsValue values[] = {
+    { G_IO_IN,   "G_IO_IN",   "in" },
+    { G_IO_OUT,  "G_IO_OUT",  "out" },
+    { G_IO_PRI,  "G_IO_PRI",  "pri" },
+    { G_IO_ERR,  "G_IO_ERR",  "err" },
+    { G_IO_HUP,  "G_IO_HUP",  "hup" },
+    { G_IO_NVAL, "G_IO_NVAL", "nval" },
+    { 0, NULL, NULL }
+  };
+  *type = g_flags_register_static ("GIOCondition", values);
+}
+
 GType
 g_io_condition_get_type (void)
 {
-  static GType etype = 0;
-  if (etype == 0)
-    {
-      static const GFlagsValue values[] = {
-	{ G_IO_IN,   "G_IO_IN",   "in" },
-	{ G_IO_OUT,  "G_IO_OUT",  "out" },
-	{ G_IO_PRI,  "G_IO_PRI",  "pri" },
-	{ G_IO_ERR,  "G_IO_ERR",  "err" },
-	{ G_IO_HUP,  "G_IO_HUP",  "hup" },
-	{ G_IO_NVAL, "G_IO_NVAL", "nval" },
-	{ 0, NULL, NULL }
-      };
-      etype = g_flags_register_static ("GIOCondition", values);
-    }
+  static GType etype;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) io_condition_register_type, &etype);
+
   return etype;
 }
 
 /* We need to hand-write this marshaler, since it doesn't have an
  * instance object.
- */
+p */
 static void
 source_closure_marshal_BOOLEAN__VOID (GClosure     *closure,
 				      GValue       *return_value,
Index: gobject/gtype.h
===================================================================
RCS file: /cvs/gnome/glib/gobject/gtype.h,v
retrieving revision 1.60
diff -u -p -r1.60 gtype.h
--- gobject/gtype.h	24 Oct 2004 01:22:29 -0000	1.60
+++ gobject/gtype.h	10 Mar 2005 04:08:44 -0000
@@ -342,7 +342,7 @@ gpointer g_type_instance_get_private    
   static const GInterfaceInfo g_implement_interface_info = { \
     (GInterfaceInitFunc) iface_init \
   }; \
-  g_type_add_interface_static (g_define_type_id, TYPE_IFACE, &g_implement_interface_info); \
+  g_type_add_interface_static (*g_define_type_id, TYPE_IFACE, &g_implement_interface_info); \
 }
 
 #define G_DEFINE_TYPE_EXTENDED(TypeName, type_name, TYPE_PARENT, flags, CODE) \
@@ -356,30 +356,35 @@ static void     type_name##_class_intern
   type_name##_class_init ((TypeName##Class*) klass); \
 } \
 \
+static void \
+type_name##_register_type (GType *g_define_type_id) \
+{ \
+  static const GTypeInfo g_define_type_info = { \
+    sizeof (TypeName##Class), \
+    (GBaseInitFunc) NULL, \
+    (GBaseFinalizeFunc) NULL, \
+    (GClassInitFunc) type_name##_class_intern_init, \
+    (GClassFinalizeFunc) NULL, \
+    NULL,   /* class_data */ \
+    sizeof (TypeName), \
+    0,      /* n_preallocs */ \
+    (GInstanceInitFunc) type_name##_init, \
+    NULL    /* value_table */ \
+  }; \
+  *g_define_type_id = g_type_register_static (TYPE_PARENT, #TypeName, &g_define_type_info, (GTypeFlags) flags); \
+  { CODE ; } \
+} \
+\
 GType \
 type_name##_get_type (void) \
 { \
-  static GType g_define_type_id = 0; \
-  if (G_UNLIKELY (g_define_type_id == 0)) \
-    { \
-      static const GTypeInfo g_define_type_info = { \
-        sizeof (TypeName##Class), \
-        (GBaseInitFunc) NULL, \
-        (GBaseFinalizeFunc) NULL, \
-        (GClassInitFunc) type_name##_class_intern_init, \
-        (GClassFinalizeFunc) NULL, \
-        NULL,   /* class_data */ \
-        sizeof (TypeName), \
-        0,      /* n_preallocs */ \
-        (GInstanceInitFunc) type_name##_init, \
-        NULL    /* value_table */ \
-      }; \
-      g_define_type_id = g_type_register_static (TYPE_PARENT, #TypeName, &g_define_type_info, (GTypeFlags) flags); \
-      { CODE ; } \
-    } \
-  return g_define_type_id; \
+  static GType type; \
+  static GOnce once = G_ONCE_INIT; \
+ \
+  g_once (&once, (GThreadFunc) type_name##_register_type, &type); \
+ \
+  return type; \
 }
-
 
 /* --- protected (for fundamental type implementations) --- */
 GTypePlugin*	 g_type_get_plugin		(GType		     type);
Index: gobject/gtypemodule.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gtypemodule.c,v
retrieving revision 1.14
diff -u -p -r1.14 gtypemodule.c
--- gobject/gtypemodule.c	16 Sep 2004 18:52:40 -0000	1.14
+++ gobject/gtypemodule.c	10 Mar 2005 04:08:44 -0000
@@ -22,6 +22,7 @@
 #include "gobjectalias.h"
 #include "gtypeplugin.h"
 #include "gtypemodule.h"
+#include "gthread.h"
 
 typedef struct _ModuleTypeInfo ModuleTypeInfo;
 typedef struct _ModuleInterfaceInfo ModuleInterfaceInfo;
@@ -99,34 +100,38 @@ g_type_module_iface_init (GTypePluginCla
   iface->complete_interface_info = g_type_module_complete_interface_info;
 }
 
-GType
-g_type_module_get_type (void)
+static void
+type_module_register_type (GType *type_module_type)
 {
-  static GType type_module_type = 0;
+  static const GTypeInfo type_module_info = {
+    sizeof (GTypeModuleClass),
+    NULL,           /* base_init */
+    NULL,           /* base_finalize */
+    (GClassInitFunc) g_type_module_class_init,
+    NULL,           /* class_finalize */
+    NULL,           /* class_data */
+    sizeof (GTypeModule),
+    0,              /* n_preallocs */
+    NULL,           /* instance_init */
+  };
+  static const GInterfaceInfo iface_info = {
+    (GInterfaceInitFunc) g_type_module_iface_init,
+    NULL,               /* interface_finalize */
+    NULL,               /* interface_data */
+  };
 
-  if (!type_module_type)
-    {
-      static const GTypeInfo type_module_info = {
-        sizeof (GTypeModuleClass),
-        NULL,           /* base_init */
-        NULL,           /* base_finalize */
-        (GClassInitFunc) g_type_module_class_init,
-        NULL,           /* class_finalize */
-        NULL,           /* class_data */
-        sizeof (GTypeModule),
-        0,              /* n_preallocs */
-        NULL,           /* instance_init */
-      };
-      static const GInterfaceInfo iface_info = {
-        (GInterfaceInitFunc) g_type_module_iface_init,
-        NULL,               /* interface_finalize */
-        NULL,               /* interface_data */
-      };
+  *type_module_type = g_type_register_static (G_TYPE_OBJECT, "GTypeModule", &type_module_info, G_TYPE_FLAG_ABSTRACT);
+
+  g_type_add_interface_static (*type_module_type, G_TYPE_TYPE_PLUGIN, &iface_info);
+}
 
-      type_module_type = g_type_register_static (G_TYPE_OBJECT, "GTypeModule", &type_module_info, G_TYPE_FLAG_ABSTRACT);
+GType
+g_type_module_get_type (void)
+{
+  static GType type_module_type;
+  static GOnce once = G_ONCE_INIT;
 
-      g_type_add_interface_static (type_module_type, G_TYPE_TYPE_PLUGIN, &iface_info);
-    }
+  g_once (&once, (GThreadFunc) type_module_register_type, &type_module_type);
   
   return type_module_type;
 }
Index: gobject/gtypeplugin.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gtypeplugin.c,v
retrieving revision 1.3
diff -u -p -r1.3 gtypeplugin.c
--- gobject/gtypeplugin.c	16 Sep 2004 18:52:40 -0000	1.3
+++ gobject/gtypeplugin.c	10 Mar 2005 04:08:44 -0000
@@ -18,25 +18,30 @@
  */
 #include	"gobjectalias.h"
 #include	"gtypeplugin.h"
+#include	"gthread.h"
 
 
 
 /* --- functions --- */
+static void
+type_plugin_register_type (GType *type_plugin_type)
+{
+  static const GTypeInfo type_plugin_info = {
+    sizeof (GTypePluginClass),
+    NULL,           /* base_init */
+    NULL,           /* base_finalize */
+  };
+      
+  *type_plugin_type = g_type_register_static (G_TYPE_INTERFACE, "GTypePlugin", &type_plugin_info, 0);
+}
+
 GType
 g_type_plugin_get_type (void)
 {
-  static GType type_plugin_type = 0;
-  
-  if (!type_plugin_type)
-    {
-      static const GTypeInfo type_plugin_info = {
-	sizeof (GTypePluginClass),
-	NULL,           /* base_init */
-	NULL,           /* base_finalize */
-      };
-      
-      type_plugin_type = g_type_register_static (G_TYPE_INTERFACE, "GTypePlugin", &type_plugin_info, 0);
-    }
+  static GType type_plugin_type;
+  static GOnce once = G_ONCE_INIT;
+
+  g_once (&once, (GThreadFunc) type_plugin_register_type, &type_plugin_type);
   
   return type_plugin_type;
 }