[PATCH] gdbserver: Don't try to reopen stdio just quit

[PATCH] gdbserver: Don't try to reopen stdio just quit

[Mark Wielaard]

The testsuite sometimes leaves around gdbserver processes that are
stuck in a loop trying to reopen stdio, which will not work. Once the
write side of the pipe is gone it won't come back because it cannot be
reconnected to another process.

This requires the buildbot to killall gdbserver after running check-gdb.

Add an extra check for remote_connection_is_stdio () in captured_main
to quit in the gdbserver was started through a pipe with - as comm
and stdio as event source is gone.
---
 gdbserver/server.cc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 6f9d2a85609b..c22aa98854cc 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -4382,13 +4382,18 @@ captured_main (int argc, char *argv[])
 
 	      - If --once was specified, we're done.
 
+	      - If the remote connection was stdio it is now closed
+		and it will not reopen.
+
 	      - If not in extended-remote mode, and we're no longer
 		debugging anything, simply exit: GDB has disconnected
 		after processing the last process exit.
 
 	      - Otherwise, close the connection and reopen it at the
 		top of the loop.  */
-	  if (run_once || (!extended_protocol && !target_running ()))
+	  if (run_once
+	      || remote_connection_is_stdio ()
+	      || (!extended_protocol && !target_running ()))
 	    throw_quit ("Quit");
 
 	  fprintf (stderr,
-- 
2.45.2

Re: [PATCH] gdbserver: Don't try to reopen stdio just quit

[Tom Tromey]

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

Mark> The testsuite sometimes leaves around gdbserver processes that are
Mark> stuck in a loop trying to reopen stdio, which will not work. Once the
Mark> write side of the pipe is gone it won't come back because it cannot be
Mark> reconnected to another process.

I was wondering if this is also fixed by

https://sourceware.org/pipermail/gdb-patches/2024-May/209398.html

Tom

Re: [PATCH] gdbserver: Don't try to reopen stdio just quit

[Mark Wielaard]

Hi Tom,

On Wed, Jun 12, 2024 at 07:55:51AM -0600, Tom Tromey wrote:
> >>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:
> 
> Mark> The testsuite sometimes leaves around gdbserver processes that are
> Mark> stuck in a loop trying to reopen stdio, which will not work. Once the
> Mark> write side of the pipe is gone it won't come back because it cannot be
> Mark> reconnected to another process.
> 
> I was wondering if this is also fixed by
> 
> https://sourceware.org/pipermail/gdb-patches/2024-May/209398.html

Yes, that is conceptually the same fix. I actually like it better. So
I withdraw mine. Please approve and apply that one.

Cheers,

Mark

Re: [PATCH] gdbserver: Don't try to reopen stdio just quit

[Tom Tromey]

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

>> https://sourceware.org/pipermail/gdb-patches/2024-May/209398.html

Mark> Yes, that is conceptually the same fix. I actually like it better. So
Mark> I withdraw mine. Please approve and apply that one.

I believe Andrew requested a test case.

Tom

Re: [PATCH] gdbserver: Don't try to reopen stdio just quit

[Mark Wielaard]

Hi Tom,

On Wed, Jun 12, 2024 at 12:44:23PM -0600, Tom Tromey wrote:
> >>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:
> 
> >> https://sourceware.org/pipermail/gdb-patches/2024-May/209398.html
> 
> Mark> Yes, that is conceptually the same fix. I actually like it better. So
> Mark> I withdraw mine. Please approve and apply that one.
> 
> I believe Andrew requested a test case.

That sounds tricky since it isn't directly observable from gdb itself
since it happens when gdb quits or closes the write side of gdbserver
stdio connection without killing gdbserver itself. So a testcase would
need to detect the gdbserver that gdb started is still running after
gdb itself is already gone. I don't know how to do that.

Maybe you could try running:

  echo | gdbserver - /bin/true

and see whether it terminates with a timeout?

It will stop gdbserver and /bin/true with the patch, but go into an
infinite loop without.

Cheers,

Mark

Re: [PATCH] gdbserver: Don't try to reopen stdio just quit

[Guinevere Larsen]

On 6/12/24 4:22 PM, Mark Wielaard wrote:
> Hi Tom,
>
> On Wed, Jun 12, 2024 at 12:44:23PM -0600, Tom Tromey wrote:
>>>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:
>>>> https://sourceware.org/pipermail/gdb-patches/2024-May/209398.html
>> Mark> Yes, that is conceptually the same fix. I actually like it better. So
>> Mark> I withdraw mine. Please approve and apply that one.
>>
>> I believe Andrew requested a test case.
> That sounds tricky since it isn't directly observable from gdb itself
> since it happens when gdb quits or closes the write side of gdbserver
> stdio connection without killing gdbserver itself. So a testcase would
> need to detect the gdbserver that gdb started is still running after
> gdb itself is already gone. I don't know how to do that.
>
> Maybe you could try running:
>
>    echo | gdbserver - /bin/true
>
> and see whether it terminates with a timeout?
In theory, you could keep gdb running and just run a disconnect. I see 
the same 5s delay with gdbserver not closing the connection when running 
that. The problem is that when I suggested this to vvilliam, the problem 
didn't reproduce on TCL (and there were some other problems with his 
setup that delayed things more)...

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> It will stop gdbserver and /bin/true with the patch, but go into an
> infinite loop without.
>
> Cheers,
>
> Mark
>

Re: [PATCH] gdbserver: Don't try to reopen stdio just quit

[Tom Tromey]

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

Mark> Maybe you could try running:

Mark>   echo | gdbserver - /bin/true

Mark> and see whether it terminates with a timeout?

It seems to me that with expect we can just send an eof and verify that
gdbserver exits?

Tom