{"thread":{"id":"34397","subject":"t0008 hang on streaming test (OS X)","startedAt":"2013-07-10T16:36:40Z","lastAt":"2013-07-12T20:42:28Z","messageCount":10,"participants":["Brian Gernhardt","Antoine Pelisse","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"223026","messageId":"6050FACA-CAD4-4E41-B7DC-D7A2036AA233@gernhardtsoftware.com","threadId":"34397","inReplyTo":null,"subject":"t0008 hang on streaming test (OS X)","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2013-07-10T16:36:40Z","receivedAt":"2013-07-10T16:36:40Z","isPatch":false,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"The newest test in t0008 \"streaming support for --stdin\", seems to hang sporadically on my MacBook Pro (running 10.8.4). The hang seems to be related to running it in parallel with other tests, as I can only reliably cause it by running with prove and -j 3. However, once that has hung I am able to semi-reliably have it occur by running the test separately (with the test hung in the background, using separate trash directories via the --root option).\n\nExperimentation has led me to find that it is hanging when trying to read the 2nd response from check-ignore.\n\nI am somewhat stuck on how to fix it. Any ideas?\n\n~~ Brian Gernhardt"},{"id":"223039","messageId":"CALWbr2zWVLGNAPFEc7QGjozxXFyuDLD639c=yZkzWhusq1kV8Q@mail.gmail.com","threadId":"34397","inReplyTo":"6050FACA-CAD4-4E41-B7DC-D7A2036AA233@gernhardtsoftware.com","subject":"Re: t0008 hang on streaming test (OS X)","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-07-10T20:35:53Z","receivedAt":"2013-07-10T20:35:53Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Jul 10, 2013 at 6:36 PM, Brian Gernhardt\n wrote:\n> I am somewhat stuck on how to fix it. Any ideas?\n\nI don't have anything to reproduce here, but usually I start\ninvestigating this kind of problems by attaching the hung process with\ngdb to see the current state (if it's stuck in a specific state), or\nto investigate the end-less loop.\nThat usually help finding a good starting point.\n\nCheers,\nAntoine\n"},{"id":"223066","messageId":"20130711133414.GF6015@sigill.intra.peff.net","threadId":"34397","inReplyTo":"6050FACA-CAD4-4E41-B7DC-D7A2036AA233@gernhardtsoftware.com","subject":"Re: t0008 hang on streaming test (OS X)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-11T13:34:14Z","receivedAt":"2013-07-11T13:34:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 10, 2013 at 12:36:40PM -0400, Brian Gernhardt wrote:\n\n> The newest test in t0008 \"streaming support for --stdin\", seems to\n> hang sporadically on my MacBook Pro (running 10.8.4). The hang seems\n> to be related to running it in parallel with other tests, as I can\n> only reliably cause it by running with prove and -j 3. However, once\n> that has hung I am able to semi-reliably have it occur by running the\n> test separately (with the test hung in the background, using separate\n> trash directories via the --root option).\n\nI can't replicate the hang here (on Linux) doing:\n\n for i in `seq 1 30`; do\n ./t0008-ignores.sh --root=/tmp/foo/$i &\n done\n\nDo you know which test it is hanging on? You mentioned that you can\nreplicate it outside of \"prove\"; what does running with \"-v\" say?\n\nThe last test in t0008, with the fifos, would make me the most\nsuspicious. The way we do it _should_ be fine, but I'm wondering if the\nshell is blocking in exec here:\n\n mkfifo in out &&\n (git check-ignore -n -v --stdin out &) &&\n exec 9>in &&\n\nThat is, if the fifo is not opened for some reason by the backgrounded\nprocess (there's a race, of course, but the outer shell should just\nblock until the sub-shell actually opens it). I wonder if the\ndescriptor-opening behavior of:\n\n cmd out &\n\nis different between shells (that is, if it backgrounds the opening of\nin and out on some shells, but not on others). But then I would expect\nit to fail consistently.\n\nJust for fun, does switching the middle line there to:\n\n (sh -c \"git check-ignore -n -v --stdin out\" &) &&\n\nhave any effect?\n\n-Peff\n"},{"id":"223077","messageId":"7vli5drsbw.fsf@alter.siamese.dyndns.org","threadId":"34397","inReplyTo":"20130711133414.GF6015@sigill.intra.peff.net","subject":"Re: t0008 hang on streaming test (OS X)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T16:09:55Z","receivedAt":"2013-07-11T16:09:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jul 10, 2013 at 12:36:40PM -0400, Brian Gernhardt wrote:\n>\n>> The newest test in t0008 \"streaming support for --stdin\", seems to\n>> hang sporadically on my MacBook Pro (running 10.8.4). The hang seems\n>> to be related to running it in parallel with other tests, as I can\n>> only reliably cause it by running with prove and -j 3. However, once\n>> that has hung I am able to semi-reliably have it occur by running the\n>> test separately (with the test hung in the background, using separate\n>> trash directories via the --root option).\n>\n> I can't replicate the hang here (on Linux) doing:\n>\n> for i in `seq 1 30`; do\n> ./t0008-ignores.sh --root=/tmp/foo/$i &\n> done\n\nIt seems to hang on me with bash, but not dash, here.\n\n> Do you know which test it is hanging on? You mentioned that you can\n> replicate it outside of \"prove\"; what does running with \"-v\" say?\n>\n> The last test in t0008, with the fifos, would make me the most\n> suspicious. The way we do it _should_ be fine, but I'm wondering if the\n> shell is blocking in exec here:\n>\n> mkfifo in out &&\n> (git check-ignore -n -v --stdin out &) &&\n> exec 9>in &&\n>\n> That is, if the fifo is not opened for some reason by the backgrounded\n> process (there's a race, of course, but the outer shell should just\n> block until the sub-shell actually opens it). I wonder if the\n> descriptor-opening behavior of:\n>\n> cmd out &\n>\n> is different between shells (that is, if it backgrounds the opening of\n> in and out on some shells, but not on others). But then I would expect\n> it to fail consistently.\n>\n> Just for fun, does switching the middle line there to:\n>\n> (sh -c \"git check-ignore -n -v --stdin out\" &) &&\n>\n> have any effect?\n>\n> -Peff\n"},{"id":"223078","messageId":"DBBE19C0-BEF4-46FC-9CB3-7BAA26F493AC@gernhardtsoftware.com","threadId":"34397","inReplyTo":"20130711133414.GF6015@sigill.intra.peff.net","subject":"Re: t0008 hang on streaming test (OS X)","fromName":"Brian Gernhardt","fromEmail":"mister.reus@gmail.com","sentAt":"2013-07-11T16:13:14Z","receivedAt":"2013-07-11T16:13:14Z","isPatch":false,"sender":{"key":"mister.reus@gmail.com","avatar":null},"body":"\nOn Jul 11, 2013, at 9:34 AM, Jeff King wrote:\n\n> On Wed, Jul 10, 2013 at 12:36:40PM -0400, Brian Gernhardt wrote:\n> \n>> The newest test in t0008 \"streaming support for --stdin\",\n\n>> Experimentation has led me to find that it is hanging when trying to read the 2nd response from check-ignore.\n\n\n> Do you know which test it is hanging on? You mentioned that you can\n> replicate it outside of \"prove\"; what does running with \"-v\" say?\n> \n> The last test in t0008, with the fifos, would make me the most\n> suspicious.\n\nThe 2nd `read response wrote:\n\n> On Wed, Jul 10, 2013 at 6:36 PM, Brian Gernhardt\n> wrote:\n>> I am somewhat stuck on how to fix it. Any ideas?\n> \n> I don't have anything to reproduce here, but usually I start\n> investigating this kind of problems by attaching the hung process with\n> gdb to see the current state (if it's stuck in a specific state), or\n> to investigate the end-less loop.\n> That usually help finding a good starting point.\n\nUnfortunately, the hung process is /bin/sh (aka bash). Using the \"Sample\" function of Activity Monitor gave me that 100% of the time was spent in libc's _open... Which enlightens me not at all.\n\n~~ Brian Gernhardt\n"},{"id":"223159","messageId":"20130712103522.GA4750@sigill.intra.peff.net","threadId":"34397","inReplyTo":"7vli5drsbw.fsf@alter.siamese.dyndns.org","subject":"[PATCH] t0008: avoid SIGPIPE race condition on fifo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-12T10:35:23Z","receivedAt":"2013-07-12T10:35:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 11, 2013 at 09:09:55AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Wed, Jul 10, 2013 at 12:36:40PM -0400, Brian Gernhardt wrote:\n> >\n> >> The newest test in t0008 \"streaming support for --stdin\", seems to\n> >> hang sporadically on my MacBook Pro (running 10.8.4). The hang seems\n> >> to be related to running it in parallel with other tests, as I can\n> >> only reliably cause it by running with prove and -j 3. However, once\n> >> that has hung I am able to semi-reliably have it occur by running the\n> >> test separately (with the test hung in the background, using separate\n> >> trash directories via the --root option).\n> >\n> > I can't replicate the hang here (on Linux) doing:\n> >\n> > for i in `seq 1 30`; do\n> > ./t0008-ignores.sh --root=/tmp/foo/$i &\n> > done\n> \n> It seems to hang on me with bash, but not dash, here.\n\nThanks, I was able to replicate it with bash, and like Brian, I saw it\nhanging in the second read. strace revealed that we were blocked on\nopen(\"out\").\n\nThe patch below should fix it. I'm still not sure why the choice of\nshell matters; it may simply be a timing fluke that bash is more likely\nto hit for some reason.\n\n-- >8 --\nSubject: [PATCH] t0008: avoid SIGPIPE race condition on fifo\n\nTo test check-ignore's --stdin feature, we use two fifos to\nsend and receive data. We carefully keep a descriptor to its\ninput open so that it does not receive EOF between input\nlines. However, we do not do the same for its output. That\nmeans there is a potential race condition in which\ncheck-ignore has opened the output pipe once (when we read\nthe first line), and then writes the second line before we\nhave re-opened the pipe.\n\nIn that case, check-ignore gets a SIGPIPE and dies. The\nouter shell then tries to open the output fifo but blocks\nindefinitely, because there is no writer. We can fix it by\nkeeping a descriptor open through the whole procedure.\n\nThis should also help if check-ignore dies for any other\nreason (we would already have opened the fifo and would\ntherefore not block, but just get EOF on read).\n\nHowever, we are technically still susceptible to\ncheck-ignore dying early, before we have opened the fifo.\nThis is an unlikely race and shouldn't generally happen in\npractice, though, so we can hopefully ignore it.\n\nSigned-off-by: Jeff King \n---\n t/t0008-ignores.sh | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex a56db80..c29342d 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -697,13 +697,21 @@ test_expect_success PIPE 'streaming support for --stdin' '\n \t# shell, and then echo to the fd. We make sure to close it at\n \t# the end, so that the subprocess does get EOF and dies\n \t# properly.\n+\t#\n+\t# Similarly, we must keep \"out\" open so that check-ignore does\n+\t# not ever get SIGPIPE trying to write to us. Not only would that\n+\t# produce incorrect results, but then there would be no writer on the\n+\t# other end of the pipe, and we would potentially block forever trying\n+\t# to open it.\n \texec 9>in &&\n+\texec 8&-\" &&\n+\ttest_when_finished \"exec 8<&-\" &&\n \techo >&9 one &&\n-\tread response &9 two &&\n-\tread response writes:\n\n> Subject: [PATCH] t0008: avoid SIGPIPE race condition on fifo\n>\n> To test check-ignore's --stdin feature, we use two fifos to\n> send and receive data. We carefully keep a descriptor to its\n> input open so that it does not receive EOF between input\n> lines. However, we do not do the same for its output. That\n> means there is a potential race condition in which\n> check-ignore has opened the output pipe once (when we read\n> the first line), and then writes the second line before we\n> have re-opened the pipe.\n>\n> In that case, check-ignore gets a SIGPIPE and dies. The\n> outer shell then tries to open the output fifo but blocks\n> indefinitely, because there is no writer. We can fix it by\n> keeping a descriptor open through the whole procedure.\n\nAhh, figures.\n\nI wish I were smart enough to figure that out immediately after\nseeing the test that does funny things to \"in\" with \"9\".\n\nThanks.\n\n> This should also help if check-ignore dies for any other\n> reason (we would already have opened the fifo and would\n> therefore not block, but just get EOF on read).\n>\n> However, we are technically still susceptible to\n> check-ignore dying early, before we have opened the fifo.\n> This is an unlikely race and shouldn't generally happen in\n> practice, though, so we can hopefully ignore it.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t0008-ignores.sh | 12 ++++++++++--\n> 1 file changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n> index a56db80..c29342d 100755\n> --- a/t/t0008-ignores.sh\n> +++ b/t/t0008-ignores.sh\n> @@ -697,13 +697,21 @@ test_expect_success PIPE 'streaming support for --stdin' '\n> \t# shell, and then echo to the fd. We make sure to close it at\n> \t# the end, so that the subprocess does get EOF and dies\n> \t# properly.\n> +\t#\n> +\t# Similarly, we must keep \"out\" open so that check-ignore does\n> +\t# not ever get SIGPIPE trying to write to us. Not only would that\n> +\t# produce incorrect results, but then there would be no writer on the\n> +\t# other end of the pipe, and we would potentially block forever trying\n> +\t# to open it.\n> \texec 9>in &&\n> +\texec 8 \ttest_when_finished \"exec 9>&-\" &&\n> +\ttest_when_finished \"exec 8<&-\" &&\n> \techo >&9 one &&\n> -\tread response +\tread response <&8 &&\n> \techo \"$response\" | grep \"^\\.gitignore:1:one\tone\" &&\n> \techo >&9 two &&\n> -\tread response +\tread response <&8 &&\n> \techo \"$response\" | grep \"^::\ttwo\"\n> '\n"},{"id":"223171","messageId":"68152DD0-64A0-4B69-A788-48F9D93BEEC3@gernhardtsoftware.com","threadId":"34397","inReplyTo":"20130712103522.GA4750@sigill.intra.peff.net","subject":"Re: [PATCH] t0008: avoid SIGPIPE race condition on fifo","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2013-07-12T16:42:30Z","receivedAt":"2013-07-12T16:42:30Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Jul 12, 2013, at 6:35 AM, Jeff King wrote:\n\n> Subject: [PATCH] t0008: avoid SIGPIPE race condition on fifo\n\nWas able to complete a prove run with this patch. Many thanks.\n\n~~ Brian\n"},{"id":"223216","messageId":"20130712204228.GB5276@sigill.intra.peff.net","threadId":"34397","inReplyTo":"7vbo67oig5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t0008: avoid SIGPIPE race condition on fifo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-12T20:42:28Z","receivedAt":"2013-07-12T20:42:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 12, 2013 at 09:23:54AM -0700, Junio C Hamano wrote:\n\n> > In that case, check-ignore gets a SIGPIPE and dies. The\n> > outer shell then tries to open the output fifo but blocks\n> > indefinitely, because there is no writer. We can fix it by\n> > keeping a descriptor open through the whole procedure.\n> \n> Ahh, figures.\n> \n> I wish I were smart enough to figure that out immediately after\n> seeing the test that does funny things to \"in\" with \"9\".\n\nI wish I were smart enough to have figured it out when I was writing the\nfunny stuff with \"in\" and \"9\" in the first place. :)\n\n-Peff\n"}]}