Fix race condition in search command (#335)

* Add a test for search race condition

When window disappear while search is trying to access it (either check
its properties, or enumerate its children), X server will
generate BadWindow error. Try to trigger this race, by opening 100
short living windows and execute xdotool search at the same time. The
search should not crash, and since the window subject to search is there
all the time, it should be found.

The test relies on a specific siming, specifically that launching 100
instance of xterm takes more than 100ms, and that going from setup to
the actual test takes less than 100ms. Those conditions are rather
reasonable, and the test works reliably for me (at this stage - it fails
- the fix is comming in the next commit).

Related to #60

* Fix race condition in search command

When a window is destroyed while search command tries to inspect it (get
its title, class or so), the X server generates BadWindow error. The
default error handler terminates the program. This means the search can
fail if any window is destroyed in the meantime, regardless if the
window matches the search criteria or not.

Fix this by setting an alternative error handler for the search time,
that ignores BadWindow error. This makes the relevant libX11 functions
return (with a negative status) in case of BadWindow error, instead of
interrupting the whole xdotool process.

While at it, fix uninitialized 'children' variable in XQueryTree error
handling.

Fix #60

Author: marmarek

t/test_search.rb

@@ -153,3 +153,26 @@ def test_search_can_find_all_windows
     end # ["name" ... ].each 
   end # def test_search_can_find_all_windows
 end # XdotoolSearchTests
+
+class XdotoolRaceSearchTests < MiniTest::Test
+  include XdoTestHelper
+
+  def setup
+    setup_vars
+    setup_ensure_x_is_healthy
+    setup_launch_xterm
+    setup_launch("sh", "-c", "for i in $(seq 100); do xterm -e sleep 0.1 & true; done")
+  end # def setup
+
+  def test_search_title
+    (1 .. 100).each do
+      status, lines = xdotool "search --name #{@title}"
+      assert_equal(0, status, "Exit status should have been 0")
+      assert_equal(1, lines.size,
+                   "Expect only one match to our search (only one window " \
+                   "running that should match)")
+      assert_equal(@wid, lines[0].to_i,
+                   "Expected the correct windowid when searching for its pid")
+    end
+  end
+end # XdotoolRaceSearchTests

xdo_search.c

@@ -364,6 +364,27 @@ static int check_window_match(const xdo_t *xdo, Window wid,
   return False;
 } /* int check_window_match */
 
+static int ignore_badwindow(Display *dpy, XErrorEvent *xerr) {
+#define ERROR_BUF_SIZE 256
+  char buf[ERROR_BUF_SIZE];
+  char request[ERROR_BUF_SIZE];
+
+  if (xerr->error_code == BadWindow)
+    return 0;
+
+  XGetErrorText(dpy, xerr->error_code, buf, sizeof(buf));
+  fprintf(stderr, "X Error of failed request: %s\n", buf);
+
+  /* Intentionally ignore errors from protocol extensions, they are not used in
+   * this file. */
+  snprintf(request, sizeof(request), "%d", xerr->request_code);
+  XGetErrorDatabaseText(dpy, "XRequest", request, "", buf,
+      sizeof(buf));
+  fprintf(stderr, "Major opcode: %d (%s)\n",
+      xerr->request_code, buf);
+  exit(1);
+}
+
 static void find_matching_windows(const xdo_t *xdo, Window window, 
                                   const xdo_search_t *search,
                                   Window **windowlist_ret,
@@ -379,8 +400,9 @@ static void find_matching_windows(const xdo_t *xdo, Window window,
    */
 
   Window dummy;
-  Window *children;
+  Window *children = NULL;
   unsigned int i, nchildren;
+  int (*old_error_handler)(Display *dpy, XErrorEvent *xerr);
 
   /* Break early, if we have enough windows already. */
   if (search->limit > 0 && *nwindows_ret >= search->limit) {
@@ -392,6 +414,9 @@ static void find_matching_windows(const xdo_t *xdo, Window window,
     return;
   }
 
+  /* Don't crash if window dissappear in the meantime */
+  old_error_handler = XSetErrorHandler(ignore_badwindow);
+
   /* Break if XQueryTree fails.
    * TODO(sissel): report an error? */
   Status success = XQueryTree(xdo->xdpy, window, &dummy, &dummy, &children, &nchildren);
@@ -422,6 +447,7 @@ static void find_matching_windows(const xdo_t *xdo, Window window,
                                 *windowlist_size * sizeof(Window));
     }
   } /* for (i in children) ... */
+  XSetErrorHandler(old_error_handler);
 
   /* Now check children-children */
   if (search->max_depth == -1 || (current_depth + 1) <= search->max_depth) {