gobject/gtype.c: Lockless type reffing/unreffing. Fixes #585375contention

This is basically done by the following:
* Move ref_count from TypeData to TypeNode
* Only access ref_count unlocked with g_atomic_int_*()

Passes all checks.

1 files changed, 163 insertions, 133 deletions

diff --git a/gobject/gtype.c b/gobject/gtype.c
index c6dff7f87..17b26fd20 100644
--- a/gobject/gtype.c
+++ b/gobject/gtype.c
@@ -185,7 +185,7 @@ static	      void			type_data_make_W		(TypeNode		*node,
 static inline void			type_data_ref_Wm		(TypeNode		*node);
 static inline void			type_data_unref_WmREC		(TypeNode		*node,
 									 gboolean		 uncached);
-static void				type_data_last_unref_Wm		(GType			 type,
+static void				type_data_last_unref_Wm		(TypeNode			 *node,
 									 gboolean		 uncached);
 static inline gpointer			type_get_qdata_L		(TypeNode		*node,
 									 GQuark			 quark);
@@ -220,24 +220,28 @@ typedef enum
 } InitState;
 
 /* --- structures --- */
-struct _TypeNode
+struct _TypeNode				/* UNLOCKED */
 {
+  gint         ref_count;			/* ATOMIC ACCESS */
   GTypePlugin *plugin;
-  guint        n_children : 12;
-  guint        n_supers : 8;
-  guint        _prot_n_ifaces_prerequisites : 9;
-  guint        is_classed : 1;
-  guint        is_instantiatable : 1;
+  guint        n_children : 12;			/* LOCKED ??? */
+  guint        n_supers : 8;			/* UNLOCKED */
+  guint        _prot_n_ifaces_prerequisites : 9;/* LOCKED ??? */
+  guint        is_classed : 1;			/* UNLOCKED */
+  guint        is_instantiatable : 1;		/* UNLOCKED */
   guint        mutatable_check_cache : 1;	/* combines some common path checks */
+						/* UNLOCKED */
   GType       *children;
-  TypeData * volatile data;
-  GQuark       qname;
+  TypeData * volatile data;			/* UNLOCKED (for existence check)
+						 * LOCKED for usage */
+  GQuark       qname;				/* UNLOCKED */
   GData       *global_gdata;
   union {
     IFaceEntry  *iface_entries;		/* for !iface types */
-    GType       *prerequisistes;
+					/* LOCKED */
+    GType       *prerequisistes;	/* LOCKED */
   } _prot;
-  GType        supers[1]; /* flexible array */
+  GType        supers[1]; /* flexible array */	/* UNLOCKED */
 };
 
 #define SIZEOF_BASE_TYPE_NODE()			(G_STRUCT_OFFSET (TypeNode, supers))
@@ -282,8 +286,7 @@ struct _IFaceEntry
 
 struct _CommonData
 {
-  guint             ref_count;
-  GTypeValueTable  *value_table;
+  GTypeValueTable  *value_table;		/* LOCKED */
 };
 
 struct _IFaceData
@@ -298,7 +301,7 @@ struct _IFaceData
   gpointer           dflt_vtable;
 };
 
-struct _ClassData
+struct _ClassData				/* UNLOCKED (for existence) */
 {
   CommonData         common;
   guint16            class_size;
@@ -308,10 +311,10 @@ struct _ClassData
   GClassInitFunc     class_init;
   GClassFinalizeFunc class_finalize;
   gconstpointer      class_data;
-  gpointer           class;
+  gpointer           class;			/* UNLOCKED */
 };
 
-struct _InstanceData
+struct _InstanceData				/* UNLOCKED */
 {
   CommonData         common;
   guint16            class_size;
@@ -321,11 +324,11 @@ struct _InstanceData
   GClassInitFunc     class_init;
   GClassFinalizeFunc class_finalize;
   gconstpointer      class_data;
-  gpointer           class;
-  guint16            instance_size;
-  guint16            private_size;
+  gpointer           class;			/* UNLOCKED */
+  guint16            instance_size;		/* UNLOCKED */
+  guint16            private_size;		/* UNLOCKED for read, LOCKED for write */
   guint16            n_preallocs;
-  GInstanceInitFunc  instance_init;
+  GInstanceInitFunc  instance_init;		/* UNLOCKED */
 };
 
 union _TypeData
@@ -1064,7 +1067,7 @@ type_data_make_W (TypeNode              *node,
     }
   
   node->data = data;
-  node->data->common.ref_count = 1;
+  g_atomic_int_set (&node->ref_count, 1);
   
   if (vtable_size)
     {
@@ -1097,69 +1100,129 @@ type_data_make_W (TypeNode              *node,
   g_assert (node->data->common.value_table != NULL); /* paranoid */
 }
 
+
+/* Only called when guaranteed that node->data doesn't exist */
 static inline void
-type_data_ref_Wm (TypeNode *node)
+certain_type_data_ref_Wm (TypeNode *node)
 {
-  if (!node->data)
-    {
-      TypeNode *pnode = lookup_type_node_I (NODE_PARENT_TYPE (node));
-      GTypeInfo tmp_info;
-      GTypeValueTable tmp_value_table;
-      
-      g_assert (node->plugin != NULL);
-      
-      if (pnode)
-	{
-	  type_data_ref_Wm (pnode);
-	  if (node->data)
-	    INVALID_RECURSION ("g_type_plugin_*", node->plugin, NODE_NAME (node));
-	}
-      
-      memset (&tmp_info, 0, sizeof (tmp_info));
-      memset (&tmp_value_table, 0, sizeof (tmp_value_table));
+  /* Really create it now */
+  TypeNode *pnode = lookup_type_node_I (NODE_PARENT_TYPE (node));
+  GTypeInfo tmp_info;
+  GTypeValueTable tmp_value_table;
+    
+  /* If we are here, it means it's a dynamic type, therefore has a plugin */
+  g_assert (node->plugin != NULL);
       
-      G_WRITE_UNLOCK (&type_rw_lock);
-      g_type_plugin_use (node->plugin);
-      g_type_plugin_complete_type_info (node->plugin, NODE_TYPE (node), &tmp_info, &tmp_value_table);
-      G_WRITE_LOCK (&type_rw_lock);
+  if (pnode)
+    {
+      type_data_ref_Wm (pnode);
       if (node->data)
 	INVALID_RECURSION ("g_type_plugin_*", node->plugin, NODE_NAME (node));
-      
-      check_type_info_I (pnode, NODE_FUNDAMENTAL_TYPE (node), NODE_NAME (node), &tmp_info);
-      type_data_make_W (node, &tmp_info,
-			check_value_table_I (NODE_NAME (node),
-					     &tmp_value_table) ? &tmp_value_table : NULL);
     }
-  else
-    {
-      g_assert (node->data->common.ref_count > 0);
       
-      node->data->common.ref_count += 1;
-    }
+  memset (&tmp_info, 0, sizeof (tmp_info));
+  memset (&tmp_value_table, 0, sizeof (tmp_value_table));
+    
+  /* Call plugin to load type */
+  G_WRITE_UNLOCK (&type_rw_lock);
+  g_type_plugin_use (node->plugin);
+  g_type_plugin_complete_type_info (node->plugin, NODE_TYPE (node), &tmp_info, &tmp_value_table);
+  G_WRITE_LOCK (&type_rw_lock);
+  if (node->data)
+    INVALID_RECURSION ("g_type_plugin_*", node->plugin, NODE_NAME (node));
+      
+  check_type_info_I (pnode, NODE_FUNDAMENTAL_TYPE (node), NODE_NAME (node), &tmp_info);
+  type_data_make_W (node, &tmp_info,
+		    check_value_table_I (NODE_NAME (node),
+					 &tmp_value_table) ? &tmp_value_table : NULL);
+
 }
 
 static inline void
-type_data_unref_WmREC (TypeNode *node,
-                       gboolean  uncached)
+type_data_ref_full (TypeNode *node, gboolean locked)
 {
-  g_assert (node->data && node->data->common.ref_count);
-  if (node->data->common.ref_count > 1)
-    node->data->common.ref_count -= 1;
-  else
+  gint old;
+
+  do
     {
-      GType node_type = NODE_TYPE (node);
-      if (!node->plugin)
+      old = g_atomic_int_get (&node->ref_count);
+
+      if (G_UNLIKELY (old == 0))
 	{
-	  g_warning ("static type `%s' unreferenced too often",
-		     NODE_NAME (node));
+	  if (!locked)
+	    G_WRITE_LOCK (&type_rw_lock);
+	  if (g_atomic_int_get (&node->ref_count) == 0)
+	    certain_type_data_ref_Wm (node);
+	  if (!locked)
+	    G_WRITE_UNLOCK (&type_rw_lock);
 	  return;
 	}
-      G_WRITE_UNLOCK (&type_rw_lock);
-      g_static_rec_mutex_lock (&class_init_rec_mutex); /* required locking order: 1) class_init_rec_mutex, 2) type_rw_lock */
-      G_WRITE_LOCK (&type_rw_lock);
-      type_data_last_unref_Wm (node_type, uncached);
-      g_static_rec_mutex_unlock (&class_init_rec_mutex);
-    }
+    } while (!g_atomic_int_compare_and_exchange (&node->ref_count, old, old + 1));
+}
+
+static inline void
+type_data_ref_Wm (TypeNode *node)
+{
+  type_data_ref_full (node, TRUE);
+}
+
+static inline void
+type_data_ref (TypeNode *node)
+{
+  type_data_ref_full (node, FALSE);
+}
+
+/* locked : TRUE if WRITE_LOCK is taken */
+static inline void
+type_data_unref_full (TypeNode *node,
+		      gboolean uncached,
+		      gboolean locked)
+{
+  gint old;
+
+  do
+    {
+      old = g_atomic_int_get (&node->ref_count);
+
+      if (G_UNLIKELY (old <= 1))
+	{
+	  if (!locked)
+	    G_WRITE_LOCK (&type_rw_lock);
+	  if (g_atomic_int_get (&node->ref_count) == 1)
+	    {
+	      if (node->plugin) {
+		G_WRITE_UNLOCK (&type_rw_lock);
+		g_static_rec_mutex_lock (&class_init_rec_mutex); /* required locking order: 1) class_init_rec_mutex, 2) type_rw_lock */
+		G_WRITE_LOCK (&type_rw_lock);
+		type_data_last_unref_Wm (node, uncached);
+		g_static_rec_mutex_unlock (&class_init_rec_mutex);
+	      }
+	    }
+	  else if (g_atomic_int_get (&node->ref_count) == 0)
+	    {
+	      G_WRITE_UNLOCK (&type_rw_lock);
+	      g_warning ("cannot drop last reference to unreferenced type `%s'",
+			 NODE_NAME (node));
+	      return;
+	    }
+	  if (!locked)
+	    G_WRITE_UNLOCK (&type_rw_lock);
+	}
+    } while (!g_atomic_int_compare_and_exchange (&node->ref_count, old, old - 1));
+}
+
+static inline void
+type_data_unref_WmREC (TypeNode *node,
+                       gboolean  uncached)
+{
+  type_data_unref_full (node, uncached, TRUE);
+}
+
+static inline void
+type_data_unref (TypeNode *node,
+		 gboolean uncached)
+{
+  type_data_unref_full (node, uncached, FALSE);
 }
 
 static void
@@ -2029,7 +2092,7 @@ type_data_finalize_class_ifaces_Wm (TypeNode *node)
 {
   guint i;
 
-  g_assert (node->is_instantiatable && node->data && node->data->class.class && node->data->common.ref_count == 0);
+  g_assert (node->is_instantiatable && node->data && node->data->class.class && node->ref_count == 0);
 
  reiterate:
   for (i = 0; i < CLASSED_NODE_N_IFACES (node); i++)
@@ -2061,7 +2124,7 @@ type_data_finalize_class_U (TypeNode  *node,
   GTypeClass *class = cdata->class;
   TypeNode *bnode;
   
-  g_assert (cdata->class && cdata->common.ref_count == 0);
+  g_assert (cdata->class && node->ref_count == 1);
   
   if (cdata->class_finalize)
     cdata->class_finalize (class, (gpointer) cdata->class_data);
@@ -2078,20 +2141,11 @@ type_data_finalize_class_U (TypeNode  *node,
 }
 
 static void
-type_data_last_unref_Wm (GType    type,
+type_data_last_unref_Wm (TypeNode  *node,
 			 gboolean uncached)
 {
-  TypeNode *node = lookup_type_node_I (type);
-  
   g_return_if_fail (node != NULL && node->plugin != NULL);
   
-  if (!node->data || node->data->common.ref_count == 0)
-    {
-      g_warning ("cannot drop last reference to unreferenced type `%s'",
-		 type_descriptive_name_I (type));
-      return;
-    }
-
   /* call class cache hooks */
   if (node->is_classed && node->data && node->data->class.class && static_n_class_cache_funcs && !uncached)
     {
@@ -2108,7 +2162,7 @@ type_data_last_unref_Wm (GType    type,
 	  G_READ_UNLOCK (&type_rw_lock);
 	  need_break = cache_func (cache_data, node->data->class.class);
 	  G_READ_LOCK (&type_rw_lock);
-	  if (!node->data || node->data->common.ref_count == 0)
+	  if (!node->data || node->ref_count == 0)
 	    INVALID_RECURSION ("GType class cache function ", cache_func, NODE_NAME (node));
 	  if (need_break)
 	    break;
@@ -2117,15 +2171,12 @@ type_data_last_unref_Wm (GType    type,
       G_WRITE_LOCK (&type_rw_lock);
     }
   
-  if (node->data->common.ref_count > 1)	/* may have been re-referenced meanwhile */
-    node->data->common.ref_count -= 1;
-  else
+  /* Check refcount again :( */
+  if (g_atomic_int_get (&node->ref_count) == 1)
     {
       GType ptype = NODE_PARENT_TYPE (node);
       TypeData *tdata;
       
-      node->data->common.ref_count = 0;
-      
       if (node->is_instantiatable)
 	{
 	  /* destroy node->data->instance.mem_chunk */
@@ -2621,28 +2672,20 @@ g_type_class_ref (GType type)
   TypeNode *node;
   GType ptype;
 
-  /* optimize for common code path */
-  G_WRITE_LOCK (&type_rw_lock);
   node = lookup_type_node_I (type);
-  if (node && node->is_classed && node->data &&
-      node->data->class.class &&
-      node->data->class.init_state == INITIALIZED)
-    {
-      type_data_ref_Wm (node);
-      G_WRITE_UNLOCK (&type_rw_lock);
-      return node->data->class.class;
-    }
-  if (!node || !node->is_classed ||
-      (node->data && node->data->common.ref_count < 1))
-    {
-      G_WRITE_UNLOCK (&type_rw_lock);
-      g_warning ("cannot retrieve class for invalid (unclassed) type `%s'",
-		 type_descriptive_name_I (type));
-      return NULL;
-    }
-  type_data_ref_Wm (node);
+
+  /* Check for as much as possible without taking lock */
+  if (G_UNLIKELY (!node || !node->is_classed)) {
+    g_warning ("cannot retrieve class for invalid (unclassed) type `%s'",
+	       type_descriptive_name_I (type));
+    return NULL;
+  }
+
+  type_data_ref (node);
+  if (G_LIKELY (node->data->class.class && node->data->class.init_state == INITIALIZED))
+    return node->data->class.class;
+
   ptype = NODE_PARENT_TYPE (node);
-  G_WRITE_UNLOCK (&type_rw_lock);
 
   g_static_rec_mutex_lock (&class_init_rec_mutex); /* required locking order: 1) class_init_rec_mutex, 2) type_rw_lock */
   /* here, we either have node->data->class.class == NULL, or a recursive
@@ -2683,14 +2726,7 @@ g_type_class_unref (gpointer g_class)
   g_return_if_fail (g_class != NULL);
   
   node = lookup_type_node_I (class->g_type);
-  G_WRITE_LOCK (&type_rw_lock);
-  if (node && node->is_classed && node->data &&
-      node->data->class.class == class && node->data->common.ref_count > 0)
-    type_data_unref_WmREC (node, FALSE);
-  else
-    g_warning ("cannot unreference class of invalid (unclassed) type `%s'",
-	       type_descriptive_name_I (class->g_type));
-  G_WRITE_UNLOCK (&type_rw_lock);
+  type_data_unref (node, TRUE);
 }
 
 /**
@@ -2709,16 +2745,9 @@ g_type_class_unref_uncached (gpointer g_class)
   GTypeClass *class = g_class;
   
   g_return_if_fail (g_class != NULL);
-  
-  G_WRITE_LOCK (&type_rw_lock);
-  node = lookup_type_node_I (class->g_type);
-  if (node && node->is_classed && node->data &&
-      node->data->class.class == class && node->data->common.ref_count > 0)
-    type_data_unref_WmREC (node, TRUE);
-  else
-    g_warning ("cannot unreference class of invalid (unclassed) type `%s'",
-	       type_descriptive_name_I (class->g_type));
-  G_WRITE_UNLOCK (&type_rw_lock);
+
+  node = lookup_type_node_I (class->g_type);  
+  type_data_unref (node, TRUE);
 }
 
 /**
@@ -2936,11 +2965,9 @@ g_type_default_interface_ref (GType g_type)
   TypeNode *node;
   gpointer dflt_vtable;
 
-  G_WRITE_LOCK (&type_rw_lock);
-
   node = lookup_type_node_I (g_type);
-  if (!node || !NODE_IS_IFACE (node) ||
-      (node->data && node->data->common.ref_count < 1))
+  G_WRITE_LOCK (&type_rw_lock);
+  if (!node || !NODE_IS_IFACE (node))
     {
       G_WRITE_UNLOCK (&type_rw_lock);
       g_warning ("cannot retrieve default vtable for invalid or non-interface type '%s'",
@@ -3022,7 +3049,7 @@ g_type_default_interface_unref (gpointer g_iface)
   G_WRITE_LOCK (&type_rw_lock);
   if (node && NODE_IS_IFACE (node) &&
       node->data->iface.dflt_vtable == g_iface &&
-      node->data->common.ref_count > 0)
+      g_atomic_int_get(&node->ref_count) > 0)
     type_data_unref_WmREC (node, FALSE);
   else
     g_warning ("cannot unreference invalid interface default vtable for '%s'",
@@ -3827,11 +3854,14 @@ type_check_is_value_type_U (GType type)
   if (node && node->mutatable_check_cache)
     return TRUE;
   
+  if (G_UNLIKELY (!node))
+    return FALSE;
+
   G_READ_LOCK (&type_rw_lock);
  restart_check:
   if (node)
     {
-      if (node->data && node->data->common.ref_count > 0 &&
+      if (node->data && g_atomic_int_get(&node->ref_count) > 0 &&
 	  node->data->common.value_table->value_init)
 	tflags = GPOINTER_TO_UINT (type_get_qdata_L (node, static_quark_type_flags));
       else if (NODE_IS_IFACE (node))
@@ -3906,7 +3936,7 @@ g_type_value_table_peek (GType type)
   G_READ_LOCK (&type_rw_lock);
   
  restart_table_peek:
-  has_refed_data = node && node->data && node->data->common.ref_count;
+  has_refed_data = node && node->data && g_atomic_int_get(&node->ref_count);
   has_table = has_refed_data && node->data->common.value_table->value_init;
   if (has_refed_data)
     {
@@ -4217,7 +4247,7 @@ g_type_instance_get_private (GTypeInstance *instance,
   if (NODE_PARENT_TYPE (private_node))
     {
       parent_node = lookup_type_node_I (NODE_PARENT_TYPE (private_node));
-      g_assert (parent_node->data && parent_node->data->common.ref_count);
+      g_assert (parent_node->data && g_atomic_int_get(&parent_node->ref_count));
 
       if (G_UNLIKELY (private_node->data->instance.private_size == parent_node->data->instance.private_size))
 	{