aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPedro Alves <pedro@palves.net>2020-07-04 00:39:47 +0100
committerPedro Alves <palves@redhat.com>2020-07-04 13:02:11 +0100
commit14b98a10cae819d8ae113c4d4cb049bac3caf0c2 (patch)
tree28d51fafec323b133a76eb004e8c3039657c52b9
parentad8464f799a4c96c7ab8bdfec3f95846cf54f9b0 (diff)
downloadgdb-14b98a10cae819d8ae113c4d4cb049bac3caf0c2.zip
gdb-14b98a10cae819d8ae113c4d4cb049bac3caf0c2.tar.gz
gdb-14b98a10cae819d8ae113c4d4cb049bac3caf0c2.tar.bz2
Fix spurious unhandled remote %Stop notifications
In non-stop mode, remote targets mark an async event source whose callback is supposed to result in calling remote_target::wait_ns to either process the event queue, or acknowledge an incoming %Stop notification. The callback in question is remote_async_inferior_event_handler, where we call inferior_event_handler, to end up in fetch_inferior_event -> target_wait -> remote_target::wait -> remote_target::wait_ns. A problem here however is that when debugging multiple targets, fetch_inferior_event can pull events out of any target picked at random, for event fairness. This means that when remote_async_inferior_event_handler returns, remote_target::wait may have not been called at all, and thus pending notifications may have not been acked. Because async event sources auto-clear, when remote_async_inferior_event_handler returns the async event handler is no longer marked, so the event loop won't automatically call remote_async_inferior_event_handler again to try to process the pending remote notifications/queue. The result is that stop events may end up not processed, e.g., "interrupt -a" seemingly not managing to stop all threads. Fix this by making remote_async_inferior_event_handler mark the event handler again before returning, if necessary. Maybe a better fix would be to make async event handlers not auto-clear themselves, make that the responsibility of the callback, so that the event loop would keep calling the callback automatically. Or, we could try making so that fetch_inferior_event would optionally handle events only for the target that it got passed down via parameter. However, I don't think now just before branching is the time to try to do any such change. gdb/ChangeLog: PR gdb/26199 * remote.c (remote_target::open_1): Pass remote target pointer as data to create_async_event_handler. (remote_async_inferior_event_handler): Mark async event handler before returning if the remote target still has either pending events or unacknowledged notifications.
-rw-r--r--gdb/remote.c15
1 files changed, 14 insertions, 1 deletions
diff --git a/gdb/remote.c b/gdb/remote.c
index f7f99dc..59075cb 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5605,7 +5605,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
/* Register extra event sources in the event loop. */
rs->remote_async_inferior_event_token
- = create_async_event_handler (remote_async_inferior_event_handler, NULL);
+ = create_async_event_handler (remote_async_inferior_event_handler, remote);
rs->notif_state = remote_notif_state_allocate (remote);
/* Reset the target state; these things will be queried either by
@@ -14164,6 +14164,19 @@ static void
remote_async_inferior_event_handler (gdb_client_data data)
{
inferior_event_handler (INF_REG_EVENT);
+
+ remote_target *remote = (remote_target *) data;
+ remote_state *rs = remote->get_remote_state ();
+
+ /* inferior_event_handler may have consumed an event pending on the
+ infrun side without calling target_wait on the REMOTE target, or
+ may have pulled an event out of a different target. Keep trying
+ for this remote target as long it still has either pending events
+ or unacknowledged notifications. */
+
+ if (rs->notif_state->pending_event[notif_client_stop.id] != NULL
+ || !rs->stop_reply_queue.empty ())
+ mark_async_event_handler (rs->remote_async_inferior_event_token);
}
int