Merge branch 'closure-cleanups' into 'main'

gclosure: Allow full set of closure flags to be queried atomically

See merge request GNOME/glib!4577

gobject/gclosure.c

@@ -101,36 +101,67 @@
                                          (cl)->n_fnotifiers + \
                                          (cl)->n_inotifiers)
 
+/* A copy of the flags bitfield from the beginning of `struct _GClosure`, which
+ * is in union with an int for atomic access to all fields at the same time.
+ *
+ * This must be kept in sync with `struct _GClosure`, but that’s easy because
+ * it’s (unfortunately) a public struct, so can never change for ABI
+ * compatibility reasons. */
 typedef union {
+  struct {
+    guint ref_count : 15;  /* (atomic) */
+    /* meta_marshal is not used anymore but must be zero for historical reasons
+       as it was exposed in the G_CLOSURE_N_NOTIFIERS macro */
+    guint meta_marshal_nouse : 1;  /* (atomic) */
+    guint n_guards : 1;  /* (atomic) */
+    guint n_fnotifiers : 2;  /* finalization notifiers (atomic) */
+    guint n_inotifiers : 8;  /* invalidation notifiers (atomic) */
+    guint in_inotify : 1;  /* (atomic) */
+    guint floating : 1;  /* (atomic) */
+    guint derivative_flag : 1;  /* (atomic) */
+    guint in_marshal : 1;  /* (atomic) */
+    guint is_invalid : 1;  /* (atomic) */
+  } flags;
   GClosure closure;
-  gint vint;
+  int atomic_int;
-} ClosureInt;
+} GClosureFlags;
 
-#define ATOMIC_CHANGE_FIELD(_closure, _field, _OP, _value, _must_set, _SET_OLD, _SET_NEW)      \
+G_STATIC_ASSERT (sizeof (GClosureFlags) == sizeof (GClosure));
+G_STATIC_ASSERT (G_ALIGNOF (GClosureFlags) == G_ALIGNOF (GClosure));
+
+#define ATOMIC_CHANGE_FIELD(_closure, _field, _OP, _value, _SET_OLD, _SET_NEW) \
 G_STMT_START {                                                                          \
-  ClosureInt *cunion = (ClosureInt*) _closure;                 		                \
+  GClosureFlags *cunion = (GClosureFlags *) _closure; \
   gint new_int, old_int, success;                              		                \
-  old_int = g_atomic_int_get (&cunion->vint);                   	                \
+  old_int = g_atomic_int_get (&cunion->atomic_int); \
   do                                                    		                \
     {                                                   		                \
-      ClosureInt tmp;                                   		                \
+      GClosureFlags tmp; \
-      tmp.vint = old_int;                               		                \
+      tmp.atomic_int = old_int; \
-      _SET_OLD tmp.closure._field;                                                      \
+      _SET_OLD tmp.flags._field; \
-      tmp.closure._field _OP _value;                      		                \
+      tmp.flags._field _OP _value; \
-      _SET_NEW tmp.closure._field;                                                      \
+      _SET_NEW tmp.flags._field; \
-      new_int = tmp.vint;                               		                \
+      new_int = tmp.atomic_int; \
-      success = g_atomic_int_compare_and_exchange_full (&cunion->vint, old_int, new_int,\
+      success = g_atomic_int_compare_and_exchange_full (&cunion->atomic_int, old_int, new_int, \
 							&old_int);			\
     }                                                   		                \
-  while (!success && _must_set);                                                        \
+  while (!success); \
 } G_STMT_END
 
-#define ATOMIC_SWAP(_closure, _field, _value, _oldv)   ATOMIC_CHANGE_FIELD (_closure, _field, =, _value, TRUE, *(_oldv) =,     (void) )
+#define ATOMIC_SWAP(_closure, _field, _value, _oldv)   ATOMIC_CHANGE_FIELD (_closure, _field, =, _value, *(_oldv) =,     (void) )
-#define ATOMIC_SET(_closure, _field, _value)           ATOMIC_CHANGE_FIELD (_closure, _field, =, _value, TRUE,     (void),     (void) )
+#define ATOMIC_SET(_closure, _field, _value)           ATOMIC_CHANGE_FIELD (_closure, _field, =, _value,     (void),     (void) )
-#define ATOMIC_INC(_closure, _field)                   ATOMIC_CHANGE_FIELD (_closure, _field, +=,     1, TRUE,     (void),     (void) )
+#define ATOMIC_INC(_closure, _field)                   ATOMIC_CHANGE_FIELD (_closure, _field, +=,     1,     (void),     (void) )
-#define ATOMIC_INC_ASSIGN(_closure, _field, _newv)     ATOMIC_CHANGE_FIELD (_closure, _field, +=,     1, TRUE,     (void), *(_newv) = )
+#define ATOMIC_INC_ASSIGN(_closure, _field, _newv)     ATOMIC_CHANGE_FIELD (_closure, _field, +=,     1,     (void), *(_newv) = )
-#define ATOMIC_DEC(_closure, _field)                   ATOMIC_CHANGE_FIELD (_closure, _field, -=,     1, TRUE,     (void),     (void) )
+#define ATOMIC_DEC(_closure, _field)                   ATOMIC_CHANGE_FIELD (_closure, _field, -=,     1,     (void),     (void) )
-#define ATOMIC_DEC_ASSIGN(_closure, _field, _newv)     ATOMIC_CHANGE_FIELD (_closure, _field, -=,     1, TRUE,     (void), *(_newv) = )
+#define ATOMIC_DEC_ASSIGN(_closure, _field, _newv)     ATOMIC_CHANGE_FIELD (_closure, _field, -=,     1,     (void), *(_newv) = )
+
+static inline GClosureFlags
+closure_atomic_get_flags (GClosure *closure)
+{
+  GClosureFlags tmp;
+  tmp.atomic_int = g_atomic_int_get (&((GClosureFlags *) closure)->atomic_int);
+  return tmp;
+}
 
 enum {
   FNOTIFY,
@@ -305,12 +336,15 @@ static void
 g_closure_set_meta_va_marshal (GClosure       *closure,
 			       GVaClosureMarshal va_meta_marshal)
 {
+  GClosureFlags old_flags;
   GRealClosure *real_closure;
 
   g_return_if_fail (closure != NULL);
   g_return_if_fail (va_meta_marshal != NULL);
-  g_return_if_fail (closure->is_invalid == FALSE);
+
-  g_return_if_fail (closure->in_marshal == FALSE);
+  old_flags = closure_atomic_get_flags (closure);
+  g_return_if_fail (!old_flags.flags.is_invalid);
+  g_return_if_fail (!old_flags.flags.in_marshal);
 
   real_closure = G_REAL_CLOSURE (closure);
 
@@ -349,12 +383,15 @@ g_closure_set_meta_marshal (GClosure       *closure,
 			    gpointer        marshal_data,
 			    GClosureMarshal meta_marshal)
 {
+  GClosureFlags old_flags;
   GRealClosure *real_closure;
 
   g_return_if_fail (closure != NULL);
   g_return_if_fail (meta_marshal != NULL);
-  g_return_if_fail (closure->is_invalid == FALSE);
+
-  g_return_if_fail (closure->in_marshal == FALSE);
+  old_flags = closure_atomic_get_flags (closure);
+  g_return_if_fail (!old_flags.flags.is_invalid);
+  g_return_if_fail (!old_flags.flags.in_marshal);
 
   real_closure = G_REAL_CLOSURE (closure);
 
@@ -388,14 +425,17 @@ g_closure_add_marshal_guards (GClosure      *closure,
 			      gpointer       post_marshal_data,
 			      GClosureNotify post_marshal_notify)
 {
+  GClosureFlags old_flags;
   guint i;
 
   g_return_if_fail (closure != NULL);
   g_return_if_fail (pre_marshal_notify != NULL);
   g_return_if_fail (post_marshal_notify != NULL);
-  g_return_if_fail (closure->is_invalid == FALSE);
+
-  g_return_if_fail (closure->in_marshal == FALSE);
+  old_flags = closure_atomic_get_flags (closure);
-  g_return_if_fail (closure->n_guards < CLOSURE_MAX_N_GUARDS);
+  g_return_if_fail (!old_flags.flags.is_invalid);
+  g_return_if_fail (!old_flags.flags.in_marshal);
+  g_return_if_fail (old_flags.flags.n_guards < CLOSURE_MAX_N_GUARDS);
 
   closure->notifiers = g_renew (GClosureNotifyData, closure->notifiers, CLOSURE_N_NOTIFIERS (closure) + 2);
   if (closure->n_inotifiers)
@@ -479,12 +519,15 @@ g_closure_add_invalidate_notifier (GClosure      *closure,
 				   gpointer       notify_data,
 				   GClosureNotify notify_func)
 {
+  GClosureFlags old_flags;
   guint i;
 
   g_return_if_fail (closure != NULL);
   g_return_if_fail (notify_func != NULL);
-  g_return_if_fail (closure->is_invalid == FALSE);
+
-  g_return_if_fail (closure->n_inotifiers < CLOSURE_MAX_N_INOTIFIERS);
+  old_flags = closure_atomic_get_flags (closure);
+  g_return_if_fail (!old_flags.flags.is_invalid);
+  g_return_if_fail (old_flags.flags.n_inotifiers < CLOSURE_MAX_N_INOTIFIERS);
 
   closure->notifiers = g_renew (GClosureNotifyData, closure->notifiers, CLOSURE_N_NOTIFIERS (closure) + 1);
   i = CLOSURE_N_MFUNCS (closure) + closure->n_fnotifiers + closure->n_inotifiers;
@@ -537,6 +580,30 @@ closure_try_remove_fnotify (GClosure       *closure,
   return FALSE;
 }
 
+static GClosureFlags
+closure_ref_internal (GClosure *closure)
+{
+  guint new_ref_count;
+  int old_int, success;
+  GClosureFlags tmp = { .atomic_int = 0, };
+
+  old_int = g_atomic_int_get (&((GClosureFlags *) closure)->atomic_int);
+  do
+    {
+      tmp.atomic_int = old_int;
+      tmp.flags.ref_count += 1;
+      success = g_atomic_int_compare_and_exchange_full (&((GClosureFlags *) closure)->atomic_int, old_int, tmp.atomic_int, &old_int);
+    }
+  while (!success);
+
+  new_ref_count = tmp.flags.ref_count;
+
+  g_return_val_if_fail (new_ref_count > 1, tmp);
+  g_return_val_if_fail (new_ref_count < CLOSURE_MAX_REF_COUNT + 1, tmp);
+
+  return tmp;
+}
+
 /**
  * g_closure_ref:
  * @closure: #GClosure to increment the reference count on
@@ -549,14 +616,9 @@ closure_try_remove_fnotify (GClosure       *closure,
 GClosure*
 g_closure_ref (GClosure *closure)
 {
-  guint new_ref_count;
   g_return_val_if_fail (closure != NULL, NULL);
-  g_return_val_if_fail (closure->ref_count > 0, NULL);
-  g_return_val_if_fail (closure->ref_count < CLOSURE_MAX_REF_COUNT, NULL);
-
-  ATOMIC_INC_ASSIGN (closure, ref_count, &new_ref_count);
-  g_return_val_if_fail (new_ref_count > 1, NULL);
 
+  closure_ref_internal (closure);
   return closure;
 }
 
@@ -594,9 +656,12 @@ closure_invalidate_internal (GClosure *closure)
 void
 g_closure_invalidate (GClosure *closure)
 {
+  GClosureFlags old_flags;
+
   g_return_if_fail (closure != NULL);
 
-  if (!closure->is_invalid)
+  old_flags = closure_atomic_get_flags (closure);
+  if (!old_flags.flags.is_invalid)
     {
       g_closure_ref (closure);           /* preserve floating flag */
       closure_invalidate_internal (closure);
@@ -618,12 +683,15 @@ void
 g_closure_unref (GClosure *closure)
 {
   guint new_ref_count;
+  GClosureFlags old_flags;
 
   g_return_if_fail (closure != NULL);
-  g_return_if_fail (closure->ref_count > 0);
+
+  old_flags = closure_atomic_get_flags (closure);
+  g_return_if_fail (old_flags.flags.ref_count > 0);
 
   /* last unref, invalidate first */
-  if (closure->ref_count == 1 && !closure->is_invalid)
+  if (old_flags.flags.ref_count == 1 && !old_flags.flags.is_invalid)
     closure_invalidate_internal (closure);
 
   ATOMIC_DEC_ASSIGN (closure, ref_count, &new_ref_count);
@@ -709,15 +777,19 @@ g_closure_unref (GClosure *closure)
 void
 g_closure_sink (GClosure *closure)
 {
+  GClosureFlags old_flags;
+
   g_return_if_fail (closure != NULL);
-  g_return_if_fail (closure->ref_count > 0);
+
+  old_flags = closure_atomic_get_flags (closure);
+  g_return_if_fail (old_flags.flags.ref_count > 0);
 
   /* floating is basically a kludge to avoid creating closures
    * with a ref_count of 0. so the initial ref_count a closure has
    * is unowned. with invoking g_closure_sink() code may
    * indicate that it takes over that initial ref_count.
    */
-  if (closure->floating)
+  if (old_flags.flags.floating)
     {
       gboolean was_floating;
       ATOMIC_SWAP (closure, floating, FALSE, &was_floating);
@@ -743,10 +815,14 @@ g_closure_remove_invalidate_notifier (GClosure      *closure,
 				      gpointer       notify_data,
 				      GClosureNotify notify_func)
 {
+  GClosureFlags old_flags;
+
   g_return_if_fail (closure != NULL);
   g_return_if_fail (notify_func != NULL);
 
-  if (closure->is_invalid && closure->in_inotify && /* account removal of notify_func() while it's called */
+  old_flags = closure_atomic_get_flags (closure);
+
+  if (old_flags.flags.is_invalid && old_flags.flags.in_inotify && /* account removal of notify_func() while it's called */
       ((gpointer) closure->marshal) == ((gpointer) notify_func) &&
       closure->data == notify_data)
     closure->marshal = NULL;
@@ -771,10 +847,14 @@ g_closure_remove_finalize_notifier (GClosure      *closure,
 				    gpointer       notify_data,
 				    GClosureNotify notify_func)
 {
+  GClosureFlags old_flags;
+
   g_return_if_fail (closure != NULL);
   g_return_if_fail (notify_func != NULL);
 
-  if (closure->is_invalid && !closure->in_inotify && /* account removal of notify_func() while it's called */
+  old_flags = closure_atomic_get_flags (closure);
+
+  if (old_flags.flags.is_invalid && !old_flags.flags.in_inotify && /* account removal of notify_func() while it's called */
       ((gpointer) closure->marshal) == ((gpointer) notify_func) &&
       closure->data == notify_data)
     closure->marshal = NULL;
@@ -804,18 +884,19 @@ g_closure_invoke (GClosure       *closure,
 		  const GValue   *param_values,
 		  gpointer        invocation_hint)
 {
+  GClosureFlags reffed_flags;
   GRealClosure *real_closure;
 
   g_return_if_fail (closure != NULL);
 
   real_closure = G_REAL_CLOSURE (closure);
 
-  g_closure_ref (closure);      /* preserve floating flag */
+  reffed_flags = closure_ref_internal (closure);  /* preserve floating flag */
-  if (!closure->is_invalid)
+  if (!reffed_flags.flags.is_invalid)
     {
       GClosureMarshal marshal;
       gpointer marshal_data;
-      gboolean in_marshal = closure->in_marshal;
+      gboolean in_marshal = reffed_flags.flags.in_marshal;
 
       g_return_if_fail (closure->marshal || real_closure->meta_marshal);
 
@@ -867,18 +948,19 @@ _g_closure_invoke_va (GClosure       *closure,
 		      int             n_params,
 		      GType          *param_types)
 {
+  GClosureFlags reffed_flags;
   GRealClosure *real_closure;
 
   g_return_if_fail (closure != NULL);
 
   real_closure = G_REAL_CLOSURE (closure);
 
-  g_closure_ref (closure);      /* preserve floating flag */
+  reffed_flags = closure_ref_internal (closure);  /* preserve floating flag */
-  if (!closure->is_invalid)
+  if (!reffed_flags.flags.is_invalid)
     {
       GVaClosureMarshal marshal;
       gpointer marshal_data;
-      gboolean in_marshal = closure->in_marshal;
+      gboolean in_marshal = reffed_flags.flags.in_marshal;
 
       g_return_if_fail (closure->marshal || real_closure->meta_marshal);
 
@@ -1095,13 +1177,16 @@ gboolean
 _g_closure_is_void (GClosure *closure,
 		    gpointer instance)
 {
+  GClosureFlags old_flags;
   GRealClosure *real_closure;
   GTypeClass *class;
   gpointer callback;
   GType itype;
   guint offset;
 
-  if (closure->is_invalid)
+  old_flags = closure_atomic_get_flags (closure);
+
+  if (old_flags.flags.is_invalid)
     return TRUE;
 
   real_closure = G_REAL_CLOSURE (closure);