{"thread":{"id":"30912","subject":"[PATCH] t9010: Open backflow FIFO once to work around kernel race condition","startedAt":"2012-06-26T22:30:06Z","lastAt":"2012-06-26T23:08:44Z","messageCount":5,"participants":["Anders Kaseorg","Jonathan Nieder","Junio C Hamano","Stefano Lattarini"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"194325","messageId":"1340749806-22734-1-git-send-email-andersk@mit.edu","threadId":"30912","inReplyTo":null,"subject":"[PATCH] t9010: Open backflow FIFO once to work around kernel race condition","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2012-06-26T22:30:06Z","receivedAt":"2012-06-26T22:30:06Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Linux’s implementation of open() on FIFOs suffers from a race\ncondition (see http://marc.info/?l=linux-kernel&m=134071285509470 )\nthat causes t9010-svn-fe.sh to occasionally fail with\n  t9010-svn-fe.sh: 27: eval: cannot create backflow: Interrupted system call\nAlthough I can’t reproduce this locally, it happens particularly often\non the Launchpad PPA builders.\n\nSidestep this problem by opening the backflow FIFO once for\nread+write.  Also, replace the stream FIFO with a shell pipe so we\ndon’t have to do manual process management.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n t/t9010-svn-fe.sh | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex b7eed24..c52a7de 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -11,9 +11,9 @@ reinit_git () {\n \t\treturn 127\n \tfi\n \trm -fr .git &&\n-\trm -f stream backflow &&\n+\trm -f backflow &&\n \tgit init &&\n-\tmkfifo stream backflow\n+\tmkfifo backflow\n }\n \n try_dump () {\n@@ -22,10 +22,9 @@ try_dump () {\n \tmaybe_fail_fi=${3:+test_$3} &&\n \n \t{\n-\t\t$maybe_fail_svnfe test-svn-fe \"$input\" >stream 3<backflow &\n-\t} &&\n-\t$maybe_fail_fi git fast-import --cat-blob-fd=3 <stream 3>backflow &&\n-\twait $!\n+\t\t$maybe_fail_svnfe test-svn-fe \"$input\" |\n+\t\t$maybe_fail_fi git fast-import --cat-blob-fd=3\n+\t} 3<>backflow\n }\n \n properties () {\n-- \n1.7.11.1\n"},{"id":"194327","messageId":"20120626224053.GA7155@burratino","threadId":"30912","inReplyTo":"1340749806-22734-1-git-send-email-andersk@mit.edu","subject":"Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-26T22:40:53Z","receivedAt":"2012-06-26T22:40:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nQuick first impressions:\n\nAnders Kaseorg wrote:\n\n> Sidestep this problem by opening the backflow FIFO once for\n> read+write.\n\nIs that portable?\n\n>             Also, replace the stream FIFO with a shell pipe so we\n> don’t have to do manual process management.\n\nDoesn't this mean we wouldn't notice if test-svn-fe crashes?\n\nMaybe it would be simpler to make test-svn-fe launch fast-import as a\nchild, avoiding the fifos altogether.\n\nThanks again for tracking this down, by the way.\nJonathan\n"},{"id":"194328","messageId":"alpine.DEB.2.00.1206261843150.3236@dr-wily.mit.edu","threadId":"30912","inReplyTo":"20120626224053.GA7155@burratino","subject":"Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2012-06-26T22:46:49Z","receivedAt":"2012-06-26T22:46:49Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Tue, 26 Jun 2012, Jonathan Nieder wrote:\n> Anders Kaseorg wrote:\n> > Sidestep this problem by opening the backflow FIFO once for\n> > read+write.\n> \n> Is that portable?\n\nPresumably; it’s POSIX, at least.\nhttp://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_07\n\n> Doesn't this mean we wouldn't notice if test-svn-fe crashes?\n\nOh hmm, good point.\n\nAnders\n"},{"id":"194331","messageId":"7vsjdh8yu8.fsf@alter.siamese.dyndns.org","threadId":"30912","inReplyTo":"20120626224053.GA7155@burratino","subject":"Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-26T22:58:39Z","receivedAt":"2012-06-26T22:58:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Hi,\n>\n> Quick first impressions:\n>\n> Anders Kaseorg wrote:\n>\n>> Sidestep this problem by opening the backflow FIFO once for\n>> read+write.\n>\n> Is that portable?\n\nIf you mean [n]<>word, it probably is, even though it is somewhat\nunusual and I wouldn't be surprised if implementations get wrong.\n\n>>             Also, replace the stream FIFO with a shell pipe so we\n>> don’t have to do manual process management.\n>\n> Doesn't this mean we wouldn't notice if test-svn-fe crashes?\n\nVery good point.\n"},{"id":"194332","messageId":"4FEA40FC.2080604@gmail.com","threadId":"30912","inReplyTo":"20120626224053.GA7155@burratino","subject":"Re: [PATCH] t9010: Open backflow FIFO once to work around kernel race condition","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-06-26T23:08:44Z","receivedAt":"2012-06-26T23:08:44Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"[Adding bug-autoconf]\n\nOn 06/27/2012 12:40 AM, Jonathan Nieder wrote:\n> Hi,\n> \n> Quick first impressions:\n> \n> Anders Kaseorg wrote:\n> \n>> Sidestep this problem by opening the backflow FIFO once for\n>> read+write.\n> \n> Is that portable?\n>\nAccording to the Autoconf manual, no:\n\n    Some shells, like ash, don't recognize bi-directional redirection (‘<>’).\n    And even on shells that recognize it, it is not portable to use on fifos:\n    Posix does not require read-write support for named pipes, and Cygwin\n    does not support it:\n\n       $ mkfifo fifo\n       $ exec 5<>fifo\n       $ echo hi >&5\n       bash: echo: write error: Communication error on send\n\nBut while the issue about Cygwin might still be relevant, the one about\nash seems to be out-of-date: I've verified that the \"exec 5<>fifo\"\ncommand works with both dash 0.5.2 and dash 0.5.5.1 (that's why I'm\nCC:ing bug-autoconf).\n\nRegards,\n  Stefano\n"}]}