Re: cat-file timing window on Cygwin

From: Jeff King <peff@peff.net>
Date: Aug 25, 2017, 15:08 UTC
Message-ID: <20170825150819.agxvbjytom7ao6n6@sigill.intra.peff.net>
In-Reply-To: <20170825112529.GA10378@dinwoodie.org>

On Fri, Aug 25, 2017 at 12:25:29PM +0100, Adam Dinwoodie wrote:

Show 5 quoted lines

> As of v2.10.0-rc1-4-g321459439 ("cat-file: support --textconv/--filters
> in batch mode"), t8010-cat-file-filters.sh has been failing on Cygwin.
> Digging into this, the test looks to expose a timing window: it appears
> that if `git cat-file --textconv --batch` receives input on stdin too
> quickly, it fails to parse some of that input.

Hmm. That commit doesn't seem to actually touch the stdin loop. That happens via strbuf_getline(), which should be pretty robust to timing. But here are a few random thoughts:

  1. Do you build with HAVE_GETDELIM? Does toggling it have any effect?

  2. Does the problem happen only with --textconv?

     If so, I wonder if spawning the child process is somehow draining
     stdin. I don't see how the child would accidentally read from our
     stdin; we replace its stdin with a pipe so it shouldn't get a
     chance to see our descriptor at all.

     If we accidentally called fflush(stdin) that would discard buffered
     input and give the results you're seeing. We don't do that
     directly, of course, but we do call fflush(NULL) in run-command
     before spawning the child. That _should_ just flush output handles,
     but it's possible that there's a cygwin bug, I guess.

I can't reproduce here on Linux. But does the patch below have any impact?

diff --git a/run-command.c b/run-command.c
index 98621faca8..064ebd1995 100644
--- a/run-command.c
+++ b/run-command.c
@@ -641,7 +641,6 @@ int start_command(struct child_process *cmd)
 	}
 
 	trace_argv_printf(cmd->argv, "trace: run_command:");
-	fflush(NULL);
 
 #ifndef GIT_WINDOWS_NATIVE
 {

-Peff