Fix gdb.mi/mi-nsmoribund.exp timeouts

The PPC64 buildbot has been showing timeouts in mi-nsmoribund.exp,
like this:

 (...)
 -thread-info
 FAIL: gdb.mi/mi-nsmoribund.exp: thread state: all running except the breakpoint thread (timeout)

... and I can reproduce this on gcc110 (PPC64) on the gcc compile
farm.

That is, the test sends "-thread-info" to GDB, but GDB never replies
back.

The problem is that these machines are too fast for gdb.  :-)

That test has a few threads running the same tight loop, and
constantly hitting a thread-specific breakpoint that needs to be
stepped over.  If threads trip on breakpoints fast enough that
linux-nat.c's event pipe associated with SIGCHLD is constantly being
written to, even if the stdin file descriptor also has an event to
handle, gdb never gets to it. because linux-nat.c's pipe comes first
in the set of descriptors served by the poll/select code in the event
loop.

Fix this by having the event loop serve file event sources in
round-robin-like fashion, similarly to how its done in
gdb_do_one_event.

Unfortunately, the poll and the select variants each need their own
fixing.

Tested on x86_64 Fedora 20 (poll and select variants), and PPC64
Fedora 18.  Fixes the timeout in the PPC64 machine in the compile farm
that times out without this, and I won't be surprised if it fixes
other random timeouts in other tests.

(gdbserver's copy of the event-loop doesn't need this (yet), as it
still pushes all ready events to an event queue.  That is, it hasn't
had 70b66289 merged yet.  We should really merge both event-loop.c
copies into a single shared file, but that's for another day.)

gdb/ChangeLog:
2015-05-15  Pedro Alves  <palves@redhat.com>
	    Simon Marchi  <simon.marchi@ericsson.com>

	* event-loop.c (gdb_notifier) <next_file_handler,
	next_poll_fds_index>: New fields.
	(get_next_file_handler_to_handle_and_advance): New function.
	(delete_file_handler): If deleting the next file handler to
	handle, advance to the next file handler.
	(gdb_wait_for_event): Bail early if no event fired.  Poll file
	handlers in round-robin fashion.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 05c0b3f08510794cae8c73b4048290d53fbd5703..b7694c769c0ba7c2e2160835580354d7b9d0d800 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2015-05-15  Pedro Alves  <palves@redhat.com>
+           Simon Marchi  <simon.marchi@ericsson.com>
+
+       * event-loop.c (gdb_notifier) <next_file_handler,
+       next_poll_fds_index>: New fields.
+       (get_next_file_handler_to_handle_and_advance): New function.
+       (delete_file_handler): If deleting the next file handler to
+       handle, advance to the next file handler.
+       (gdb_wait_for_event): Bail early if no event fired.  Poll file
+       handlers in round-robin fashion.
+
 2015-05-15  Pedro Alves  <palves@redhat.com>
 
        * linux-tdep.c (linux_find_memory_regions_full): Rename local

diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 79e41fd549ea4215265298c0a781dd6ff130aafc..9ac49086f0c33fab4880a76c6773bb27d4c70e1c 100644 (file)
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -165,10 +165,24 @@ static struct
     /* Ptr to head of file handler list.  */
     file_handler *first_file_handler;
 
+    /* Next file handler to handle, for the select variant.  To level
+       the fairness across event sources, we serve file handlers in a
+       round-robin-like fashion.  The number and order of the polled
+       file handlers may change between invocations, but this is good
+       enough.  */
+    file_handler *next_file_handler;
+
 #ifdef HAVE_POLL
     /* Ptr to array of pollfd structures.  */
     struct pollfd *poll_fds;
 
+    /* Next file descriptor to handle, for the poll variant.  To level
+       the fairness across event sources, we poll the file descriptors
+       in a round-robin-like fashion.  The number and order of the
+       polled file descriptors may change between invocations, but
+       this is good enough.  */
+    int next_poll_fds_index;
+
     /* Timeout in milliseconds for calls to poll().  */
     int poll_timeout;
 #endif
@@ -494,6 +508,31 @@ create_file_handler (int fd, int mask, handler_func * proc,
   file_ptr->mask = mask;
 }
 
+/* Return the next file handler to handle, and advance to the next
+   file handler, wrapping around if the end of the list is
+   reached.  */
+
+static file_handler *
+get_next_file_handler_to_handle_and_advance (void)
+{
+  file_handler *curr_next;
+
+  /* The first time around, this is still NULL.  */
+  if (gdb_notifier.next_file_handler == NULL)
+    gdb_notifier.next_file_handler = gdb_notifier.first_file_handler;
+
+  curr_next = gdb_notifier.next_file_handler;
+  gdb_assert (curr_next != NULL);
+
+  /* Advance.  */
+  gdb_notifier.next_file_handler = curr_next->next_file;
+  /* Wrap around, if necessary.  */
+  if (gdb_notifier.next_file_handler == NULL)
+    gdb_notifier.next_file_handler = gdb_notifier.first_file_handler;
+
+  return curr_next;
+}
+
 /* Remove the file descriptor FD from the list of monitored fd's: 
    i.e. we don't care anymore about events on the FD.  */
 void
@@ -576,6 +615,17 @@ delete_file_handler (int fd)
 
   file_ptr->mask = 0;
 
+  /* If this file handler was going to be the next one to be handled,
+     advance to the next's next, if any.  */
+  if (gdb_notifier.next_file_handler == file_ptr)
+    {
+      if (file_ptr->next_file == NULL
+         && file_ptr == gdb_notifier.first_file_handler)
+       gdb_notifier.next_file_handler = NULL;
+      else
+       get_next_file_handler_to_handle_and_advance ();
+    }
+
   /* Get rid of the file handler in the file handler list.  */
   if (file_ptr == gdb_notifier.first_file_handler)
     gdb_notifier.first_file_handler = file_ptr->next_file;
@@ -672,7 +722,6 @@ gdb_wait_for_event (int block)
 {
   file_handler *file_ptr;
   int num_found = 0;
-  int i;
 
   /* Make sure all output is done before getting another event.  */
   gdb_flush (gdb_stdout);
@@ -743,37 +792,47 @@ gdb_wait_for_event (int block)
        }
     }
 
+  /* Avoid looking at poll_fds[i]->revents if no event fired.  */
+  if (num_found <= 0)
+    return 0;
+
   /* Run event handlers.  We always run just one handler and go back
      to polling, in case a handler changes the notifier list.  Since
      events for sources we haven't consumed yet wake poll/select
      immediately, no event is lost.  */
 
+  /* To level the fairness across event descriptors, we handle them in
+     a round-robin-like fashion.  The number and order of descriptors
+     may change between invocations, but this is good enough.  */
   if (use_poll)
     {
 #ifdef HAVE_POLL
-      for (i = 0; (i < gdb_notifier.num_fds) && (num_found > 0); i++)
-       {
-         if ((gdb_notifier.poll_fds + i)->revents)
-           num_found--;
-         else
-           continue;
+      int i;
+      int mask;
 
-         for (file_ptr = gdb_notifier.first_file_handler;
-              file_ptr != NULL;
-              file_ptr = file_ptr->next_file)
-           {
-             if (file_ptr->fd == (gdb_notifier.poll_fds + i)->fd)
-               break;
-           }
+      while (1)
+       {
+         if (gdb_notifier.next_poll_fds_index >= gdb_notifier.num_fds)
+           gdb_notifier.next_poll_fds_index = 0;
+         i = gdb_notifier.next_poll_fds_index++;
 
-         if (file_ptr)
-           {
-             int mask = (gdb_notifier.poll_fds + i)->revents;
+         gdb_assert (i < gdb_notifier.num_fds);
+         if ((gdb_notifier.poll_fds + i)->revents)
+           break;
+       }
 
-             handle_file_event (file_ptr, mask);
-             return 1;
-           }
+      for (file_ptr = gdb_notifier.first_file_handler;
+          file_ptr != NULL;
+          file_ptr = file_ptr->next_file)
+       {
+         if (file_ptr->fd == (gdb_notifier.poll_fds + i)->fd)
+           break;
        }
+      gdb_assert (file_ptr != NULL);
+
+      mask = (gdb_notifier.poll_fds + i)->revents;
+      handle_file_event (file_ptr, mask);
+      return 1;
 #else
       internal_error (__FILE__, __LINE__,
                      _("use_poll without HAVE_POLL"));
@@ -781,11 +840,12 @@ gdb_wait_for_event (int block)
     }
   else
     {
-      for (file_ptr = gdb_notifier.first_file_handler;
-          (file_ptr != NULL) && (num_found > 0);
-          file_ptr = file_ptr->next_file)
+      /* See comment about even source fairness above.  */
+      int mask = 0;
+
+      do
        {
-         int mask = 0;
+         file_ptr = get_next_file_handler_to_handle_and_advance ();
 
          if (FD_ISSET (file_ptr->fd, &gdb_notifier.ready_masks[0]))
            mask |= GDB_READABLE;
@@ -793,15 +853,11 @@ gdb_wait_for_event (int block)
            mask |= GDB_WRITABLE;
          if (FD_ISSET (file_ptr->fd, &gdb_notifier.ready_masks[2]))
            mask |= GDB_EXCEPTION;
-
-         if (!mask)
-           continue;
-         else
-           num_found--;
-
-         handle_file_event (file_ptr, mask);
-         return 1;
        }
+      while (mask == 0);
+
+      handle_file_event (file_ptr, mask);
+      return 1;
     }
   return 0;
 }