Fix spurious unhandled remote %Stop notifications
authorPedro Alves <pedro@palves.net>
Mon, 6 Jul 2020 19:02:46 +0000 (20:02 +0100)
committerSimon Marchi <simon.marchi@efficios.com>
Wed, 8 Jul 2020 18:19:08 +0000 (14:19 -0400)
commit2fd8ab4bb319f88b8fc8c3aa8b1a7c5c709f753b
tree22fb43dba0128484396e71ca5ed267fb3a3dc489
parentd1076c4151af8e8d4a343906d2ed1e26cb7809eb
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.
gdb/remote.c
This page took 0.023738 seconds and 4 git commands to generate.