Re: [PATCH] gdb/python: avoid switching threads for send_packet where possible

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> Bug PR gdb/32120 identifies a case where a user tried to write a
> Andrew> custom frame unwinder that made use of
> Andrew> RemoteTargetConnection.send_packet
>
> Just for the record this seems like a weird and bad choice.

Probably, I ended up looking at this because I added the send_packet
API, and I was curious why this didn't "just work".

>
> Andrew>   scoped_restore_current_thread restore_thread;
> Andrew>   switch_to_target_no_thread (conn->target);
>
> Andrew> The purpose of this is to ensure that the currently selected inferior
> Andrew> has the required remote connection pushed on its target stack.  This
> Andrew> is necessary because `send_remote_packet` (see remote.c) still relies
> Andrew> on global state read from the currently selected inferior.
>
> Andrew> Switching inferior and thread like this, and then switching back
> Andrew> thanks to the scoped restore, will flush the frame cache.
>
> ...
>
> Andrew> I propose that we special case connpy_send_packet so if the correct
> Andrew> inferior is already current then we avoid the switch completely.  This
> Andrew> avoids flushing the frame cache, and all the associated problems.
>
> No problem with the patch at all, but I wonder about the logic of
> putting this check here and not elsewhere.  Like would we want some sort
> of "switch to this inferior and we don't care which thread" -- which
> would be a no-op if the inferior is already selected?

I posted a v2 that moves the conditional logic into a new helper
function.  It's still a little weird because when we switch we likely
want to use switch_to_target_no_thread which ends with no thread
selected, while in the case where we don't need to switch we leave the
current thread selected.  That thread / no-thread difference is strange,
but hopefully with a good comment it shouldn't cause too many problems.

Anyway, feel free to checkout the V2 and see if that looks OK to you.

Thanks,
Andrew