Re: [patch] Code cleanup: Remove parameter quitting [Re: Crash of GDB with gdbserver btrace enabled]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi Pedro,

I do not plan to do any changes mentioned below, it is just an affirmation
future development has an agreed upon design.


On Mon, 18 Mar 2013 21:25:12 +0100, Pedro Alves wrote:
> Other Debug APIs may
> require a "debug_api_foo_close ()" call that actually results in traffic,
> to properly release the reference to the debug api library.  remote-mips.c
> seems to be doing something like that, and it's the sort of code that
> could hang/block for a while, so it seems to me like the original intent
> of QUITTING would be to handle _that_ kind of blocking.  If "QUITTING",
> GDB is quitting, so don't bother being nice to your debug API library,
> if that takes long and hang a fast "quit" experience, as the whole GDB
> process is going away anyway.  Otherwise, if not QUITTING, take care to
> clean up properly, because the user may want to spawn a new debug session,
> in the same GDB process.

Do you think that the mips_close packet communication should have been done
only if !QUITTING?  This would mean the GDB "quit" command would leave the
MIPS target in some inconsistent state, possibly not ready for a new
connection later (assuming the MIPS target cannot accept new connection
without previous mips_exit_debug packets).

quit_command already does disconnect_tracing and then (target_detach or
target_kill) before target_close.  I miss target_disconnect in quit_command.
This could do the packet communication part of mips_close.  mips_close then
would not communicate anything so that mips_close is always safe in some
broken communication cases.  And even the quit_command call of
disconnect_tracing could be then removed and disconnect_tracing called only
from remote_disconnect.

That target_detach or target_kill could be kept only for compatibility with
old targets (if there are any), normal targets should do
target_detach/target_kill automatically from to_disconnect.


> Yes, it certainly should _not_ send "D", as that's not RSP's "close
> connection packet", it has more meaning than that -- it makes gdbserver itself
> detach from the inferior, for example.

I see I have mistaken "D".  So remote_close does not need to do any packet
communication thanks to the gdbserver protocol design.


> Releasing
> resources in the backend debug API side (the remote stub, in the RSP case)
> related to the inferior IMO (breakpoints, etc.) should not be done in
> to_close.  If necessary, these resources should be released through some
> other mechanism or target hook.  E.g., removing breakpoints in always inserted
> mode could be done even as late as in to_detach, but not in to_close.

OK, thanks for clarification of the design.


Jan