From: Pedro Alves <pedro@palves.net>
To: gdb-patches@sourceware.org
Subject: [PATCH 1/7] Fix spurious unhandled remote %Stop notifications
Date: Mon, 6 Jul 2020 20:02:46 +0100 [thread overview]
Message-ID: <20200706190252.22552-2-pedro@palves.net> (raw)
In-Reply-To: <20200706190252.22552-1-pedro@palves.net>
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.
---
gdb/remote.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index f7f99dc24f..59075cb09f 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
--
2.14.5
next prev parent reply other threads:[~2020-07-06 19:02 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-06 19:02 [PATCH 0/7] GDB busy loop when interrupting non-stop program (PR 26199) Pedro Alves
2020-07-06 19:02 ` Pedro Alves [this message]
2020-12-12 22:13 ` [PATCH 1/7] Fix spurious unhandled remote %Stop notifications Andrew Burgess
2020-12-13 0:46 ` Simon Marchi via Gdb-patches
2020-07-06 19:02 ` [PATCH 2/7] Fix latent bug in target_pass_ctrlc Pedro Alves
2020-07-06 19:02 ` [PATCH 3/7] Avoid constant stream of TARGET_WAITKIND_NO_RESUMED Pedro Alves
2020-07-06 19:02 ` [PATCH 4/7] Fix handle_no_resumed w/ multiple targets Pedro Alves
2020-07-06 19:02 ` [PATCH 5/7] Make handle_no_resumed transfer terminal Pedro Alves
2020-07-06 19:02 ` [PATCH 6/7] Testcase for previous handle_no_resumed fixes Pedro Alves
2020-07-06 19:02 ` [PATCH 7/7] Fix GDB busy loop when interrupting non-stop program (PR 26199) Pedro Alves
2020-07-06 21:28 ` [PATCH 0/7] " Simon Marchi
2020-07-07 0:25 ` Pedro Alves
2020-07-07 1:27 ` Pedro Alves
2020-07-07 1:29 ` Pedro Alves
2020-07-10 23:02 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200706190252.22552-2-pedro@palves.net \
--to=pedro@palves.net \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox