stable ordering of test output

[Augie Fackler]

On Thu, Apr 13, 2017 at 3:55 PM, Augie Fackler <raf at durin42.com> wrote:
> On Wed, Mar 8, 2017 at 10:44 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>> On Tue, 7 Mar 2017 17:56:58 +0100, Pierre-Yves David wrote:
>>> On the other hand, this is probably not so bundle2 specific. We have
>>> some "select" logic to read stdout and stderr as soon as possible. This
>>> is the main suspect as it is possible that this logic behave different
>>> under linux and other unix (not too much effort have been put into it).
>>
>> posix.poll() waits every type of operation no matter if fd is e.g. writable
>> or not. IIRC, this doesn't always work on FreeBSD since the underlying resource
>> of read/write ends might be shared in the kernel.
>>
>> But I don't think this is the source of the unstable output.
>
> I've had a little time today between things to try and debug this.
> What I've found so far:
>
> 1) when the test passes, the remote: output is printed by the
> _forwardoutput method in sshpeer, presumably since stderr makes it to
> the client before the close of stdout.
> 2) When the test fails (as on BSD, and I guess Solaris), the client
> notices that stdout closed before stderr. It then aborts the
> transaction and sshpeer.cleanup() notices some data chilling on stderr
> and ensures it gets read and printed.
>
> I'm not really sure why BSD systems would be quicker at communicating
> the closed FD than other systems. I'm poking at dummyssh now to see if
> maybe it's weirdness from there...

Here's a patch that seems to work. I'm not happy about it, but it
makes the behavior consistent, and it looks mostly harmless.

# HG changeset patch
# User Augie Fackler <augie at google.com>
# Date 1492114180 14400
#      Thu Apr 13 16:09:40 2017 -0400
# Node ID ec81fd7580f3e31aa92e8834ffbcf2a8e80e72e3
# Parent  35afb54dbb4df2975dbbf0e1525b98611f18ba85
sshpeer: try harder to snag stderr when stdout closes unexpectedly

Resolves test failures on FreeBSD, but I'm not happy about the fix.

diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -110,9 +110,17 @@ class doublepipe(object):
             if mainready:
                 meth = getattr(self._main, methname)
                 if data is None:
-                    return meth()
+                    r = meth()
                 else:
-                    return meth(data)
+                    r = meth(data)
+                if not r and data != 0:
+                    # We've observed a condition that indicates the
+                    # stdout closed unexpectedly. Check stderr one
+                    # more time and snag anything that's there before
+                    # letting anyone know the main part of the pipe
+                    # closed prematurely.
+                    _forwardoutput(self._ui, self._side)
+                return r

     def close(self):
         return self._main.close()