gdb: make remote target clear its handler in remote_target::wait

The remote target's remote_async_inferior_event_token is a flag that
tells when it wants the infrun loop to call its wait method.  The flag
is cleared in the async_event_handler's callback
(remote_async_inferior_event_handler), just before calling
inferior_event_handler.  However, since inferior_event_handler may
actually call another target's wait method, there needs to be code that
checks if we need to re-raise the flag.

It would be simpler instead for remote_target::wait to clear the flag
when it returns an event and there are no more to report after that.  If
another target's wait method gets called by inferior_event_handler, the
remote target's flag will stay naturally stay marked.

Note that this is already partially implemented in remote_target::wait,
since the remote target may have multiple events to report (and it can
only report one at the time):

  if (target_is_async_p ())
    {
      remote_state *rs = get_remote_state ();

      /* If there are are events left in the queue tell the event loop
	 to return here.  */
      if (!rs->stop_reply_queue.empty ())
	mark_async_event_handler (rs->remote_async_inferior_event_token);
    }

The code in remote_async_inferior_event_handler also checks for pending
events as well, in addition to the stop reply queue, so I've made
remote_target::wait check for that as well.  I'm not completely sure
this is ok, since I don't understand very well how the pending events
mechanism works.  But I figured it was safer to do this, worst case it
just leads to unnecessary calls to remote_target::wait.

gdb/ChangeLog:

	* remote.c (remote_target::wait): Clear async event handler at
	beginning, mark if needed at the end.
	(remote_async_inferior_event_handler): Don't set or clear async
	event handler.

Change-Id: I20117f5b5acc8a9972c90f16280249b766c1bf37

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 137f3be6a7c6107c9384de1d4b194993cfe8960a..c8cae1c77da57d452d86dbbf2f3b16eba1e1082d 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2021-02-04  Simon Marchi  <simon.marchi@efficios.com>
+
+       * remote.c (remote_target::wait): Clear async event handler at
+       beginning, mark if needed at the end.
+       (remote_async_inferior_event_handler): Don't set or clear async
+       event handler.
+
 2021-02-04  Simon Marchi  <simon.marchi@efficios.com>
 
        * async-event.h (async_event_handler_func):  Add documentation.

diff --git a/gdb/remote.c b/gdb/remote.c
index 47538fce1969845bc10d54b5ea0d7f3dec358e5e..31c6e17a1c4bd381ca2d0c2f917f482b0d39983e 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5672,7 +5672,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, remote,
+    = create_async_event_handler (remote_async_inferior_event_handler, nullptr,
                                  "remote");
   rs->notif_state = remote_notif_state_allocate (remote);
 
@@ -8155,6 +8155,13 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status,
 {
   REMOTE_SCOPED_DEBUG_ENTER_EXIT;
 
+  remote_state *rs = get_remote_state ();
+
+  /* Start by clearing the flag that asks for our wait method to be called,
+     we'll mark it again at the end if needed.  */
+  if (target_is_async_p ())
+    clear_async_event_handler (rs->remote_async_inferior_event_token);
+
   ptid_t event_ptid;
 
   if (target_is_non_stop_p ())
@@ -8164,11 +8171,10 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status,
 
   if (target_is_async_p ())
     {
-      remote_state *rs = get_remote_state ();
-
-      /* If there are are events left in the queue tell the event loop
-        to return here.  */
-      if (!rs->stop_reply_queue.empty ())
+      /* If there are events left in the queue, or unacknowledged
+        notifications, then tell the event loop to call us again.  */
+      if (!rs->stop_reply_queue.empty ()
+         || rs->notif_state->pending_event[notif_client_stop.id] != nullptr)
        mark_async_event_handler (rs->remote_async_inferior_event_token);
     }
 
@@ -14259,21 +14265,7 @@ remote_async_serial_handler (struct serial *scb, void *context)
 static void
 remote_async_inferior_event_handler (gdb_client_data data)
 {
-  remote_target *remote = (remote_target *) data;
-  remote_state *rs = remote->get_remote_state ();
-  clear_async_event_handler (rs->remote_async_inferior_event_token);
-
   inferior_event_handler (INF_REG_EVENT);
-
-  /* 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