Re: [PATCH, RFC] fix gdbserver channel leaks

[Sandra Loosemore <sandra at codesourcery dot com>]

On 1/17/19 9:12 AM, Pedro Alves wrote:
> On 01/16/2019 09:18 PM, Sandra Loosemore wrote:
> > I've been trying to remove some ancient cruft in our local remote-target gdbserver test harness that has caused problems with some newer GDB tests.  This has exposed what I think is a problem in GDB's own gdbserver-support.exp:  having opened a channel to the gdbserver spawn in gdbserver_start, in many cases it is failing to close it cleanly, causing it to leak so many open channels that eventually TCL runs out and starts giving ERRORs.  Specifically, gdbserver_start is discarding any already-open server_spawn_id without closing it when opening a new channel, and the logic in gdbserver_gdb_exit that tries to clean up the gdbserver connection is failing to actually close the channel before discarding server_spawn_id.
>
> When I read this, a question comes to mind:
>
> How does it happen that we're starting a new gdbserver
> without closing the original one?  It sounds like this may
> be papering over a bug elsewhere?  Shouldn't that be
> an error instead?  Why are we "failing to close it cleanly"?
>
> I mean, default_gdb_spawn doesn't start a new gdb if
> there's already one either.

Well, I initially thought this should be an error too.  But here is one 
example I tracked down where the error was triggering.  (There might be 
others as well.)

gdbserver_gdb_load (in config/gdbserver.exp) calls gdbserver_spawn. 
We're using our own hacked-up version but it was derived from from this 
source; presumably this behavior is part of the interface description of 
what this function is supposed to do.

mi_gdb_target_load calls gdbserver_gdb_load.  It issues the "kill" 
command to kill any already-running program first, but doesn't do 
anything to close the open gdbserver channel.

mi_run_cmd calls mi_gdb_target_load indirectly via mi_run_cmd_full.

mi_runto calls mi_run_cmd indirectly via mi_runto_helper.

There are a lot of individual tests that call mi_runto without doing 
anything to shut down gdbserver first.

-Sandra