diff --git a/gobject/ChangeLog b/gobject/ChangeLog index cfdb1bae8..3a5cbeb27 100644 --- a/gobject/ChangeLog +++ b/gobject/ChangeLog @@ -1,3 +1,13 @@ +Mon Aug 1 23:00:42 2005 Tim Janik + + * gclosure.c: turned all modifications to the first 32 integer bits in a + closure into atomic accesses. wrapped write accesses into special macros + to keep the atomic modification logic in a single place. comment cleanups. + + * gclosure.h: made all atomicly accessed closure fields volatile. + + * gobject.h: made ref_count field volatile. + Sun Jul 31 02:04:23 2005 Tim Janik * gobject.c: use g_datalist_set_flags() and g_datalist_unset_flags() to diff --git a/gobject/gclosure.c b/gobject/gclosure.c index 2323384af..c28064453 100644 --- a/gobject/gclosure.c +++ b/gobject/gclosure.c @@ -1,5 +1,6 @@ /* GObject - GLib Type, Object, Parameter and Signal Library * Copyright (C) 2000-2001 Red Hat, Inc. + * Copyright (C) 2005 Imendio AB * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -18,14 +19,15 @@ */ #include "gclosure.h" +/* + * MT safe with regards to reference counting. + */ + #include "gvalue.h" #include "gobjectalias.h" #include -/* FIXME: need caching allocators - */ - #define CLOSURE_MAX_REF_COUNT ((1 << 15) - 1) #define CLOSURE_MAX_N_GUARDS ((1 << 1) - 1) #define CLOSURE_MAX_N_FNOTIFIERS ((1 << 2) - 1) @@ -36,6 +38,45 @@ #define CLOSURE_N_NOTIFIERS(cl) (CLOSURE_N_MFUNCS (cl) + \ (cl)->n_fnotifiers + \ (cl)->n_inotifiers) + +typedef union { + GClosure closure; + gint vint; +} ClosureInt; + +#define CHANGE_FIELD(_closure, _field, _OP, _value, _must_set, _SET_OLD, _SET_NEW) \ +G_STMT_START { \ + ClosureInt *cunion = (ClosureInt*) _closure; \ + gint new_int, old_int, success; \ + do \ + { \ + ClosureInt tmp; \ + tmp.vint = old_int = cunion->vint; \ + _SET_OLD tmp.closure._field; \ + tmp.closure._field _OP _value; \ + _SET_NEW tmp.closure._field; \ + new_int = tmp.vint; \ + success = g_atomic_int_compare_and_exchange (&cunion->vint, old_int, new_int); \ + } \ + while (!success && _must_set); \ +} G_STMT_END + +#define SWAP(_closure, _field, _value, _oldv) CHANGE_FIELD (_closure, _field, =, _value, TRUE, *(_oldv) =, (void) ) +#define SET(_closure, _field, _value) CHANGE_FIELD (_closure, _field, =, _value, TRUE, (void), (void) ) +#define INC(_closure, _field) CHANGE_FIELD (_closure, _field, +=, 1, TRUE, (void), (void) ) +#define INC_ASSIGN(_closure, _field, _newv) CHANGE_FIELD (_closure, _field, +=, 1, TRUE, (void), *(_newv) = ) +#define DEC(_closure, _field) CHANGE_FIELD (_closure, _field, -=, 1, TRUE, (void), (void) ) +#define DEC_ASSIGN(_closure, _field, _newv) CHANGE_FIELD (_closure, _field, -=, 1, TRUE, (void), *(_newv) = ) + +#if 0 // for non-thread-safe closures +#define SWAP(cl,f,v,o) (void) (*(o) = cl->f, cl->f = v) +#define SET(cl,f,v) (void) (cl->f = v) +#define INC(cl,f) (void) (cl->f += 1) +#define INC_ASSIGN(cl,f,n) (void) (cl->f += 1, *(n) = cl->f) +#define DEC(cl,f) (void) (cl->f -= 1) +#define DEC_ASSIGN(cl,f,n) (void) (cl->f -= 1, *(n) = cl->f) +#endif + enum { FNOTIFY, INOTIFY, @@ -53,17 +94,17 @@ g_closure_new_simple (guint sizeof_closure, g_return_val_if_fail (sizeof_closure >= sizeof (GClosure), NULL); - closure = g_malloc (sizeof_closure); - closure->ref_count = 1; - closure->meta_marshal = 0; - closure->n_guards = 0; - closure->n_fnotifiers = 0; - closure->n_inotifiers = 0; - closure->in_inotify = FALSE; - closure->floating = TRUE; - closure->derivative_flag = 0; - closure->in_marshal = FALSE; - closure->is_invalid = FALSE; + closure = g_malloc0 (sizeof_closure); + SET (closure, ref_count, 1); + SET (closure, meta_marshal, 0); + SET (closure, n_guards, 0); + SET (closure, n_fnotifiers, 0); + SET (closure, n_inotifiers, 0); + SET (closure, in_inotify, FALSE); + SET (closure, floating, TRUE); + SET (closure, derivative_flag, 0); + SET (closure, in_marshal, FALSE); + SET (closure, is_invalid, FALSE); closure->marshal = NULL; closure->data = data; closure->notifiers = NULL; @@ -87,8 +128,8 @@ closure_invoke_notifiers (GClosure *closure, * - closure->notifiers may be reloacted during callback * - closure->n_fnotifiers and closure->n_inotifiers may change during callback * - i.e. callbacks can be removed/added during invocation - * - have to prepare for callback removal during invocation (->marshal & ->data) - * - have to distinguish (->marshal & ->data) for INOTIFY/FNOTIFY (->in_inotify) + * - must prepare for callback removal during FNOTIFY and INOTIFY (done via ->marshal= & ->data=) + * - must distinguish (->marshal= & ->data=) for INOTIFY vs. FNOTIFY (via ->in_inotify) * + closure->n_guards is const during PRE_NOTIFY & POST_NOTIFY * + closure->meta_marshal is const for all cases * + none of the callbacks can cause recursion @@ -101,7 +142,8 @@ closure_invoke_notifiers (GClosure *closure, case FNOTIFY: while (closure->n_fnotifiers) { - register guint n = --closure->n_fnotifiers; + guint n; + DEC_ASSIGN (closure, n_fnotifiers, &n); ndata = closure->notifiers + CLOSURE_N_MFUNCS (closure) + n; closure->marshal = (GClosureMarshal) ndata->notify; @@ -112,10 +154,11 @@ closure_invoke_notifiers (GClosure *closure, closure->data = NULL; break; case INOTIFY: - closure->in_inotify = TRUE; + SET (closure, in_inotify, TRUE); while (closure->n_inotifiers) { - register guint n = --closure->n_inotifiers; + guint n; + DEC_ASSIGN (closure, n_inotifiers, &n); ndata = closure->notifiers + CLOSURE_N_MFUNCS (closure) + closure->n_fnotifiers + n; closure->marshal = (GClosureMarshal) ndata->notify; @@ -124,7 +167,7 @@ closure_invoke_notifiers (GClosure *closure, } closure->marshal = NULL; closure->data = NULL; - closure->in_inotify = FALSE; + SET (closure, in_inotify, FALSE); break; case PRE_NOTIFY: i = closure->n_guards; @@ -174,7 +217,7 @@ g_closure_set_meta_marshal (GClosure *closure, } closure->notifiers[0].data = marshal_data; closure->notifiers[0].notify = (GClosureNotify) meta_marshal; - closure->meta_marshal = 1; + SET (closure, meta_marshal, 1); } void @@ -214,11 +257,12 @@ g_closure_add_marshal_guards (GClosure *closure, closure->notifiers[(closure->meta_marshal + closure->n_guards + closure->n_guards + 1)] = closure->notifiers[closure->meta_marshal + closure->n_guards]; - i = closure->n_guards++; + i = closure->n_guards; closure->notifiers[closure->meta_marshal + i].data = pre_marshal_data; closure->notifiers[closure->meta_marshal + i].notify = pre_marshal_notify; closure->notifiers[closure->meta_marshal + i + 1].data = post_marshal_data; closure->notifiers[closure->meta_marshal + i + 1].notify = post_marshal_notify; + INC (closure, n_guards); } void @@ -238,9 +282,10 @@ g_closure_add_finalize_notifier (GClosure *closure, closure->n_fnotifiers + closure->n_inotifiers)] = closure->notifiers[(CLOSURE_N_MFUNCS (closure) + closure->n_fnotifiers + 0)]; - i = CLOSURE_N_MFUNCS (closure) + closure->n_fnotifiers++; + i = CLOSURE_N_MFUNCS (closure) + closure->n_fnotifiers; closure->notifiers[i].data = notify_data; closure->notifiers[i].notify = notify_func; + INC (closure, n_fnotifiers); } void @@ -256,9 +301,10 @@ g_closure_add_invalidate_notifier (GClosure *closure, g_return_if_fail (closure->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++; + i = CLOSURE_N_MFUNCS (closure) + closure->n_fnotifiers + closure->n_inotifiers; closure->notifiers[i].data = notify_data; closure->notifiers[i].notify = notify_func; + INC (closure, n_inotifiers); } static inline gboolean @@ -272,7 +318,7 @@ closure_try_remove_inotify (GClosure *closure, for (ndata = nlast + 1 - closure->n_inotifiers; ndata <= nlast; ndata++) if (ndata->notify == notify_func && ndata->data == notify_data) { - closure->n_inotifiers -= 1; + DEC (closure, n_inotifiers); if (ndata < nlast) *ndata = *nlast; @@ -292,7 +338,7 @@ closure_try_remove_fnotify (GClosure *closure, for (ndata = nlast + 1 - closure->n_fnotifiers; ndata <= nlast; ndata++) if (ndata->notify == notify_func && ndata->data == notify_data) { - closure->n_fnotifiers -= 1; + DEC (closure, n_fnotifiers); if (ndata < nlast) *ndata = *nlast; if (closure->n_inotifiers) @@ -308,11 +354,13 @@ 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); - closure->ref_count += 1; + INC_ASSIGN (closure, ref_count, &new_ref_count); + g_return_val_if_fail (new_ref_count > 1, NULL); return closure; } @@ -324,9 +372,12 @@ g_closure_invalidate (GClosure *closure) if (!closure->is_invalid) { - closure->ref_count += 1; /* preserve floating flag */ - closure->is_invalid = TRUE; - closure_invoke_notifiers (closure, INOTIFY); + gboolean was_invalid; + g_closure_ref (closure); /* preserve floating flag */ + SWAP (closure, is_invalid, TRUE, &was_invalid); + /* invalidate only once */ + if (!was_invalid) + closure_invoke_notifiers (closure, INOTIFY); g_closure_unref (closure); } } @@ -334,15 +385,17 @@ g_closure_invalidate (GClosure *closure) void g_closure_unref (GClosure *closure) { + guint new_ref_count; + g_return_if_fail (closure != NULL); g_return_if_fail (closure->ref_count > 0); if (closure->ref_count == 1) /* last unref, invalidate first */ g_closure_invalidate (closure); - closure->ref_count -= 1; + DEC_ASSIGN (closure, ref_count, &new_ref_count); - if (closure->ref_count == 0) + if (new_ref_count == 0) { closure_invoke_notifiers (closure, FNOTIFY); g_free (closure->notifiers); @@ -363,11 +416,11 @@ g_closure_sink (GClosure *closure) */ if (closure->floating) { - closure->floating = FALSE; - if (closure->ref_count > 1) - closure->ref_count -= 1; - else - g_closure_unref (closure); + gboolean was_floating; + SWAP (closure, floating, FALSE, &was_floating); + /* unref floating flag only once */ + if (was_floating) + g_closure_unref (closure); } } @@ -380,7 +433,8 @@ g_closure_remove_invalidate_notifier (GClosure *closure, g_return_if_fail (notify_func != NULL); if (closure->is_invalid && closure->in_inotify && /* account removal of notify_func() while its called */ - ((gpointer) closure->marshal) == ((gpointer) notify_func) && closure->data == notify_data) + ((gpointer) closure->marshal) == ((gpointer) notify_func) && + closure->data == notify_data) closure->marshal = NULL; else if (!closure_try_remove_inotify (closure, notify_data, notify_func)) g_warning (G_STRLOC ": unable to remove uninstalled invalidation notifier: %p (%p)", @@ -396,7 +450,8 @@ g_closure_remove_finalize_notifier (GClosure *closure, g_return_if_fail (notify_func != NULL); if (closure->is_invalid && !closure->in_inotify && /* account removal of notify_func() while its called */ - ((gpointer) closure->marshal) == ((gpointer) notify_func) && closure->data == notify_data) + ((gpointer) closure->marshal) == ((gpointer) notify_func) && + closure->data == notify_data) closure->marshal = NULL; else if (!closure_try_remove_fnotify (closure, notify_data, notify_func)) g_warning (G_STRLOC ": unable to remove uninstalled finalization notifier: %p (%p)", @@ -412,6 +467,7 @@ g_closure_invoke (GClosure *closure, { g_return_if_fail (closure != NULL); + g_closure_ref (closure); /* preserve floating flag */ if (!closure->is_invalid) { GClosureMarshal marshal; @@ -420,8 +476,7 @@ g_closure_invoke (GClosure *closure, g_return_if_fail (closure->marshal || closure->meta_marshal); - closure->ref_count += 1; /* preserve floating flag */ - closure->in_marshal = TRUE; + SET (closure, in_marshal, TRUE); if (closure->meta_marshal) { marshal_data = closure->notifiers[0].data; @@ -441,9 +496,9 @@ g_closure_invoke (GClosure *closure, marshal_data); if (!in_marshal) closure_invoke_notifiers (closure, POST_NOTIFY); - closure->in_marshal = in_marshal; - g_closure_unref (closure); + SET (closure, in_marshal, in_marshal); } + g_closure_unref (closure); } void @@ -490,7 +545,7 @@ g_cclosure_new_swap (GCallback callback_func, if (destroy_data) g_closure_add_finalize_notifier (closure, user_data, destroy_data); ((GCClosure*) closure)->callback = (gpointer) callback_func; - closure->derivative_flag = TRUE; + SET (closure, derivative_flag, TRUE); return closure; } diff --git a/gobject/gclosure.h b/gobject/gclosure.h index ecb5e3f2a..7a2078d58 100644 --- a/gobject/gclosure.h +++ b/gobject/gclosure.h @@ -1,5 +1,6 @@ /* GObject - GLib Type, Object, Parameter and Signal Library * Copyright (C) 2000-2001 Red Hat, Inc. + * Copyright (C) 2005 Imendio AB * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -58,16 +59,19 @@ struct _GClosureNotifyData }; struct _GClosure { - /*< private >*/ guint ref_count : 15; - /*< private >*/ guint meta_marshal : 1; - /*< private >*/ guint n_guards : 1; - /*< private >*/ guint n_fnotifiers : 2; /* finalization notifiers */ - /*< private >*/ guint n_inotifiers : 8; /* invalidation notifiers */ - /*< private >*/ guint in_inotify : 1; - /*< private >*/ guint floating : 1; - /*< protected >*/ guint derivative_flag : 1; - /*< public >*/ guint in_marshal : 1; - /*< public >*/ guint is_invalid : 1; + /*< private >*/ + volatile guint ref_count : 15; + volatile guint meta_marshal : 1; + volatile guint n_guards : 1; + volatile guint n_fnotifiers : 2; /* finalization notifiers */ + volatile guint n_inotifiers : 8; /* invalidation notifiers */ + volatile guint in_inotify : 1; + volatile guint floating : 1; + /*< protected >*/ + volatile guint derivative_flag : 1; + /*< public >*/ + volatile guint in_marshal : 1; + volatile guint is_invalid : 1; /*< private >*/ void (*marshal) (GClosure *closure, GValue /*out*/ *return_value, diff --git a/gobject/gobject.c b/gobject/gobject.c index e3f3e2ddc..6628ba7d4 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -21,7 +21,7 @@ #include /* - * MT safe + * MT safe with regards to reference counting. */ #include "gvaluecollector.h" diff --git a/gobject/gobject.h b/gobject/gobject.h index c56e330dd..da2c47c64 100644 --- a/gobject/gobject.h +++ b/gobject/gobject.h @@ -62,11 +62,11 @@ typedef void (*GWeakNotify) (gpointer data, GObject *where_the_object_was); struct _GObject { - GTypeInstance g_type_instance; + GTypeInstance g_type_instance; /*< private >*/ - guint ref_count; - GData *qdata; + volatile guint ref_count; + GData *qdata; }; struct _GObjectClass {