[PATCH] wireproto: explicitly flush stdio to prevent stalls on Windows

[Yuya Nishihara]

On Sun, 11 Mar 2018 10:10:21 -0700, Gregory Szorc wrote:
> On Sun, Mar 11, 2018 at 7:52 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> > On Sun, 11 Mar 2018 08:55:41 -0400, Matt Harbison wrote:
> > >
> > > > On Mar 11, 2018, at 5:43 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> > > >
> > > >> On Sun, 11 Mar 2018 00:04:45 -0500, Matt Harbison wrote:
> > > >> # HG changeset patch
> > > >> # User Matt Harbison <matt_harbison at yahoo.com>
> > > >> # Date 1520744281 18000
> > > >> #      Sat Mar 10 23:58:01 2018 -0500
> > > >> # Node ID 1145671119a40e82932f63f83ad4049727416174
> > > >> # Parent  963b4223d14fa419df2a82fbe47cd55075707b6a
> > > >> wireproto: explicitly flush stdio to prevent stalls on Windows
> > > >>
> > > >> This is the key to fixing the hangs on Windows in D2720[1].  I put
> > flushes in a
> > > >> bunch of other places that didn't help, but I suspect that's more a
> > lack of test
> > > >> coverage than anything else.
> > > >>
> > > >> Chasing down stuff like this is pretty painful.  I'm wondering if we
> > can put a
> > > >> proxy around sys.stderr (and sys.stdout?) on Windows (only when
> > daemonized?)
> > > >> that will flush on every write (or at least every write with a '\n').
> > > >>
> > > >> [1] https://www.mercurial-scm.org/pipermail/mercurial-devel/
> > 2018-March/113352.html
> > > >>
> > > >> diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
> > > >> --- a/mercurial/debugcommands.py
> > > >> +++ b/mercurial/debugcommands.py
> > > >> @@ -2769,6 +2769,8 @@ def debugwireproto(ui, repo, **opts):
> > > >>
> > > >>     # Now perform actions based on the parsed wire language
> > instructions.
> > > >>     for action, lines in blocks:
> > > >> +        util.stdout.flush()
> > > >
> > > > This one is suspicious. Can you clarify which data is flushed? I think
> > > > fileobjectobserver should flush log per method call.
> > >
> > > Yeah, this made no sense to me. I would have expected that it needed to
> > be flushed on the server side.  But, here’s where it stalls without this:
> > >
> > > https://www.mercurial-scm.org/pipermail/mercurial-devel/
> > 2018-March/113388.html
> >
> > Ok, if the server stalls, that might be because of the --noreadstderr
> > option.
> > The client must read stderr so the server can write into the pipe.
> >
> 
> That's probably it. We're using unbuffered pipes to the process running the
> SSH protocol server. So we could very easily get into a deadlock situation
> if the client isn't reading from a pipe when the other end writes to it.
> 
> Maybe the correct thing to do is have the client always read from stderr
> but not print the data unless explicitly instructed to. That would make the
> code a bit more complicated. But I'm not sure how else to get deterministic
> results here, since the very nature of polling two pipes is
> non-deterministic. The only way to be deterministic is to perform blocking
> reads for deterministic amounts - either readline() or read(n).

Maybe we can redirect server's stderr to /dev/null if autoreadstderr is off.
I don't know that's a correct thing, though.