From 323df2b2800383832ed3c2e43626f2c6821c33ec Mon Sep 17 00:00:00 2001 From: William Jon McCann Date: Sat, 20 Nov 2010 17:58:50 -0500 Subject: [PATCH] Make gdk_event_apply_filters safe against changes in filter list An event filter may add or remove filters itself. This patch does two things to address this case. The first is to take a temporary reference to the filter while it is being used. The second is to wait until after the filter function is run before determining the next node in the list to process. This guards against changes to the next node. It also does not run functions that have been marked as removed. Though I'm not sure if this case can arise. https://bugzilla.gnome.org/show_bug.cgi?id=635380 --- gdk/gdkinternals.h | 6 ++++++ gdk/gdkwindow.c | 12 +++++++++++- gdk/quartz/gdkevents-quartz.c | 33 +++++++++++++++++++++++++++------ gdk/win32/gdkevents-win32.c | 33 +++++++++++++++++++++++++++------ gdk/x11/gdkeventsource.c | 30 +++++++++++++++++++++++++----- 5 files changed, 96 insertions(+), 18 deletions(-) diff --git a/gdk/gdkinternals.h b/gdk/gdkinternals.h index e1b458e803..c425447871 100644 --- a/gdk/gdkinternals.h +++ b/gdk/gdkinternals.h @@ -59,9 +59,15 @@ struct _GdkColorInfo guint ref_count; }; +typedef enum { + GDK_EVENT_FILTER_REMOVED = 1 << 0 +} GdkEventFilterFlags; + struct _GdkEventFilter { GdkFilterFunc function; gpointer data; + GdkEventFilterFlags flags; + guint ref_count; }; struct _GdkClientFilter { diff --git a/gdk/gdkwindow.c b/gdk/gdkwindow.c index c2485980f5..87d9d55abe 100644 --- a/gdk/gdkwindow.c +++ b/gdk/gdkwindow.c @@ -2577,13 +2577,18 @@ gdk_window_add_filter (GdkWindow *window, { filter = (GdkEventFilter *)tmp_list->data; if ((filter->function == function) && (filter->data == data)) - return; + { + filter->ref_count++; + return; + } tmp_list = tmp_list->next; } filter = g_new (GdkEventFilter, 1); filter->function = function; filter->data = data; + filter->ref_count = 1; + filter->flags = 0; if (private) private->filters = g_list_append (private->filters, filter); @@ -2626,6 +2631,11 @@ gdk_window_remove_filter (GdkWindow *window, if ((filter->function == function) && (filter->data == data)) { + filter->flags |= GDK_EVENT_FILTER_REMOVED; + filter->ref_count--; + if (filter->ref_count != 0) + return; + if (private) private->filters = g_list_remove_link (private->filters, node); else diff --git a/gdk/quartz/gdkevents-quartz.c b/gdk/quartz/gdkevents-quartz.c index 2f8b0c8c1b..c6512fc4db 100644 --- a/gdk/quartz/gdkevents-quartz.c +++ b/gdk/quartz/gdkevents-quartz.c @@ -171,19 +171,40 @@ append_event (GdkEvent *event, static gint gdk_event_apply_filters (NSEvent *nsevent, GdkEvent *event, - GList *filters) + GList **filters) { GList *tmp_list; GdkFilterReturn result; - tmp_list = filters; + tmp_list = *filters; while (tmp_list) { GdkEventFilter *filter = (GdkEventFilter*) tmp_list->data; - - tmp_list = tmp_list->next; + GList *node; + + if ((filter->flags & GDK_EVENT_FILTER_REMOVED) != 0) + { + tmp_list = tmp_list->next; + continue; + } + + filter->ref_count++; result = filter->function (nsevent, event, filter->data); + + /* get the next node after running the function since the + function may add or remove a next node */ + node = tmp_list; + tmp_list = tmp_list->next; + + filter->ref_count--; + if (filter->ref_count == 0) + { + *filters = g_list_remove_link (*filters, node); + g_list_free_1 (node); + g_free (filter); + } + if (result != GDK_FILTER_CONTINUE) return result; } @@ -1165,7 +1186,7 @@ gdk_event_translate (GdkEvent *event, /* Apply global filters */ GdkFilterReturn result; - result = gdk_event_apply_filters (nsevent, event, _gdk_default_filters); + result = gdk_event_apply_filters (nsevent, event, &_gdk_default_filters); if (result != GDK_FILTER_CONTINUE) { return_val = (result == GDK_FILTER_TRANSLATE) ? TRUE : FALSE; @@ -1206,7 +1227,7 @@ gdk_event_translate (GdkEvent *event, { g_object_ref (window); - result = gdk_event_apply_filters (nsevent, event, filter_private->filters); + result = gdk_event_apply_filters (nsevent, event, &filter_private->filters); g_object_unref (window); diff --git a/gdk/win32/gdkevents-win32.c b/gdk/win32/gdkevents-win32.c index fb8a7d04c7..bea28a8b7e 100644 --- a/gdk/win32/gdkevents-win32.c +++ b/gdk/win32/gdkevents-win32.c @@ -1025,7 +1025,7 @@ fill_key_event_string (GdkEvent *event) static GdkFilterReturn apply_event_filters (GdkWindow *window, MSG *msg, - GList *filters) + GList **filters) { GdkFilterReturn result = GDK_FILTER_CONTINUE; GdkEvent *event; @@ -1043,13 +1043,34 @@ apply_event_filters (GdkWindow *window, */ node = _gdk_event_queue_append (_gdk_display, event); - tmp_list = filters; + tmp_list = *filters; while (tmp_list) { GdkEventFilter *filter = (GdkEventFilter *) tmp_list->data; - - tmp_list = tmp_list->next; + GList *node; + + if ((filter->flags & GDK_EVENT_FILTER_REMOVED) != 0) + { + tmp_list = tmp_list->next; + continue; + } + + filter->ref_count++; result = filter->function (msg, event, filter->data); + + /* get the next node after running the function since the + function may add or remove a next node */ + node = tmp_list; + tmp_list = tmp_list->next; + + filter->ref_count--; + if (filter->ref_count == 0) + { + *filters = g_list_remove_link (*filters, node); + g_list_free_1 (node); + g_free (filter); + } + if (result != GDK_FILTER_CONTINUE) break; } @@ -1756,7 +1777,7 @@ gdk_event_translate (MSG *msg, { /* Apply global filters */ - GdkFilterReturn result = apply_event_filters (NULL, msg, _gdk_default_filters); + GdkFilterReturn result = apply_event_filters (NULL, msg, &_gdk_default_filters); /* If result is GDK_FILTER_CONTINUE, we continue as if nothing * happened. If it is GDK_FILTER_REMOVE or GDK_FILTER_TRANSLATE, @@ -1822,7 +1843,7 @@ gdk_event_translate (MSG *msg, { /* Apply per-window filters */ - GdkFilterReturn result = apply_event_filters (window, msg, ((GdkWindowObject *) window)->filters); + GdkFilterReturn result = apply_event_filters (window, msg, &((GdkWindowObject *) window)->filters); if (result == GDK_FILTER_REMOVE || result == GDK_FILTER_TRANSLATE) { diff --git a/gdk/x11/gdkeventsource.c b/gdk/x11/gdkeventsource.c index f3b850e417..7a23b59b2c 100644 --- a/gdk/x11/gdkeventsource.c +++ b/gdk/x11/gdkeventsource.c @@ -57,20 +57,40 @@ static GList *event_sources = NULL; static gint gdk_event_apply_filters (XEvent *xevent, GdkEvent *event, - GList *filters) + GList **filters) { GList *tmp_list; GdkFilterReturn result; - tmp_list = filters; + tmp_list = *filters; while (tmp_list) { GdkEventFilter *filter = (GdkEventFilter*) tmp_list->data; + GList *node; - tmp_list = tmp_list->next; + if ((filter->flags & GDK_EVENT_FILTER_REMOVED) != 0) + { + tmp_list = tmp_list->next; + continue; + } + + filter->ref_count++; result = filter->function (xevent, event, filter->data); + /* get the next node after running the function since the + function may add or remove a next node */ + node = tmp_list; + tmp_list = tmp_list->next; + + filter->ref_count--; + if (filter->ref_count == 0) + { + *filters = g_list_remove_link (*filters, node); + g_list_free_1 (node); + g_free (filter); + } + if (result != GDK_FILTER_CONTINUE) return result; } @@ -143,7 +163,7 @@ gdk_event_source_translate_event (GdkEventSource *event_source, /* Apply global filters */ result = gdk_event_apply_filters (xevent, event, - _gdk_default_filters); + &_gdk_default_filters); if (result == GDK_FILTER_REMOVE) { @@ -167,7 +187,7 @@ gdk_event_source_translate_event (GdkEventSource *event_source, if (filter_private->filters) { result = gdk_event_apply_filters (xevent, event, - filter_private->filters); + &filter_private->filters); if (result == GDK_FILTER_REMOVE) {