[PATCH, RFC] fix gdbserver channel leaks

[Sandra Loosemore]

Ping.  Any additional thoughts on this?

On 1/17/19 12:32 PM, Sandra Loosemore wrote:
> 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