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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c292927e49cac3d8cdc962b059d8e5a661099e12..b90e45581d33022f8f9348198e56a7ceecf9fbe5 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-07-10  Pedro Alves  <pedro@palves.net>
+
+       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.
+
 2020-07-10  John Baldwin  <jhb@FreeBSD.org>
 
        * fbsd-nat.h (fbsd_nat_target::supports_multi_process): New

diff --git a/gdb/remote.c b/gdb/remote.c
index f7f99dc24fe8acc081de437368f7737d0cb337f6..59075cb09f2052cac6a9332e8ce553c92bf037dd 100644 (file)
--- 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