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

Re: Is t7006-pager.sh racy?

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Oct 28, 2021, 19:55 UTC
Message-ID
<20211028195527.GA2574@szeder.dev>
In-Reply-To
<YXbsPrU6nRSboQ7r@coredump.intra.peff.net>
On Mon, Oct 25, 2021 at 01:41:18PM -0400, Jeff King wrote:
Show 25 quoted lines
> On Sun, Oct 24, 2021 at 07:03:49PM +0200, SZEDER Gábor wrote:
> 
> > 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.

Oh, that's clever. Alas it's not applicable to our tests, because 'yes' is not portable; 8648732e29 (t/test-lib.sh: provide a shell implementation of the 'yes' utility, 2009-08-28).

Show 46 quoted lines
> 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 --
Ugh.  I think this would work reliably, but...  ugh :)

I wonder whether we could do this as a new pair of 'test-tool' helpers, one to run the pager through the usual pager-invoking machinery and to generate a lot of output, the other to be used as the early-exiting pager, with a pipe between the two to ensure that the SIGPIPE does happen. Well, essentially the same that you outlined above but in C instead of shell, which I somehow find less "ugh".

Previous: Jeff KingNext: Jeff King
Message 4 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.