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

Re: Is t7006-pager.sh racy?

From
Jeff King <peff@peff.net>
Date
Oct 25, 2021, 17:41 UTC
Message-ID
<YXbsPrU6nRSboQ7r@coredump.intra.peff.net>
In-Reply-To
<20211024170349.GA2101@szeder.dev>
On Sun, Oct 24, 2021 at 07:03:49PM +0200, SZEDER Gábor wrote:
Show 9 quoted lines
> On Sat, Oct 23, 2021 at 05:04:42PM -0700, Junio C Hamano wrote:
> > It seems under --stress it is fairly easy to break the said test,
> > especially the one near the end
> 
> I couldn't reproduce a failure with --stress, but after a cursory look
> into those tests I doubt that either that test or any of the
> preceeding SIGPIPE tests added in c24b7f6736 (pager: test for exit
> code with and without SIGPIPE, 2021-02-02) actually check what they
> are supposed to.

Yeah, I am puzzled that they are using test_terminal in the first place (as opposed to just "git -p"). And you are right that a raw git-log is unlikely to be slow enough to get SIGPIPE in most cases.

My usual test for an intentional SIGPIPE is "yes". So something like:
  git -p \
    -c core.pager='exit 0' \
    -c alias.yes='!yes' \
    yes

will reliably trigger SIGPIPE from yes, which git.c will then translate into an exit code of 141.

If you really want to see SIGPIPE from a builtin (which arguably is the more interesting case here, though I think it behaves the same with respect to the pager), it's a bit trickier. One way to do it is with a command that doesn't generate output until after it gets EOF on stdin.

So something like "git log --stdin" works, but you have to contort yourself a bit to make it race-free:

-- >8 -- # The I/O setup here is: # # fifo:log-in stdout # shell -----------> git-log ------> pager # ^ / # \-------------------------------/ # fifo:pager-closed # # The pager closes its stdin, which will give git-log SIGPIPE. But the # tricky part is that after doing so, it signals via fifo to the shell, # which then writes to git-log's stdin, triggering it to actually # generate output (and get SIGPIPE). # # You can verify that it's race-free by inserting a "sleep 3" at the # front of the pager command (before the exec) and seeing that the # other processes wait (and we still get SIGPIPE).

mkfifo pager-closed mkfifo log-in git config core.pager 'exec 0<&-; echo ready >pager-closed; exit 0' (git -p log --stdin <log-in; echo $? >exit-code) &

# we have to open a descriptor rather than just "echo HEAD >log-in", because # that will give git-log an immediate EOF on its input when echo closes it, and # we must wait until the signal from pager-closed. Likewise we cannot wait # for that signal before the echo, because the subshell is blocking on opening # log-in until somebody is hooked up to the write end of the pipe. exec 9>log-in read ok <pager-closed echo HEAD >&9 exec 9>&-

# now we can wait for the subshell to finish and retrieve any output # it produced wait cat exit-code -- >8 --

-Peff
Previous: SZEDER GáborNext: SZEDER Gábor
Message 3 of 17 in “Is t7006-pager.sh racy?”
  1. Junio C HamanoOct 24, 2021
  2. SZEDER GáborOct 24, 2021
  3. Jeff KingOct 25, 2021
  4. SZEDER GáborOct 28, 2021
  5. Jeff KingOct 28, 2021
  6. Jeff KingNov 21, 2021
  7. Jeff KingNov 21, 2021
  8. t7006: clean up SIGPIPE handling in trace2 testsJeff King, Nov 21, 2021
  9. Jeff KingNov 21, 2021
  10. Junio C HamanoNov 22, 2021
  11. Jeff KingNov 22, 2021
  12. Jeff KingNov 22, 2021
  13. Junio C HamanoNov 22, 2021
  14. Junio C HamanoNov 22, 2021
  15. Jeff KingNov 22, 2021
  16. run-command: unify signal and regular logic for wait_or_whine()Jeff King, Nov 22, 2021
  17. Ævar Arnfjörð BjarmasonDec 1, 2021

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.