Improve tests for X error traps, fix two bugs

* don't lose track of previous X error handler
  if nested traps are pushed
* free any remaining traps when display
  is finalized

Test will fail unless bug 630032 is closed so
gdk_display_close() works.

https://bugzilla.gnome.org/show_bug.cgi?id=630033

diff --git a/gdk/x11/gdkdisplay-x11.c b/gdk/x11/gdkdisplay-x11.c
index ea4e9914dc5e97693de15d78414e90e70c53b196..3b82590e3afb8ff5c85703cbd8b1d7a2d623f776 100644 (file)
--- a/gdk/x11/gdkdisplay-x11.c
+++ b/gdk/x11/gdkdisplay-x11.c
@@ -1911,6 +1911,21 @@ gdk_display_x11_finalize (GObject *object)
 
   XCloseDisplay (display_x11->xdisplay);
 
+  /* error traps */
+  while (display_x11->error_traps != NULL)
+    {
+      GdkErrorTrap *trap = display_x11->error_traps->data;
+
+      display_x11->error_traps =
+        g_slist_delete_link (display_x11->error_traps,
+                             display_x11->error_traps);
+
+      if (trap->end_sequence == 0)
+        g_warning ("Display finalized with an unpopped error trap");
+
+      g_slice_free (GdkErrorTrap, trap);
+    }
+
   G_OBJECT_CLASS (_gdk_display_x11_parent_class)->finalize (object);
 }
 

diff --git a/gdk/x11/gdkmain-x11.c b/gdk/x11/gdkmain-x11.c
index 996715c75fce572b9726dec9a2166d785b147aba..ca20f973045f0474008982da5a262406d4ee9667 100644 (file)
--- a/gdk/x11/gdkmain-x11.c
+++ b/gdk/x11/gdkmain-x11.c
@@ -62,7 +62,8 @@ struct _GdkPredicate
 };
 
 /* non-GDK previous error handler */
-static int (*_gdk_old_error_handler) (Display *, XErrorEvent *);
+typedef int (*GdkXErrorHandler) (Display *, XErrorEvent *);
+static GdkXErrorHandler _gdk_old_error_handler;
 /* number of times we've pushed the GDK error handler */
 static int _gdk_error_handler_push_count = 0;
 
@@ -386,13 +387,19 @@ gdk_x_error (Display       *xdisplay,
 void
 _gdk_x11_error_handler_push (void)
 {
-  _gdk_old_error_handler = XSetErrorHandler (gdk_x_error);
+  GdkXErrorHandler previous;
+
+  previous = XSetErrorHandler (gdk_x_error);
 
   if (_gdk_error_handler_push_count > 0)
     {
-      if (_gdk_old_error_handler != gdk_x_error)
+      if (previous != gdk_x_error)
         g_warning ("XSetErrorHandler() called with a GDK error trap pushed. Don't do that.");
     }
+  else
+    {
+      _gdk_old_error_handler = previous;
+    }
 
   _gdk_error_handler_push_count += 1;
 }

diff --git a/tests/testerrors.c b/tests/testerrors.c
index 63b98cc772b82e569c77430dee6da3d7b0ba997e..b8d1a8a812653fd365c31cc50363536143b563e3 100755 (executable)
--- a/tests/testerrors.c
+++ b/tests/testerrors.c
@@ -23,25 +23,23 @@
 #include <gtk/gtk.h>
 #include "x11/gdkx.h"
 
-gint
-main (gint argc, gchar *argv[])
+static void
+test_error_trapping (GdkDisplay *gdk_display)
 {
   Display *d;
   int dummy;
   int error;
 
-  gtk_init (&argc, &argv);
-
-  d = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
+  d = GDK_DISPLAY_XDISPLAY (gdk_display);
 
   /* verify that we can catch errors */
   gdk_error_trap_push ();
-  XListProperties (d, 0, &dummy);
+  XListProperties (d, 0, &dummy); /* round trip */
   error = gdk_error_trap_pop ();
   g_assert (error == BadWindow);
 
   gdk_error_trap_push ();
-  XSetCloseDownMode (d, 12345);
+  XSetCloseDownMode (d, 12345); /* not a round trip */
   XSetCloseDownMode (d, DestroyAll);
   error = gdk_error_trap_pop ();
   g_assert (error == BadValue);
@@ -58,6 +56,104 @@ main (gint argc, gchar *argv[])
 
   XSync (d, TRUE);
 
+  /* verify that we can catch with nested traps */
+  gdk_error_trap_push ();
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  error = gdk_error_trap_pop ();
+  g_assert (error == BadValue);
+  error = gdk_error_trap_pop ();
+  g_assert (error == BadValue);
+
+  /* try nested, without sync */
+  gdk_error_trap_push ();
+  gdk_error_trap_push ();
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_pop_ignored ();
+  gdk_error_trap_pop_ignored ();
+  gdk_error_trap_pop_ignored ();
+
+  XSync (d, TRUE);
+
+  /* try nested, without sync, with interleaved calls */
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_pop_ignored ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_pop_ignored ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_pop_ignored ();
+
+  XSync (d, TRUE);
+
+  /* don't want to get errors that weren't in our push range */
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  gdk_error_trap_push ();
+  XSync (d, TRUE); /* not an error */
+  error = gdk_error_trap_pop ();
+  g_assert (error == Success);
+  error = gdk_error_trap_pop ();
+  g_assert (error == BadValue);
+
+  /* non-roundtrip non-error request after error request, inside trap */
+  gdk_error_trap_push ();
+  XSetCloseDownMode (d, 12345);
+  XMapWindow (d, DefaultRootWindow (d));
+  error = gdk_error_trap_pop ();
+  g_assert (error == BadValue);
+
+  /* a non-roundtrip non-error request before error request, inside trap */
+  gdk_error_trap_push ();
+  XMapWindow (d, DefaultRootWindow (d));
+  XSetCloseDownMode (d, 12345);
+  error = gdk_error_trap_pop ();
+  g_assert (error == BadValue);
+
+  /* Not part of any test, just a double-check
+   * that all errors have arrived
+   */
+  XSync (d, TRUE);
+}
+
+gint
+main (gint argc, gchar *argv[])
+{
+  GdkDisplay *extra_display;
+
+  gtk_init (&argc, &argv);
+
+  test_error_trapping (gdk_display_get_default ());
+
+  extra_display = gdk_display_open (NULL);
+  test_error_trapping (extra_display);
+  gdk_display_close (extra_display);
+
+  test_error_trapping (gdk_display_get_default ());
+
+  /* open a display with a trap pushed and see if we
+   * get confused
+   */
+  gdk_error_trap_push ();
+  gdk_error_trap_push ();
+
+  extra_display = gdk_display_open (NULL);
+  test_error_trapping (extra_display);
+  gdk_display_close (extra_display);
+
+  gdk_error_trap_pop_ignored ();
+  gdk_error_trap_pop_ignored ();
+
+  test_error_trapping (gdk_display_get_default ());
+
+  /* reassure us that the tests ran. */
+  g_print("All errors properly trapped.\n");
+
   return 0;
 }