From a545f66a5c34b66d1a31387105d114ca3edd3a46 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 21 Jan 2010 23:41:18 +0000 Subject: [PATCH] master clock: Improve the timeline advancement protection The commit 1c69c61745ed510f0b6ab16cb963ca01994cb9fc which improved the protection against timeline removals during the master clock advancement was only doing half the job - and actually broke the chaining of animations inside the ::completed signal. We cannot simply take a reference on the timelines and still use the list held by the master clock because the do_tick() might result in the creation of a new timeline, which gets added at the end of the list with no reference increase and thus gets disposed at the end of the iteration. We also cannot steal the master clock timelines list because a timeline might be removed as the direct result of do_tick() and remove_timeline() would not find the timeline, failing and leaving a dangling pointer behind. For this reason we copy the list of timelines out of the one that the Master Clock holds, take a reference on each timeline, advance them all, release the reference and free the list. --- clutter/clutter-master-clock.c | 41 +++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index b61ad436c..ca4f9d95a 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -446,7 +446,7 @@ _clutter_master_clock_start_running (ClutterMasterClock *master_clock) void _clutter_master_clock_advance (ClutterMasterClock *master_clock) { - GSList *l, *next; + GSList *timelines, *l; CLUTTER_STATIC_TIMER (master_timeline_advance, "Master Clock", @@ -456,23 +456,38 @@ _clutter_master_clock_advance (ClutterMasterClock *master_clock) g_return_if_fail (CLUTTER_IS_MASTER_CLOCK (master_clock)); - /* we protect ourselves from timelines being removed during - * the advancement by other timelines - */ - g_slist_foreach (master_clock->timelines, (GFunc) g_object_ref, NULL); - CLUTTER_TIMER_START (_clutter_uprof_context, master_timeline_advance); - for (l = master_clock->timelines; l != NULL; l = next) - { - next = l->next; + /* we protect ourselves from timelines being removed during + * the advancement by other timelines by copying the list of + * timelines, taking a reference on them, iterating over the + * copied list and then releasing the reference. + * + * we cannot simply take a reference on the timelines and still + * use the list held by the master clock because the do_tick() + * might result in the creation of a new timeline, which gets + * added at the end of the list with no reference increase and + * thus gets disposed at the end of the iteration. + * + * this implies that a newly added timeline will not be advanced + * by this clock iteration, which is perfectly fine since we're + * in its first cycle. + * + * we also cannot steal the master clock timelines list because + * a timeline might be removed as the direct result of do_tick() + * and remove_timeline() would not find the timeline, failing + * and leaving a dangling pointer behind. + */ + timelines = g_slist_copy (master_clock->timelines); + g_slist_foreach (timelines, (GFunc) g_object_ref, NULL); - clutter_timeline_do_tick (l->data, &master_clock->cur_tick); - } + for (l = timelines; l != NULL; l = l->next) + clutter_timeline_do_tick (l->data, &master_clock->cur_tick); + + g_slist_foreach (timelines, (GFunc) g_object_unref, NULL); + g_slist_free (timelines); CLUTTER_TIMER_STOP (_clutter_uprof_context, master_timeline_advance); - - g_slist_foreach (master_clock->timelines, (GFunc) g_object_unref, NULL); } /**