gobject: fix race and use g_datalist_id_update_atomic() for weak refs

Note that we have two calls

  g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);

that don't take OPTIONAL_BIT_LOCK_WEAK_REFS lock. One is right before
finalize(). At that point, no other thread can hold a reference to the
object to race. One call is from g_object_real_dispose(). At this point,
theoretically the object could have been resurrected and a pointer
passed to another thread, so it can race against g_object_weak_ref()
and g_object_weak_unref().

Fix that, by performing all operations on the WeakRefStack while holding
the GData lock. By using g_datalist_id_update_atomic(), we no longer
need the OPTIONAL_BIT_LOCK_WEAK_REFS lock. On the other hand, we might
now do slightly more work while holding the GData lock.

Also, during g_object_weak_unref() free the WeakRefStack, if there
are no more references.

gobject/gobject.c

@@ -124,8 +124,7 @@ enum {
  * parallel as possible. The alternative would be to add individual locking
  * integers to GObjectPrivate. But increasing memory usage for more parallelism
  * (per-object!) is not worth it. */
-#define OPTIONAL_BIT_LOCK_WEAK_REFS      1
+#define OPTIONAL_BIT_LOCK_TOGGLE_REFS 1
-#define OPTIONAL_BIT_LOCK_TOGGLE_REFS    2
 
 #if SIZEOF_INT == 4 && GLIB_SIZEOF_VOID_P >= 8
 #define HAVE_OPTIONAL_FLAGS_IN_GOBJECT 1
@@ -3714,6 +3713,40 @@ weak_refs_notify (gpointer data)
   g_free (wstack);
 }
 
+static gpointer
+g_object_weak_ref_cb (gpointer *data,
+                      GDestroyNotify *destroy_notify,
+                      gpointer user_data)
+{
+  GObject *object  = ((gpointer *) user_data)[0];
+  GWeakNotify notify = ((gpointer *) user_data)[1];
+  gpointer notify_data = ((gpointer *) user_data)[2];
+  WeakRefStack *wstack = *data;
+  guint i;
+
+  if (!wstack)
+    {
+      wstack = g_new (WeakRefStack, 1);
+      wstack->object = object;
+      wstack->n_weak_refs = 1;
+      i = 0;
+
+      *destroy_notify = weak_refs_notify;
+    }
+  else
+    {
+      i = wstack->n_weak_refs++;
+      wstack = g_realloc (wstack, sizeof (*wstack) + sizeof (wstack->weak_refs[0]) * i);
+    }
+
+  *data = wstack;
+
+  wstack->weak_refs[i].notify = notify;
+  wstack->weak_refs[i].data = notify_data;
+
+  return NULL;
+}
+
 /**
  * g_object_weak_ref: (skip)
  * @object: #GObject to reference weakly
@@ -3732,35 +3765,58 @@ weak_refs_notify (gpointer data)
  * Use #GWeakRef if thread-safety is required.
  */
 void
-g_object_weak_ref (GObject    *object,
+g_object_weak_ref (GObject *object,
-		   GWeakNotify notify,
+                   GWeakNotify notify,
-		   gpointer    data)
+                   gpointer data)
 {
-  WeakRefStack *wstack;
-  guint i;
-  
   g_return_if_fail (G_IS_OBJECT (object));
   g_return_if_fail (notify != NULL);
   g_return_if_fail (g_atomic_int_get (&object->ref_count) >= 1);
 
-  object_bit_lock (object, OPTIONAL_BIT_LOCK_WEAK_REFS);
+  _g_datalist_id_update_atomic (&object->qdata,
-  wstack = g_datalist_id_remove_no_notify (&object->qdata, quark_weak_notifies);
+                                quark_weak_notifies,
+                                g_object_weak_ref_cb,
+                                ((gpointer[]){ object, notify, data }));
+}
+
+static gpointer
+g_object_weak_unref_cb (gpointer *data,
+                        GDestroyNotify *destroy_notify,
+                        gpointer user_data)
+{
+  GWeakNotify notify = ((gpointer *) user_data)[0];
+  gpointer notify_data = ((gpointer *) user_data)[1];
+  WeakRefStack *wstack = *data;
+  gboolean found_one = FALSE;
+  guint i;
+
   if (wstack)
     {
-      i = wstack->n_weak_refs++;
+      for (i = 0; i < wstack->n_weak_refs; i++)
-      wstack = g_realloc (wstack, sizeof (*wstack) + sizeof (wstack->weak_refs[0]) * i);
+        {
+          if (wstack->weak_refs[i].notify != notify ||
+              wstack->weak_refs[i].data != notify_data)
+            continue;
+
+
+          wstack->n_weak_refs -= 1;
+          if (wstack->n_weak_refs == 0)
+            {
+              g_free (wstack);
+              *data = NULL;
+            }
+          else if (i != wstack->n_weak_refs)
+            wstack->weak_refs[i] = wstack->weak_refs[wstack->n_weak_refs];
+
+          found_one = TRUE;
+          break;
+        }
     }
-  else
+
-    {
+  if (!found_one)
-      wstack = g_renew (WeakRefStack, NULL, 1);
+    g_critical ("%s: couldn't find weak ref %p(%p)", G_STRFUNC, notify, notify_data);
-      wstack->object = object;
+
-      wstack->n_weak_refs = 1;
+  return NULL;
-      i = 0;
-    }
-  wstack->weak_refs[i].notify = notify;
-  wstack->weak_refs[i].data = data;
-  g_datalist_id_set_data_full (&object->qdata, quark_weak_notifies, wstack, weak_refs_notify);
-  object_bit_unlock (object, OPTIONAL_BIT_LOCK_WEAK_REFS);
 }
 
 /**
@@ -3772,37 +3828,17 @@ g_object_weak_ref (GObject    *object,
  * Removes a weak reference callback to an object.
  */
 void
-g_object_weak_unref (GObject    *object,
+g_object_weak_unref (GObject *object,
-		     GWeakNotify notify,
+                     GWeakNotify notify,
-		     gpointer    data)
+                     gpointer data)
 {
-  WeakRefStack *wstack;
-  gboolean found_one = FALSE;
-
   g_return_if_fail (G_IS_OBJECT (object));
   g_return_if_fail (notify != NULL);
 
-  object_bit_lock (object, OPTIONAL_BIT_LOCK_WEAK_REFS);
+  _g_datalist_id_update_atomic (&object->qdata,
-  wstack = g_datalist_id_get_data (&object->qdata, quark_weak_notifies);
+                                quark_weak_notifies,
-  if (wstack)
+                                g_object_weak_unref_cb,
-    {
+                                ((gpointer[]){ notify, data }));
-      guint i;
-
-      for (i = 0; i < wstack->n_weak_refs; i++)
-	if (wstack->weak_refs[i].notify == notify &&
-	    wstack->weak_refs[i].data == data)
-	  {
-	    found_one = TRUE;
-	    wstack->n_weak_refs -= 1;
-	    if (i != wstack->n_weak_refs)
-	      wstack->weak_refs[i] = wstack->weak_refs[wstack->n_weak_refs];
-
-	    break;
-	  }
-    }
-  object_bit_unlock (object, OPTIONAL_BIT_LOCK_WEAK_REFS);
-  if (!found_one)
-    g_critical ("%s: couldn't find weak ref %p(%p)", G_STRFUNC, notify, data);
 }
 
 /**