* [PATCH] t9010: Open backflow FIFO once to work around kernel race condition
@ 2012-06-26 22:30 Anders Kaseorg
2012-06-26 22:40 ` Jonathan Nieder
0 siblings, 1 reply; 5+ messages in thread
From: Anders Kaseorg @ 2012-06-26 22:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Barr, Jonathan Nieder, git, Anders Kaseorg
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 <andersk@mit.edu>
---
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 3<backflow &
- } &&
- $maybe_fail_fi git fast-import --cat-blob-fd=3 <stream 3>backflow &&
- 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition
2012-06-26 22:30 [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Anders Kaseorg
@ 2012-06-26 22:40 ` Jonathan Nieder
2012-06-26 22:46 ` Anders Kaseorg
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jonathan Nieder @ 2012-06-26 22:40 UTC (permalink / raw)
To: Anders Kaseorg; +Cc: Junio C Hamano, David Barr, git
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition
2012-06-26 22:40 ` Jonathan Nieder
@ 2012-06-26 22:46 ` Anders Kaseorg
2012-06-26 22:58 ` Junio C Hamano
2012-06-26 23:08 ` Stefano Lattarini
2 siblings, 0 replies; 5+ messages in thread
From: Anders Kaseorg @ 2012-06-26 22:46 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition
2012-06-26 22:40 ` Jonathan Nieder
2012-06-26 22:46 ` Anders Kaseorg
@ 2012-06-26 22:58 ` Junio C Hamano
2012-06-26 23:08 ` Stefano Lattarini
2 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2012-06-26 22:58 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Anders Kaseorg, David Barr, git
Jonathan Nieder <jrnieder@gmail.com> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition
2012-06-26 22:40 ` Jonathan Nieder
2012-06-26 22:46 ` Anders Kaseorg
2012-06-26 22:58 ` Junio C Hamano
@ 2012-06-26 23:08 ` Stefano Lattarini
2 siblings, 0 replies; 5+ messages in thread
From: Stefano Lattarini @ 2012-06-26 23:08 UTC (permalink / raw)
To: Jonathan Nieder
Cc: David Barr, bug-autoconf, git, Junio C Hamano, Anders Kaseorg
[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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-06-26 23:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-26 22:30 [PATCH] t9010: Open backflow FIFO once to work around kernel race condition Anders Kaseorg
2012-06-26 22:40 ` Jonathan Nieder
2012-06-26 22:46 ` Anders Kaseorg
2012-06-26 22:58 ` Junio C Hamano
2012-06-26 23:08 ` Stefano Lattarini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.