[PATCH 2 of 2] chg: send SIGPIPE to server immediately when pager exits (issue5278)

[Jun Wu]

This works for hg commends like:

    @command('wait', [], 'wait', norepo=True)
    def _crash(ui, *args):
        for i in xrange(4000):
            ui.write('%s\n' % i)
        time.sleep(30)

But does not work with shell aliases (in which case SIGCHLD handler is not
triggered). I will investigate later.

Excerpts from Jun Wu's message of 2016-06-24 17:10:18 +0100:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1466778070 -3600
> #      Fri Jun 24 15:21:10 2016 +0100
> # Node ID 701ec0855d2c4b9a40aeb3c8d317aa6b54a3e308
> # Parent  f1195d649b4237400de44ac741f701aded61eb95
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 701ec0855d2c
> chg: send SIGPIPE to server immediately when pager exits (issue5278)
> 
> If the user press 'q' to leave the 'less' pager, it is expected to end the
> hg process immediately. We currently rely on SIGPIPE for this behavior. But
> SIGPIPE won't arrive if we don't write anything (like doing heavy
> computation, reading from network etc). If that happens, the user will feel
> that the hg process just hangs.
> 
> The patch address the issue by adding a SIGCHLD signal handler and sends
> SIGPIPE to the server as soon as the pager exits.
> 
> This is also an issue with hg's pager implementation.
> 
> diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
> --- a/contrib/chg/chg.c
> +++ b/contrib/chg/chg.c
> @@ -338,6 +338,7 @@ static void killcmdserver(const struct c
>      }
>  }
>  
> +static pid_t pagerpid = 0;
>  static pid_t peerpid = 0;
>  
>  static void forwardsignal(int sig)
> @@ -380,6 +381,17 @@ error:
>      abortmsgerrno("failed to handle stop signal");
>  }
>  
> +static void handlechildsignal(int sig)
> +{

Missed "(void)sig" to avoid unused variable warning.

> +    if (peerpid == 0 || pagerpid == 0)
> +        return;
> +    /* if pager exits, notify the server with SIGPIPE immediately.
> +     * otherwise the server won't get SIGPIPE if it does not write
> +     * anything. (issue5278) */
> +    if (waitpid(pagerpid, NULL, WNOHANG) == pagerpid)
> +        kill(peerpid, SIGPIPE);
> +}
> +
>  static void setupsignalhandler(pid_t pid)
>  {
>      if (pid <= 0)
> @@ -416,6 +428,11 @@ static void setupsignalhandler(pid_t pid
>      sa.sa_flags = SA_RESTART;
>      if (sigaction(SIGTSTP, &sa, NULL) < 0)
>          goto error;
> +    /* get notified when pager exits */
> +    sa.sa_handler = handlechildsignal;
> +    sa.sa_flags = SA_RESTART;
> +    if (sigaction(SIGCHLD, &sa, NULL) < 0)
> +        goto error;
>  
>      return;
>  
> @@ -638,7 +655,7 @@ int main(int argc, const char *argv[], c
>      }
>  
>      setupsignalhandler(hgc_peerpid(hgc));
> -    pid_t pagerpid = setuppager(hgc, argv + 1, argc - 1);
> +    pagerpid = setuppager(hgc, argv + 1, argc - 1);
>      int exitcode = hgc_runcommand(hgc, argv + 1, argc - 1);
>      restoresignalhandler();
>      hgc_close(hgc);