From 63a05fca9d570bc2829fe537859236da8934eda6 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 1 Sep 2011 17:12:46 +0100 Subject: [PATCH] Lock the main context when modifying the repaint functions list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repaint functions list can (and should) be manipulated from different threads, but it currently doesn't prevent multiple threads from accessing it concurrently. We should have a simple lock and take it when adding and removing elements from the list; the invocation is still performed under the Big Clutter Lock™, so it doesn't require special handling. --- clutter/clutter-main.c | 82 ++++++++++++++++++++++++++++++--------- clutter/clutter-private.h | 3 ++ 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c index 3a6bb1122..ef605de60 100644 --- a/clutter/clutter-main.c +++ b/clutter/clutter-main.c @@ -116,6 +116,7 @@ /* main context */ static ClutterMainContext *ClutterCntx = NULL; +G_LOCK_DEFINE_STATIC (ClutterCntx); /* main lock and locking/unlocking functions */ static GMutex *clutter_threads_mutex = NULL; @@ -1068,6 +1069,18 @@ clutter_get_debug_enabled (void) #endif } +void +_clutter_context_lock (void) +{ + G_LOCK (ClutterCntx); +} + +void +_clutter_context_unlock (void) +{ + G_UNLOCK (ClutterCntx); +} + gboolean _clutter_context_is_initialized (void) { @@ -1077,8 +1090,8 @@ _clutter_context_is_initialized (void) return ClutterCntx->is_initialized; } -ClutterMainContext * -_clutter_context_get_default (void) +static inline ClutterMainContext * +clutter_context_get_default_unlocked (void) { if (G_UNLIKELY (ClutterCntx == NULL)) { @@ -1096,11 +1109,27 @@ _clutter_context_get_default (void) ctx->timer = g_timer_new (); g_timer_start (ctx->timer); #endif + + ctx->last_repaint_id = 1; } return ClutterCntx; } +ClutterMainContext * +_clutter_context_get_default (void) +{ + ClutterMainContext *retval; + + _clutter_context_lock (); + + retval = clutter_context_get_default_unlocked (); + + _clutter_context_unlock (); + + return retval; +} + /** * clutter_get_timestamp: * @@ -1116,11 +1145,15 @@ clutter_get_timestamp (void) ClutterMainContext *ctx; gdouble seconds; - ctx = _clutter_context_get_default (); + _clutter_context_lock (); + + ctx = clutter_context_get_default_unlocked (); /* FIXME: may need a custom timer for embedded setups */ seconds = g_timer_elapsed (ctx->timer, NULL); + _clutter_context_unlock (); + return (gulong)(seconds / 1.0e-6); #else return 0; @@ -2817,7 +2850,9 @@ clutter_threads_remove_repaint_func (guint handle_id) g_return_if_fail (handle_id > 0); - context = _clutter_context_get_default (); + _clutter_context_lock (); + + context = clutter_context_get_default_unlocked (); l = context->repaint_funcs; while (l != NULL) { @@ -2835,11 +2870,13 @@ clutter_threads_remove_repaint_func (guint handle_id) g_slice_free (ClutterRepaintFunction, repaint_func); - return; + break; } l = l->next; } + + _clutter_context_unlock (); } /** @@ -2875,19 +2912,18 @@ clutter_threads_add_repaint_func (GSourceFunc func, gpointer data, GDestroyNotify notify) { - static guint repaint_id = 1; ClutterMainContext *context; ClutterRepaintFunction *repaint_func; g_return_val_if_fail (func != NULL, 0); - context = _clutter_context_get_default (); + _clutter_context_lock (); - /* XXX lock the context */ + context = clutter_context_get_default_unlocked (); repaint_func = g_slice_new (ClutterRepaintFunction); - repaint_func->id = repaint_id++; + repaint_func->id = context->last_repaint_id++; repaint_func->func = func; repaint_func->data = data; repaint_func->notify = notify; @@ -2895,7 +2931,7 @@ clutter_threads_add_repaint_func (GSourceFunc func, context->repaint_funcs = g_list_prepend (context->repaint_funcs, repaint_func); - /* XXX unlock the context */ + _clutter_context_unlock (); return repaint_func->id; } @@ -2914,23 +2950,26 @@ _clutter_run_repaint_functions (void) { ClutterMainContext *context = _clutter_context_get_default (); ClutterRepaintFunction *repaint_func; - GList *reinvoke_list, *l; + GList *invoke_list, *reinvoke_list, *l; if (context->repaint_funcs == NULL) return; + /* steal the list */ + invoke_list = context->repaint_funcs; + context->repaint_funcs = NULL; + reinvoke_list = NULL; /* consume the whole list while we execute the functions */ - while (context->repaint_funcs) + while (invoke_list != NULL) { gboolean res = FALSE; - repaint_func = context->repaint_funcs->data; + repaint_func = invoke_list->data; - l = context->repaint_funcs; - context->repaint_funcs = - g_list_remove_link (context->repaint_funcs, context->repaint_funcs); + l = invoke_list; + invoke_list = g_list_remove_link (invoke_list, invoke_list); g_list_free (l); @@ -2940,15 +2979,20 @@ _clutter_run_repaint_functions (void) reinvoke_list = g_list_prepend (reinvoke_list, repaint_func); else { - if (repaint_func->notify) + if (repaint_func->notify != NULL) repaint_func->notify (repaint_func->data); g_slice_free (ClutterRepaintFunction, repaint_func); } } - if (reinvoke_list) - context->repaint_funcs = reinvoke_list; + if (context->repaint_funcs != NULL) + { + context->repaint_funcs = g_list_concat (context->repaint_funcs, + g_list_reverse (reinvoke_list)); + } + else + context->repaint_funcs = g_list_reverse (reinvoke_list); } /** diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h index e9d0bd26b..556d26885 100644 --- a/clutter/clutter-private.h +++ b/clutter/clutter-private.h @@ -163,6 +163,7 @@ struct _ClutterMainContext * clutter_threads_add_repaint_func() */ GList *repaint_funcs; + guint last_repaint_id; /* main settings singleton */ ClutterSettings *settings; @@ -186,6 +187,8 @@ gboolean _clutter_threads_dispatch (gpointer data); void _clutter_threads_dispatch_free (gpointer data); ClutterMainContext * _clutter_context_get_default (void); +void _clutter_context_lock (void); +void _clutter_context_unlock (void); gboolean _clutter_context_is_initialized (void); PangoContext * _clutter_context_create_pango_context (void); PangoContext * _clutter_context_get_pango_context (void);