From 94c735a2aa6d24d68446ecb5501ccba3764fd032 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 14 Dec 2023 12:02:23 +0100
Subject: [PATCH] gobject: don't freeze to queue notify event in
 g_object_notify_by_spec_internal()

Previously:

- if the object is currently not frozen, we called
  g_object_notify_queue_freeze() once. Afterwards dispatch the event
  directly. This is probably the common case, and requires one
  notify_lock lock.

- if the object is currently frozen, we call
  g_object_notify_queue_freeze(), g_object_notify_queue_add().
  g_object_notify_queue_thaw().
  This required taking the notify_lock three times.

- if the object is currently not frozen and in_init, then we called
  g_object_notify_queue_freeze(), g_object_notify_queue_freeze(),
  g_object_notify_queue_add(). This also required to take
  the lock three times. There is another thaw at the end of
  object initialization.

That was because we first call g_object_notify_queue_freeze() to see
whether we are frozen. And depending on that, queue the event (and thaw
again).

Instead, g_object_notify_queue_add() can do the check and queueing in
one step. There is no need to call a freeze() to (conditionally) enqueue
a notification. Now only one lock is taken in all cases.

Also, g_object_notify_queue_freeze() and g_object_notify_queue_thaw()
both call g_datalist_id_get_data() (which also take a bit lock). As the
thaw is no longer necessary, the second lock is also saved.
---
 gobject/gobject.c | 60 +++++++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/gobject/gobject.c b/gobject/gobject.c
index 97dfd8571..cd8c1669b 100644
--- a/gobject/gobject.c
+++ b/gobject/gobject.c
@@ -320,13 +320,41 @@ g_object_notify_queue_thaw (GObject            *object,
   g_free (free_me);
 }
 
-static void
+static gboolean
 g_object_notify_queue_add (GObject            *object,
                            GObjectNotifyQueue *nqueue,
-                           GParamSpec         *pspec)
+                           GParamSpec         *pspec,
+                           gboolean            in_init)
 {
   G_LOCK(notify_lock);
 
+  if (!nqueue)
+    {
+      /* We are called without an nqueue. Figure out whether a notification
+       * should be queued. */
+      nqueue = g_datalist_id_get_data (&object->qdata, quark_notify_queue);
+
+      if (!nqueue)
+        {
+          if (!in_init)
+            {
+              /* We don't have a notify queue and are not in_init. The event
+               * is not to be queued. The caller will dispatch directly. */
+              G_UNLOCK (notify_lock);
+              return FALSE;
+            }
+
+          /* We are "in_init", but did not freeze the queue in g_object_init
+           * yet. Instead, we gained a notify handler in instance init, so now
+           * we need to freeze just-in-time.
+           *
+           * Note that this freeze will be balanced at the end of object
+           * initialization.
+           */
+          nqueue = g_object_notify_queue_create_queue_frozen (object);
+        }
+    }
+
   g_assert (nqueue->n_pspecs < 65535);
 
   if (g_slist_find (nqueue->pspecs, pspec) == NULL)
@@ -336,6 +364,8 @@ g_object_notify_queue_add (GObject            *object,
     }
 
   G_UNLOCK(notify_lock);
+
+  return TRUE;
 }
 
 #ifdef	G_ENABLE_DEBUG
@@ -1506,29 +1536,7 @@ g_object_notify_by_spec_internal (GObject    *object,
 
   if (pspec != NULL && needs_notify)
     {
-      GObjectNotifyQueue *nqueue;
-      gboolean need_thaw = TRUE;
-
-      /* conditional freeze: only increase freeze count if already frozen */
-      nqueue = g_object_notify_queue_freeze (object, TRUE);
-      if (in_init && !nqueue)
-        {
-          /* We did not freeze the queue in g_object_init, but
-           * we gained a notify handler in instance init, so
-           * now we need to freeze just-in-time
-           */
-          nqueue = g_object_notify_queue_freeze (object, FALSE);
-          need_thaw = FALSE;
-        }
-
-      if (nqueue != NULL)
-        {
-          /* we're frozen, so add to the queue and release our freeze */
-          g_object_notify_queue_add (object, nqueue, pspec);
-          if (need_thaw)
-            g_object_notify_queue_thaw (object, nqueue, FALSE);
-        }
-      else
+      if (!g_object_notify_queue_add (object, NULL, pspec, in_init))
         {
           /*
            * Coverity doesn’t understand the paired ref/unref here and seems to
@@ -1825,7 +1833,7 @@ object_set_property (GObject             *object,
 
   if ((pspec->flags & (G_PARAM_EXPLICIT_NOTIFY | G_PARAM_READABLE)) == G_PARAM_READABLE &&
       nqueue != NULL)
-    g_object_notify_queue_add (object, nqueue, pspec);
+    g_object_notify_queue_add (object, nqueue, pspec, FALSE);
 }
 
 static void