# [PATCH] t9010: Open backflow FIFO once to work around kernel race condition 5 messages from 2012-06-26 to 2012-06-26. Participants: Anders Kaseorg, Jonathan Nieder, Junio C Hamano, Stefano Lattarini. Thread: https://gitlist.dev/t/30912 ## Anders Kaseorg, 2012-06-26 22:30 Subject: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Message-ID: <1340749806-22734-1-git-send-email-andersk@mit.edu> URL: https://gitlist.dev/e/1340749806-22734-1-git-send-email-andersk%40mit.edu ``` Linux’s implementation of open() on FIFOs suffers from a race condition (see http://marc.info/?l=linux-kernel&m=134071285509470 ) that causes t9010-svn-fe.sh to occasionally fail with t9010-svn-fe.sh: 27: eval: cannot create backflow: Interrupted system call Although I can’t reproduce this locally, it happens particularly often on the Launchpad PPA builders. Sidestep this problem by opening the backflow FIFO once for read+write. Also, replace the stream FIFO with a shell pipe so we don’t have to do manual process management. Signed-off-by: Anders Kaseorg --- t/t9010-svn-fe.sh | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index b7eed24..c52a7de 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -11,9 +11,9 @@ reinit_git () { return 127 fi rm -fr .git && - rm -f stream backflow && + rm -f backflow && git init && - mkfifo stream backflow + mkfifo backflow } try_dump () { @@ -22,10 +22,9 @@ try_dump () { maybe_fail_fi=${3:+test_$3} && { - $maybe_fail_svnfe test-svn-fe "$input" >stream 3backflow && - wait $! + $maybe_fail_svnfe test-svn-fe "$input" | + $maybe_fail_fi git fast-import --cat-blob-fd=3 + } 3<>backflow } properties () { -- 1.7.11.1 ``` ## Jonathan Nieder, 2012-06-26 22:40 Subject: Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Message-ID: <20120626224053.GA7155@burratino> URL: https://gitlist.dev/e/20120626224053.GA7155%40burratino In-Reply-To: <1340749806-22734-1-git-send-email-andersk@mit.edu> ``` Hi, Quick first impressions: Anders Kaseorg wrote: > Sidestep this problem by opening the backflow FIFO once for > read+write. Is that portable? > Also, replace the stream FIFO with a shell pipe so we > don’t have to do manual process management. Doesn't this mean we wouldn't notice if test-svn-fe crashes? Maybe it would be simpler to make test-svn-fe launch fast-import as a child, avoiding the fifos altogether. Thanks again for tracking this down, by the way. Jonathan ``` ## Anders Kaseorg, 2012-06-26 22:46 Subject: Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1206261843150.3236%40dr-wily.mit.edu In-Reply-To: <20120626224053.GA7155@burratino> ``` On Tue, 26 Jun 2012, Jonathan Nieder wrote: > Anders Kaseorg wrote: > > Sidestep this problem by opening the backflow FIFO once for > > read+write. > > Is that portable? Presumably; it’s POSIX, at least. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_07 > Doesn't this mean we wouldn't notice if test-svn-fe crashes? Oh hmm, good point. Anders ``` ## Junio C Hamano, 2012-06-26 22:58 Subject: Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Message-ID: <7vsjdh8yu8.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsjdh8yu8.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120626224053.GA7155@burratino> ``` Jonathan Nieder writes: > Hi, > > Quick first impressions: > > Anders Kaseorg wrote: > >> Sidestep this problem by opening the backflow FIFO once for >> read+write. > > Is that portable? If you mean [n]<>word, it probably is, even though it is somewhat unusual and I wouldn't be surprised if implementations get wrong. >> Also, replace the stream FIFO with a shell pipe so we >> don’t have to do manual process management. > > Doesn't this mean we wouldn't notice if test-svn-fe crashes? Very good point. ``` ## Stefano Lattarini, 2012-06-26 23:08 Subject: Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Message-ID: <4FEA40FC.2080604@gmail.com> URL: https://gitlist.dev/e/4FEA40FC.2080604%40gmail.com In-Reply-To: <20120626224053.GA7155@burratino> ``` [Adding bug-autoconf] On 06/27/2012 12:40 AM, Jonathan Nieder wrote: > Hi, > > Quick first impressions: > > Anders Kaseorg wrote: > >> Sidestep this problem by opening the backflow FIFO once for >> read+write. > > Is that portable? > According to the Autoconf manual, no: Some shells, like ash, don't recognize bi-directional redirection (‘<>’). And even on shells that recognize it, it is not portable to use on fifos: Posix does not require read-write support for named pipes, and Cygwin does not support it: $ mkfifo fifo $ exec 5<>fifo $ echo hi >&5 bash: echo: write error: Communication error on send But while the issue about Cygwin might still be relevant, the one about ash seems to be out-of-date: I've verified that the "exec 5<>fifo" command works with both dash 0.5.2 and dash 0.5.5.1 (that's why I'm CC:ing bug-autoconf). Regards, Stefano ```