From: Omar Polo <op@omarpolo.com>
Subject: Re: make 'got send' detect closed connections
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Wed, 13 Sep 2023 20:08:42 +0200

On 2023/09/13 12:28:53 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Wed, Sep 13, 2023 at 12:18:12AM +0200, Omar Polo wrote:
> > On 2023/09/12 21:17:38 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> The idea certainly works (see diff below):
> 
> $  got send -b qwx gothub
> Connecting to "gothub" ssh://stsp@gothub.org/src.git
> Host key fingerprint is SHA256:HT7ohAIc4knlmUm3vFUyyK9/DW/sFccZuPG0oNwBWYA
> 15671 commits coloredgotsh: operation timed out
> 
> got: server unexpectedly closed the connection
> $
> 
> However, the patch as I wrote it has unintended side effects.
> For some reason I am seeing sporadic errors during test runs once
> we start catching sigchld.
> There are two chldren involved (got-send-pack and ssh), which might
> be relatad to this issue.

Yeah, I forgot that we have other children other than ssh... sorry.

It starts to get tricky... 

> I don't have time right now to dig into how we could avoid this.
> 
> $ ./send.sh
> test_send_basic got: ppoll: Interrupted system call
> got clone command failed unexpectedly
> test failed; leaving test data in /tmp/got-test-send_basic-uyH6MHTmyu
> test_send_rebase_required ok
> test_send_rebase_required_overwrite got: ppoll: Interrupted system call

This is SIGCHLD interrupting ppoll() in lib/pollfd.c...

We'd need something like this on top of your diff but then I'm not
sure.  Your initial diff was more self-contained.

If you really want to stop the coloring early then your original diff
is ok op@.  I remembered that the progress callback is ratelimited, so
there's no need to worry about issuing too many poll()s.


diff /home/op/w/got
commit - 62eab86e6a1d5aea8a1bf90999c3c058b9aedd07
path + /home/op/w/got
blob - 32efcd7f1a512fe6f1444aaba4ff7e8aa6d051b1
file + lib/pollfd.c
--- lib/pollfd.c
+++ lib/pollfd.c
@@ -43,6 +43,8 @@ got_poll_fd(int fd, int events, int timeout)
 		return got_error_from_errno("sigemptyset");
 	if (sigaddset(&sigset, SIGWINCH) == -1)
 		return got_error_from_errno("sigaddset");
+	if (sigaddset(&sigset, SIGCHLD) == -1)
+		return got_error_from_errno("sigaddset");
 
 	n = ppoll(pfd, 1, timeout == INFTIM ? NULL : &ts, &sigset);
 	if (n == -1)