[glib/glib-2-72: 19/39] gsignal: Perform signal unlocked handlers block, unblock and disconnect ops

[Matthias Clasen <matthiasc src gnome org>]

commit f29beb246c78d80f98320806d957343e9ef0e556
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Fri Jul 15 16:05:23 2022 +0200

    gsignal: Perform signal unlocked handlers block, unblock and disconnect ops
    
    We used to perform unneeded lock/unlock dances to perform block, unblock
    and disconnect actions, and these were potentially unsafe because we
    might have looped in data that could be potentially be changed by other
    threads.
    
    We could have also done the same by saving the handlers ids in a
    temporary array and eventually remove them, but I don't see a reason for
    that since we can just keep all locked without the risk of creating
    deadlocks.
    
    Coverity CID: #1474757, #1474771, #1474429
    
    (cherry-picked from commit ae14f3219a756fa99dbbbb54555f10dd48eb0fea)

 gobject/gsignal.c | 102 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 34 deletions(-)
---
diff --git a/gobject/gsignal.c b/gobject/gsignal.c
index 1fafcae5db..aeaf24407e 100644
--- a/gobject/gsignal.c
+++ b/gobject/gsignal.c
@@ -2616,6 +2616,10 @@ g_signal_connect_data (gpointer       instance,
   return handler_seq_no;
 }
 
+static void
+signal_handler_block_unlocked (gpointer instance,
+                               gulong   handler_id);
+
 /**
  * g_signal_handler_block:
  * @instance: (type GObject.Object): The instance to block the signal handler of.
@@ -2634,12 +2638,20 @@ void
 g_signal_handler_block (gpointer instance,
                         gulong   handler_id)
 {
-  Handler *handler;
-  
   g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance));
   g_return_if_fail (handler_id > 0);
   
   SIGNAL_LOCK ();
+  signal_handler_block_unlocked (instance, handler_id);
+  SIGNAL_UNLOCK ();
+}
+
+static void
+signal_handler_block_unlocked (gpointer instance,
+                               gulong   handler_id)
+{
+  Handler *handler;
+
   handler = handler_lookup (instance, handler_id, NULL, NULL);
   if (handler)
     {
@@ -2651,9 +2663,12 @@ g_signal_handler_block (gpointer instance,
     }
   else
     g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id);
-  SIGNAL_UNLOCK ();
 }
 
+static void
+signal_handler_unblock_unlocked (gpointer instance,
+                                 gulong   handler_id);
+
 /**
  * g_signal_handler_unblock:
  * @instance: (type GObject.Object): The instance to unblock the signal handler of.
@@ -2677,12 +2692,20 @@ void
 g_signal_handler_unblock (gpointer instance,
                           gulong   handler_id)
 {
-  Handler *handler;
-  
   g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance));
   g_return_if_fail (handler_id > 0);
   
   SIGNAL_LOCK ();
+  signal_handler_unblock_unlocked (instance, handler_id);
+  SIGNAL_UNLOCK ();
+}
+
+static void
+signal_handler_unblock_unlocked (gpointer instance,
+                                 gulong   handler_id)
+{
+  Handler *handler;
+
   handler = handler_lookup (instance, handler_id, NULL, NULL);
   if (handler)
     {
@@ -2693,9 +2716,12 @@ g_signal_handler_unblock (gpointer instance,
     }
   else
     g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id);
-  SIGNAL_UNLOCK ();
 }
 
+static void
+signal_handler_disconnect_unlocked (gpointer instance,
+                                    gulong   handler_id);
+
 /**
  * g_signal_handler_disconnect:
  * @instance: (type GObject.Object): The instance to remove the signal handler from.
@@ -2712,12 +2738,20 @@ void
 g_signal_handler_disconnect (gpointer instance,
                              gulong   handler_id)
 {
-  Handler *handler;
-  
   g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance));
   g_return_if_fail (handler_id > 0);
   
   SIGNAL_LOCK ();
+  signal_handler_disconnect_unlocked (instance, handler_id);
+  SIGNAL_UNLOCK ();
+}
+
+static void
+signal_handler_disconnect_unlocked (gpointer instance,
+                                    gulong   handler_id)
+{
+  Handler *handler;
+
   handler = handler_lookup (instance, handler_id, 0, 0);
   if (handler)
     {
@@ -2729,7 +2763,6 @@ g_signal_handler_disconnect (gpointer instance,
     }
   else
     g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id);
-  SIGNAL_UNLOCK ();
 }
 
 /**
@@ -2860,33 +2893,31 @@ g_signal_handler_find (gpointer         instance,
   return handler_seq_no;
 }
 
+typedef void (*CallbackHandlerFunc) (gpointer instance, gulong handler_seq_no);
+
 static guint
-signal_handlers_foreach_matched_R (gpointer         instance,
-                                  GSignalMatchType mask,
-                                  guint            signal_id,
-                                  GQuark           detail,
-                                  GClosure        *closure,
-                                  gpointer         func,
-                                  gpointer         data,
-                                  void           (*callback) (gpointer instance,
-                                                              gulong   handler_seq_no))
+signal_handlers_foreach_matched_unlocked_R (gpointer             instance,
+                                            GSignalMatchType     mask,
+                                            guint                signal_id,
+                                            GQuark               detail,
+                                            GClosure            *closure,
+                                            gpointer             func,
+                                            gpointer             data,
+                                            CallbackHandlerFunc  callback)
 {
   HandlerMatch *mlist;
   guint n_handlers = 0;
-  
+
   mlist = handlers_find (instance, mask, signal_id, detail, closure, func, data, FALSE);
   while (mlist)
     {
       n_handlers++;
       if (mlist->handler->sequential_number)
-       {
-         SIGNAL_UNLOCK ();
-         callback (instance, mlist->handler->sequential_number);
-         SIGNAL_LOCK ();
-       }
+        callback (instance, mlist->handler->sequential_number);
+
       mlist = handler_match_free1_R (mlist, instance);
     }
-  
+
   return n_handlers;
 }
 
@@ -2928,9 +2959,10 @@ g_signal_handlers_block_matched (gpointer         instance,
   if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA))
     {
       SIGNAL_LOCK ();
-      n_handlers = signal_handlers_foreach_matched_R (instance, mask, signal_id, detail,
-                                                     closure, func, data,
-                                                     g_signal_handler_block);
+      n_handlers =
+        signal_handlers_foreach_matched_unlocked_R (instance, mask, signal_id, detail,
+                                                    closure, func, data,
+                                                    signal_handler_block_unlocked);
       SIGNAL_UNLOCK ();
     }
   
@@ -2976,9 +3008,10 @@ g_signal_handlers_unblock_matched (gpointer         instance,
   if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA))
     {
       SIGNAL_LOCK ();
-      n_handlers = signal_handlers_foreach_matched_R (instance, mask, signal_id, detail,
-                                                     closure, func, data,
-                                                     g_signal_handler_unblock);
+      n_handlers =
+        signal_handlers_foreach_matched_unlocked_R (instance, mask, signal_id, detail,
+                                                    closure, func, data,
+                                                    signal_handler_unblock_unlocked);
       SIGNAL_UNLOCK ();
     }
   
@@ -3024,9 +3057,10 @@ g_signal_handlers_disconnect_matched (gpointer         instance,
   if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA))
     {
       SIGNAL_LOCK ();
-      n_handlers = signal_handlers_foreach_matched_R (instance, mask, signal_id, detail,
-                                                     closure, func, data,
-                                                     g_signal_handler_disconnect);
+      n_handlers =
+        signal_handlers_foreach_matched_unlocked_R (instance, mask, signal_id, detail,
+                                                    closure, func, data,
+                                                    signal_handler_disconnect_unlocked);
       SIGNAL_UNLOCK ();
     }