[binutils-gdb] Fix gdb.mi/mi-nsmoribund.exp timeouts

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4e63d0ac896b6036edd8e2c09a4dac7aa16a46d1

commit 4e63d0ac896b6036edd8e2c09a4dac7aa16a46d1
Author: Pedro Alves <palves@redhat.com>
Date:   Fri May 15 16:26:53 2015 +0100

    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:
---
 gdb/ChangeLog    |  11 +++++
 gdb/event-loop.c | 120 ++++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 99 insertions(+), 32 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 05c0b3f..b7694c7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,15 @@
 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
 	'private' to 'priv'.
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 79e41fd..9ac4908 100644
--- 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;
 }