Re: [RFC 1/3] [gdb/dap] Fix exit race

[Tom de Vries <tdevries@suse.de>]

On 2/14/24 17:18, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
>>> Ok.  FWIW I think the Queue idea seems totally fine, and combining
>>> these
>>> patches seems natural to me.
> 
> Tom> Sorry for going on this topic, but I'd like to understand why you
> Tom> think this.
> 
> My thinking is that the code is written this way to solve the flushing
> issue (perhaps not well).  Removing the quit fixes one issue, but it
> risks introducing another.  I'm not really a stickler for this though.

Thanks for the explanation.

OK, I think I start to understand your position.  You've written both 
pieces of code with the intent to address one problem, which is why 
you're thinking about it as a whole.

I haven't added the code, I'm just looking at the behaviour of two 
different races and am seeing two independent problems.

[ FWIW, I understood the send_gdb("quit") as a means to ensure that 
gdb's main thread doesn't keep hanging after the dap threads have 
exited, which AFAICT is a problem unrelated to the flushing race. ]

To follow up on the risk remark, I'm not sure which risk you're 
referring to.

I've documented one risk in the commit log:
...
     So, for the system I'm running this on, the send_gdb("quit") is
     actually not needed.

     I'm not sure if we support any systems where it's actually needed.
...
There's a risk that indeed there are such systems, but I figured we can 
deal with that when we encounter them.

If you mean the risk of not flushing in time, I don't think there's any 
relation with send_gdb("quit").  The problem with not flushing in time 
is there, and AFAIU the problem is not made any better or worse by 
removing the send_gdb("quit").

I'll proceed to commit the approved patch, since I'm keen on getting rid 
of the segfault.

As for the flushing race, as I've documented in the PR, the queue 
approach doesn't completely fix the race.  I wonder if using a 
gdb_exiting hook to wait for the maint dap thread to exit is the way to 
go there.

Thanks,
- Tom