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

diff --git a/gdk/gdkinternals.h b/gdk/gdkinternals.h
index e1b458e8038f4a240f0e5571024ce9313063a376..c425447871db18c59436addd710aaf33c11c8feb 100644 (file)
--- 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 c2485980f5fc3173a3057133090e57664ed3e173..87d9d55abe2f1ed4cf0f9f54b6d82452cbc9ea9f 100644 (file)
--- 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 2f8b0c8c1bc4b25b221e6bf03235f95990c96b46..c6512fc4db9977708665906471d0d6ad276f1bbe 100644 (file)
--- 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 fb8a7d04c715f326903d80bda89f24be40de3c48..bea28a8b7e3e36a0a8ec6be3f7a100ee53dbdd74 100644 (file)
--- 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 f3b850e417388db09d63a99b201923e636669679..7a23b59b2c3722357b5e01d98a5f54b2f02cce99 100644 (file)
--- 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)
             {