On Mon, Mar 12, 2018 at 7:17 AM, Yuya Nishihara <y...@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <y...@tcha.org>
> # Date 1520862453 -32400
> #      Mon Mar 12 22:47:33 2018 +0900
> # Node ID 2e2a5376d006ad77ba9a07d341f6bc5418289af1
> # Parent  ff541b8cdee0cf9b75874639388bdc8b9854ac20
> debugwireproto: close the write end before consuming all available data
>

I queued this one.


>
> And make it read all available data deterministically. Otherwise
> util.poll()
> may deadlock because both stdout and stderr could have no data.
>
> Spotted by the next patch which removes stderr from the fds.
>
> diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
> --- a/mercurial/debugcommands.py
> +++ b/mercurial/debugcommands.py
> @@ -2690,7 +2690,8 @@ def debugwireproto(ui, repo, **opts):
>      readavailable
>      -------------
>
> -    Read all available data from the server.
> +    Close the write end of the connection and read all available data from
> +    the server.
>
>      If the connection to the server encompasses multiple pipes, we poll
> both
>      pipes and read available data.
> @@ -2835,17 +2836,9 @@ def debugwireproto(ui, repo, **opts):
>          elif action == 'close':
>              peer.close()
>          elif action == 'readavailable':
> -            fds = [stdout.fileno(), stderr.fileno()]
> -            try:
> -                act = util.poll(fds)
> -            except NotImplementedError:
> -                # non supported yet case, assume all have data.
> -                act = fds
> -
> -            if stdout.fileno() in act:
> -                util.readpipe(stdout)
> -            if stderr.fileno() in act:
> -                util.readpipe(stderr)
> +            stdin.close()
> +            stdout.read()
> +            stderr.read()
>          elif action == 'readline':
>              stdout.readline()
>          else:
> diff --git a/tests/test-ssh-proto-unbundle.t b/tests/test-ssh-proto-
> unbundle.t
> --- a/tests/test-ssh-proto-unbundle.t
> +++ b/tests/test-ssh-proto-unbundle.t
> @@ -93,6 +93,7 @@ Test pushing bundle1 payload to a server
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 115:
>    e>     abort: incompatible Mercurial client; bundle2 required\n
>    e>     (see https://www.mercurial-scm.org/wiki/IncompatibleClient)\n
> @@ -143,6 +144,7 @@ Test pushing bundle1 payload to a server
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 115:
>    e>     abort: incompatible Mercurial client; bundle2 required\n
>    e>     (see https://www.mercurial-scm.org/wiki/IncompatibleClient)\n
> @@ -260,6 +262,7 @@ ui.write() in hook is redirected to stde
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 196:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -316,6 +319,7 @@ ui.write() in hook is redirected to stde
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 196:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -386,6 +390,7 @@ And a variation that writes multiple lin
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 218:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -443,6 +448,7 @@ And a variation that writes multiple lin
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 218:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -514,6 +520,7 @@ And a variation that does a ui.flush() a
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 202:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -570,6 +577,7 @@ And a variation that does a ui.flush() a
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 202:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -640,6 +648,7 @@ Multiple writes + flush
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 206:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -697,6 +706,7 @@ Multiple writes + flush
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 206:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -768,6 +778,7 @@ ui.write() + ui.write_err() output is ca
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 232:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -827,6 +838,7 @@ ui.write() + ui.write_err() output is ca
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 232:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -900,6 +912,7 @@ print() output is captured
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 193:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -956,6 +969,7 @@ print() output is captured
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 193:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1026,6 +1040,7 @@ Mixed print() and ui.write() are both ca
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 218:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1085,6 +1100,7 @@ Mixed print() and ui.write() are both ca
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 218:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1158,6 +1174,7 @@ print() to stdout and stderr both get ca
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 216:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1217,6 +1234,7 @@ print() to stdout and stderr both get ca
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 216:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1296,6 +1314,7 @@ Shell hook writing to stdout has output
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 212:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1353,6 +1372,7 @@ Shell hook writing to stdout has output
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 212:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1425,6 +1445,7 @@ Shell hook writing to stderr has output
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 212:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1482,6 +1503,7 @@ Shell hook writing to stderr has output
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 212:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1556,6 +1578,7 @@ Shell hook writing to stdout and stderr
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 230:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1615,6 +1638,7 @@ Shell hook writing to stdout and stderr
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 230:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1697,6 +1721,7 @@ Shell and Python hooks writing to stdout
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 273:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1760,6 +1785,7 @@ Shell and Python hooks writing to stdout
>    o> read(1) -> 1: 0
>    result: 0
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 273:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1837,6 +1863,7 @@ Pushing a bundle1 with no output
>    o> read(1) -> 1: 1
>    result: 1
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 100:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1889,6 +1916,7 @@ Pushing a bundle1 with no output
>    o> read(1) -> 1: 1
>    result: 1
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 100:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -1967,6 +1995,7 @@ Pushing a bundle1 with ui.write() and ui
>    o> read(1) -> 1: 1
>    result: 1
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 152:
>    e>     adding changesets\n
>    e>     adding manifests\n
> @@ -2023,6 +2052,7 @@ Pushing a bundle1 with ui.write() and ui
>    o> read(1) -> 1: 1
>    result: 1
>    remote output:
> +  o> read(-1) -> 0:
>    e> read(-1) -> 152:
>    e>     adding changesets\n
>    e>     adding manifests\n
> diff --git a/tests/test-ssh-proto.t b/tests/test-ssh-proto.t
> --- a/tests/test-ssh-proto.t
> +++ b/tests/test-ssh-proto.t
> @@ -1138,6 +1138,7 @@ Multiple upgrades is not allowed
>    i>     hello\n
>    o> readline() -> 1:
>    o>     \n
> +  o> read(-1) -> 0:
>    e> read(-1) -> 42:
>    e>     cannot upgrade protocols multiple times\n
>    e>     -\n
> @@ -1229,6 +1230,7 @@ Upgrade request must be followed by hell
>    i>     invalid\n
>    o> readline() -> 1:
>    o>     \n
> +  o> read(-1) -> 0:
>    e> read(-1) -> 46:
>    e>     malformed handshake protocol: missing hello\n
>    e>     -\n
> @@ -1248,6 +1250,7 @@ Upgrade request must be followed by hell
>    i>     invalid\n
>    o> readline() -> 1:
>    o>     \n
> +  o> read(-1) -> 0:
>    e> read(-1) -> 48:
>    e>     malformed handshake protocol: missing between\n
>    e>     -\n
> @@ -1269,6 +1272,7 @@ Upgrade request must be followed by hell
>    i>     invalid\n
>    o> readline() -> 1:
>    o>     \n
> +  o> read(-1) -> 0:
>    e> read(-1) -> 49:
>    e>     malformed handshake protocol: missing pairs 81\n
>    e>     -\n
>
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to