git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] unblock and unignore SIGPIPE

From
Johannes Sixt <j6t@kdbg.org>
Date
Sep 20, 2014, 08:42 UTC
Message-ID
<541D3E0C.4030400@kdbg.org>
In-Reply-To
<1411059429-23868-1-git-send-email-patrick.reynolds@github.com>
Am 18.09.2014 um 18:57 schrieb Patrick Reynolds:
Show 56 quoted lines
> Blocked and ignored signals -- but not caught signals -- are inherited
> across exec.  Some callers with sloppy signal-handling behavior can call
> git with SIGPIPE blocked or ignored, even non-deterministically.  When
> SIGPIPE is blocked or ignored, several git commands can run indefinitely,
> ignoring EPIPE returns from write() calls, even when the process that
> called them has gone away.  Our specific case involved a pipe of git
> diff-tree output to a script that reads a limited amount of diff data.
> 
> In an ideal world, git would never be called with SIGPIPE blocked or
> ignored.  But in the real world, several real potential callers, including
> Perl, Apache, and Unicorn, sometimes spawn subprocesses with SIGPIPE
> ignored.  It is easier and more productive to harden git against this
> mistake than to clean it up in every potential parent process.
> 
> Signed-off-by: Patrick Reynolds <patrick.reynolds@github.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 1. Merged Junio's work from pu: moved restore_sigpipe_to_default into
> git.c and restyled the new tests.
> 2. Moved the new tests into t0005.  This meant switching back to `git
> diff` as our data generator, as the sample repo in t0005 doesn't have any
> files for `git ls-files` to output.
> 3. Squashed.
> 
>  git.c              | 22 ++++++++++++++++++++++
>  t/t0005-signals.sh | 22 ++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/git.c b/git.c
> index 210f1ae..0f03d56 100644
> --- a/git.c
> +++ b/git.c
> @@ -593,6 +593,26 @@ static int run_argv(int *argcp, const char ***argv)
>  	return done_alias;
>  }
>  
> +/*
> + * Many parts of Git have subprograms communicate via pipe, expect the
> + * upstream of the pipe to die with SIGPIPE and the downstream process
> + * even knows to check and handle EPIPE correctly.  Some third-party
> + * programs that ignore or block SIGPIPE for their own reason forget
> + * to restore SIGPIPE handling to the default before spawning Git and
> + * break this carefully orchestrated machinery.
> + *
> + * Restore the way SIGPIPE is handled to default, which is what we
> + * expect.
> + */
> +static void restore_sigpipe_to_default(void)
> +{
> +	sigset_t unblock;
> +
> +	sigemptyset(&unblock);
> +	sigaddset(&unblock, SIGPIPE);
> +	sigprocmask(SIG_UNBLOCK, &unblock, NULL);
> +	signal(SIGPIPE, SIG_DFL);
> +}

This does not build on MinGW due to missing sigaddset() and sigprocmask(). I've a patch that adds dummies for them (but I ran out of time to complete it for submission). But then the test cases ...

Show 9 quoted lines
> +test_expect_success 'a constipated git dies with SIGPIPE' '
> +	OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 )
> +	test "$OUT" -eq 141
> +'
> +
> +test_expect_success 'a constipated git dies with SIGPIPE even if parent ignores it' '
> +	OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 )
> +	test "$OUT" -eq 141
> +'

... fail always because we neither get SIGPIPE (we don't have it on Windows) nor do we see a write error (e.g. EPIPE) when writing to the pipe. Should I protect these tests with !MINGW or would it be an option to drop these tests alltogether?

-- Hannes
Previous: James NylenNext: Junio C Hamano
Message 4 of 6 in “unblock and unignore SIGPIPE”
  1. unblock and unignore SIGPIPEPatrick Reynolds, Sep 18, 2014
  2. Junio C HamanoSep 18, 2014
  3. James NylenSep 18, 2014
  4. Johannes SixtSep 20, 2014
  5. Junio C HamanoSep 22, 2014
  6. mingw.h: add dummy functions for sigset_t operationsJohannes Sixt, Sep 22, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.