[PATCH 1 of 3] debugwireproto: close the write end before consuming all available data
Gregory Szorc
gregory.szorc at gmail.com
Mon Mar 12 18:09:54 UTC 2018
On Mon, Mar 12, 2018 at 7:17 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20180312/b122cdb2/attachment-0002.html>
More information about the Mercurial-devel
mailing list