Re: [PATCH v7 2/2] http-backend: respect CONTENT_LENGTH for receive-pack

From: Jeff King <peff@peff.net>
Date: Jun 11, 2018, 09:24 UTC
Message-ID: <20180611092440.GC16414@sigill.intra.peff.net>
In-Reply-To: <20180611091813.GB16414@sigill.intra.peff.net>

On Mon, Jun 11, 2018 at 05:18:13AM -0400, Jeff King wrote:

Show 27 quoted lines

> > >> +sleep 1; # is interrupted by SIGCHLD
> > >> +if (!$exited) {
> > >> +        close($out);
> > >> +        die "Command did not exit after reading whole body";
> > >> +}
> > 
> > > Also, do we need to protect ourselves against other signals being
> > > delivered? E.g., if I resize my xterm and this process gets SIGWINCH, is
> > > it going to erroneously end the sleep and say "nope, no exited signal"?
> > 
> > I'll check, but what could I do? Should I add blocking other
> > signals there?
> 
> I think a more robust check may be to waitpid() on the child for up to N
> seconds. Something like this:
> 
>   $SIG{ALRM} = sub {
> 	  kill(9, $pid);
> 	  die "command did not exit after reading whole body"
>   };
>   alarm(60);
>   waitpid($pid, 0);
>   alarm(0);
> 
> That should exit immediately if $pid does, and otherwise die after
> exactly 60 seconds. Perl's waitpid implementation will restart
> automatically if it gets another signal.

I tried your original, delivering some signals to it. I think it actually is OK, too, because perl's sleep() implementation will also restart for something like SIGWINCH.

E.g., stracing looks like this:

  nanosleep({tv_sec=60, tv_nsec=0}, {tv_sec=57, tv_nsec=791891377}) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
  --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} ---
  restart_syscall(<... resuming interrupted nanosleep ...>

-Peff