util: fix a reentrancy problem with meta_later

Calling meta_later_add() or meta_later_remove() from within a
META_LATER_BEFORE_REDRAW callback ended up being a no-op, because of
how run_repaint_laters() was fiddling with the laters list. (This
resulted in a crash in window.c:idle_calc_repaint(), which assumed it
would only be called when a certain queue was non-empty, but was
getting called anyway because of a failed meta_later_remove() call.)

Fix this by having run_repaint_laters() work on a copy of the laters
list instead, and add refcounting to MetaLater so that removing a
later that run_repaint_laters() hasn't gotten to yet won't cause
problems.

https://bugzilla.gnome.org/show_bug.cgi?id=642957
This commit is contained in:
Dan Winship 2011-03-29 15:18:57 -04:00
parent 12f71c9795
commit cd7a968093

View File

@ -708,6 +708,7 @@ static guint last_later_id = 0;
typedef struct typedef struct
{ {
guint id; guint id;
guint ref_count;
MetaLaterType when; MetaLaterType when;
GSourceFunc func; GSourceFunc func;
gpointer data; gpointer data;
@ -723,14 +724,30 @@ static guint later_repaint_func = 0;
static void ensure_later_repaint_func (void); static void ensure_later_repaint_func (void);
static void
unref_later (MetaLater *later)
{
if (--later->ref_count == 0)
{
if (later->notify)
{
later->notify (later->data);
later->notify = NULL;
}
g_slice_free (MetaLater, later);
}
}
static void static void
destroy_later (MetaLater *later) destroy_later (MetaLater *later)
{ {
if (later->source) if (later->source)
{
g_source_remove (later->source); g_source_remove (later->source);
if (later->notify) later->source = 0;
later->notify (later->data); }
g_slice_free (MetaLater, later); later->func = NULL;
unref_later (later);
} }
/* Used to sort the list of laters with the highest priority /* Used to sort the list of laters with the highest priority
@ -746,34 +763,41 @@ compare_laters (gconstpointer a,
static gboolean static gboolean
run_repaint_laters (gpointer data) run_repaint_laters (gpointer data)
{ {
GSList *old_laters = laters; GSList *laters_copy;
GSList *l; GSList *l;
gboolean keep_timeline_running = FALSE; gboolean keep_timeline_running = FALSE;
laters = NULL;
for (l = old_laters; l; l = l->next) laters_copy = NULL;
for (l = laters; l; l = l->next)
{ {
MetaLater *later = l->data; MetaLater *later = l->data;
if (later->source == 0 || if (later->source == 0 ||
(later->when <= META_LATER_BEFORE_REDRAW && !later->run_once)) (later->when <= META_LATER_BEFORE_REDRAW && !later->run_once))
{ {
if (later->func (later->data)) later->ref_count++;
laters_copy = g_slist_prepend (laters_copy, later);
}
}
laters_copy = g_slist_reverse (laters_copy);
for (l = laters_copy; l; l = l->next)
{
MetaLater *later = l->data;
if (later->func && later->func (later->data))
{ {
if (later->source == 0) if (later->source == 0)
keep_timeline_running = TRUE; keep_timeline_running = TRUE;
laters = g_slist_insert_sorted (laters, later, compare_laters);
} }
else else
destroy_later (later); meta_later_remove (later->id);
} unref_later (later);
else
laters = g_slist_insert_sorted (laters, later, compare_laters);
} }
if (!keep_timeline_running) if (!keep_timeline_running)
clutter_timeline_stop (later_timeline); clutter_timeline_stop (later_timeline);
g_slist_free (old_laters); g_slist_free (laters_copy);
/* Just keep the repaint func around - it's cheap if the list is empty */ /* Just keep the repaint func around - it's cheap if the list is empty */
return TRUE; return TRUE;
@ -800,9 +824,7 @@ call_idle_later (gpointer data)
if (!later->func (later->data)) if (!later->func (later->data))
{ {
laters = g_slist_remove (laters, later); meta_later_remove (later->id);
later->source = 0;
destroy_later (later);
return FALSE; return FALSE;
} }
else else
@ -838,6 +860,7 @@ meta_later_add (MetaLaterType when,
MetaLater *later = g_slice_new0 (MetaLater); MetaLater *later = g_slice_new0 (MetaLater);
later->id = ++last_later_id; later->id = ++last_later_id;
later->ref_count = 1;
later->when = when; later->when = when;
later->func = func; later->func = func;
later->data = data; later->data = data;