diff options
author | Pedro Alves <palves@redhat.com> | 2015-05-15 16:26:53 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2015-05-15 16:26:53 +0100 |
commit | 4e63d0ac896b6036edd8e2c09a4dac7aa16a46d1 (patch) | |
tree | cf9a40558ffa3b91f6b42609f147aa664c7458a1 /gdb/event-loop.c | |
parent | 452003ef2ca3e1cf81b17d960b59b8a8c167054b (diff) | |
download | gdb-4e63d0ac896b6036edd8e2c09a4dac7aa16a46d1.zip gdb-4e63d0ac896b6036edd8e2c09a4dac7aa16a46d1.tar.gz gdb-4e63d0ac896b6036edd8e2c09a4dac7aa16a46d1.tar.bz2 |
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.
Diffstat (limited to 'gdb/event-loop.c')
-rw-r--r-- | gdb/event-loop.c | 120 |
1 files changed, 88 insertions, 32 deletions
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; } |