From baa8575b29fe83e158e203485d4d019621c0d245 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 4 Feb 2021 13:34:11 -0500 Subject: [PATCH] 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 --- gdb/ChangeLog | 7 +++++++ gdb/remote.c | 32 ++++++++++++-------------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 137f3be6a7c..c8cae1c77da 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2021-02-04 Simon Marchi + + * 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 * async-event.h (async_event_handler_func): Add documentation. diff --git a/gdb/remote.c b/gdb/remote.c index 47538fce196..31c6e17a1c4 100644 --- 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 -- 2.30.2