Re: [PATCH stable v2] worker: avoid potential partial write of pickled data
On 22/05/2022 10.09, Manuel Jacob wrote: For some reason, CI fails: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/553261 The test is flaky on my machine as well. Obviously this is related to my change, so please do not merge. cPickle.load() does not properly handle EINTR. This is fixed in another patch "worker: prevent cPickle from directly interacting with C FILEs" that I sent to the mailing list. The reading side of the worker logic suppressed the error if cPickle raised IOError with EINTR (fixed by patch "worker: do not suppress EINTR"). In the failing test, this caused the stream to be in the middle of some pickled data when reading from the stream later, resulting in nonsense. Writing to the stream in smaller pieces made it more likely that reading from the other end of the pipe errored with EINTR. If this patch is put on top of "worker: prevent cPickle from directly interacting with C FILEs", it works. On 22/05/2022 07.28, Manuel Jacob wrote: # HG changeset patch # User Manuel Jacob # Date 1653184234 -7200 # Sun May 22 03:50:34 2022 +0200 # Branch stable # Node ID b475b0ea695438f6b2994eba0ddb3189d8b4fd05 # Parent 477b5145e1a02715f846ce017b460858a58e03b1 # EXP-Topic worker-pickle-fix_partial_write worker: avoid potential partial write of pickled data Previously, the code wrote the pickled data using os.write(). However, os.write() can write less bytes than passed to it. To trigger the problem, the pickled data had to be larger than 2147479552 bytes on my system. Instead, open a file object and pass it to pickle.dump(). This also has the advantage that it doesn’t buffer the whole pickled data in memory. Note that the opened file must be buffered because pickle doesn’t support unbuffered streams because unbuffered streams’ write() method might write less bytes than passed to it (like os.write()) but pickle.dump() relies on that all bytes are written (see https://github.com/python/cpython/issues/93050). The side effect of using a file object and a with statement is that wfd is explicitly closed now while it seems like before it was implicitly closed by process exit. diff --git a/mercurial/worker.py b/mercurial/worker.py --- a/mercurial/worker.py +++ b/mercurial/worker.py @@ -255,8 +255,10 @@ os.close(r) os.close(w) os.close(rfd) - for result in func(*(staticargs + (pargs,))): - os.write(wfd, util.pickle.dumps(result)) + with os.fdopen(wfd, 'wb') as wf: + for result in func(*(staticargs + (pargs,))): + util.pickle.dump(result, wf) + wf.flush() return 0 ret = scmutil.callcatch(ui, workerfunc) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 stable] worker: prevent cPickle from directly interacting with C FILEs
# HG changeset patch # User Manuel Jacob # Date 1653434314 -7200 # Wed May 25 01:18:34 2022 +0200 # Branch stable # Node ID a9e51487cb9a73f871daf0d43a2b6eec9adadbbd # Parent d058898bdd462b03c5bff38ad40d1f855ea51c23 # EXP-Topic worker-pickle-load-EINTR worker: prevent cPickle from directly interacting with C FILEs diff --git a/mercurial/worker.py b/mercurial/worker.py --- a/mercurial/worker.py +++ b/mercurial/worker.py @@ -100,6 +100,8 @@ del buf[pos:] return bytes(buf) +_picklereader = _blockingreader + else: @@ -111,6 +113,24 @@ def _blockingreader(wrapped): return wrapped +# The following hack is needed to prevent cPickle from using its buggy fast +# path for directly interacting with C FILEs. Instead, the file object +# should interact with C FILEs. For that, we need to make cPickle think +# that we did not actually pass a file object. +# CPickle's interaction with FILEs has at least the following two problems: +# 1) CPickle does not retry operations on EINTR, while file objects do. +# 2) CPickle does not check if an error (e.g. EINTR) happened during +#getc(). Instead, it thinks that EOF was reached. + +class _passthrough(object): +def __init__(self, wrapped): +self._wrapped = wrapped + +def __getattr__(self, name): +return getattr(self._wrapped, name) + +_picklereader = _passthrough + if pycompat.isposix or pycompat.iswindows: _STARTUP_COST = 0.01 @@ -292,7 +312,7 @@ while openpipes > 0: for key, events in selector.select(): try: -res = util.pickle.load(_blockingreader(key.fileobj)) +res = util.pickle.load(_picklereader(key.fileobj)) if hasretval and res[0]: retval.update(res[1]) else: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 stable] worker: do not suppress EINTR
# HG changeset patch # User Manuel Jacob # Date 1653433864 -7200 # Wed May 25 01:11:04 2022 +0200 # Branch stable # Node ID d058898bdd462b03c5bff38ad40d1f855ea51c23 # Parent 477b5145e1a02715f846ce017b460858a58e03b1 # EXP-Topic worker-pickle-load-EINTR worker: do not suppress EINTR Before this change, when IOError with errno EINTR was raised during pickle.load(), the error was suppressed and loading from other file descriptors was continued. On Python 3, system calls failing with EINTR are retried (PEP 475). Therefore, the removal of this code should not make any difference. On Python 2, this is not generally the case. CPickle has no handling of EINTR. In one place it misinterprets it as EOF. In another place, it will raise IOError. However, this could happen in the middle of the stream. In this case, if pickle tries to load from the stream later, it will behave wrongly (usually it will raise an error, but loading of incorrect data or interpreter crashes are thinkable). diff --git a/mercurial/worker.py b/mercurial/worker.py --- a/mercurial/worker.py +++ b/mercurial/worker.py @@ -301,10 +301,6 @@ selector.unregister(key.fileobj) key.fileobj.close() openpipes -= 1 -except IOError as e: -if e.errno == errno.EINTR: -continue -raise except: # re-raises killworkers() cleanup() ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
pycompat.ispy3 in tests
I’ve seen that there are some pycompat.ispy3 left in tests/, while they were removed in mercurial/. Was there a specific reason that they are left in the tests (e.g. the possibility to run new tests with older Mercurial versions) or was the removal just not done yet? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@49212: 2 new changesets
2 new changesets in mercurial:
https://www.mercurial-scm.org/repo/hg/rev/675594a0a71a
changeset: 49211:675594a0a71a
parent: 49207:8960d8eb747b
user:Anton Shestakov
date:Tue May 17 21:49:36 2022 +0400
summary: revlog: use %d to format int instead of %lu (issue6565)
https://www.mercurial-scm.org/repo/hg/rev/d3d3495a5749
changeset: 49212:d3d3495a5749
bookmark:@
tag: tip
user:Anton Shestakov
date:Tue May 24 19:09:24 2022 +0400
summary: revlog: use appropriate format char for int ("i" instead of "I")
--
Repository URL: https://www.mercurial-scm.org/repo/hg
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@49210: 2 new changesets (2 on stable)
2 new changesets (2 on stable) in mercurial: https://www.mercurial-scm.org/repo/hg/rev/2dd53a33aefa changeset: 49209:2dd53a33aefa branch: stable user:Arseniy Alekseyev date:Sun May 22 23:26:06 2022 +0200 summary: test-revlog: test a repository that contains a diff against nullrev https://www.mercurial-scm.org/repo/hg/rev/cc92ad0e8185 changeset: 49210:cc92ad0e8185 branch: stable tag: tip user:Arseniy Alekseyev date:Sun May 22 14:21:59 2022 +0200 summary: rhg: correctly handle the case where diffs are encoded relative to nullrev -- Repository URL: https://www.mercurial-scm.org/repo/hg ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@49208: new changeset (1 on stable)
New changeset (1 on stable) in mercurial: https://www.mercurial-scm.org/repo/hg/rev/2fe4efaa59af changeset: 49208:2fe4efaa59af branch: stable tag: tip parent: 49205:45e71954612c user:Matt Harbison date:Tue May 17 14:36:57 2022 -0400 summary: worker: adapt _blockingreader to work around a python3.8.[0-1] bug (issue6444) -- Repository URL: https://www.mercurial-scm.org/repo/hg ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@49207: 5 new changesets (2 on stable)
5 new changesets (2 on stable) in mercurial: https://www.mercurial-scm.org/repo/hg/rev/f923bdd7477d changeset: 49203:f923bdd7477d parent: 49200:71774d799de7 user:Pierre-Yves David date:Fri May 13 15:19:57 2022 +0200 summary: branchmap: use a context manager when writing the branchmap https://www.mercurial-scm.org/repo/hg/rev/3f9125db466f changeset: 49204:3f9125db466f branch: stable parent: 49202:2d0e22171ef9 user:Anton Shestakov date:Wed May 04 13:48:40 2022 +0400 summary: check-py3-compat: use an absolute path in sys.path https://www.mercurial-scm.org/repo/hg/rev/45e71954612c changeset: 49205:45e71954612c branch: stable user:Anton Shestakov date:Wed May 04 13:53:12 2022 +0400 summary: doc: use an absolute path in sys.path https://www.mercurial-scm.org/repo/hg/rev/b5858e02e3ba changeset: 49206:b5858e02e3ba parent: 49203:f923bdd7477d user:Pierre-Yves David date:Tue Apr 06 03:23:46 2021 +0200 summary: manifest: improve error message in case for tree manifest https://www.mercurial-scm.org/repo/hg/rev/8960d8eb747b changeset: 49207:8960d8eb747b bookmark:@ tag: tip user:Pierre-Yves David date:Tue Apr 06 03:24:26 2021 +0200 summary: filelog: show the passed argument on error -- Repository URL: https://www.mercurial-scm.org/repo/hg ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial-devel | Failed pipeline for branch/stable | 2d0e2217
Pipeline #54285 has failed! Project: mercurial-devel ( https://foss.heptapod.net/mercurial/mercurial-devel ) Branch: branch/stable ( https://foss.heptapod.net/mercurial/mercurial-devel/-/commits/branch/stable ) Commit: 2d0e2217 ( https://foss.heptapod.net/mercurial/mercurial-devel/-/commit/2d0e22171ef9747cc9f17e82d78e6442457c69ea ) Commit Message: rhg: align the dirstate v2 writing algorithm wi... Commit Author: Arseniy Alekseyev ( https://foss.heptapod.net/aalekseyev ) Pipeline #54285 ( https://foss.heptapod.net/mercurial/mercurial-devel/-/pipelines/54285 ) triggered by Raphaël Gomès ( https://foss.heptapod.net/raphael.gomes ) had 1 failed job. Job #553849 ( https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/553849/raw ) Stage: tests Name: test-py3-chg -- You're receiving this email because of your account on foss.heptapod.net. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@49202: 12 new changesets (2 on stable)
12 new changesets (2 on stable) in mercurial: https://www.mercurial-scm.org/repo/hg/rev/4450faeb52bb changeset: 49191:4450faeb52bb user:Raphaël Gomès date:Fri Apr 15 22:02:07 2022 +0200 summary: rust: make requirements public https://www.mercurial-scm.org/repo/hg/rev/2ab79873786e changeset: 49192:2ab79873786e user:Pierre-Yves David date:Tue Apr 05 05:19:47 2022 +0200 summary: auto-upgrade: introduce a way to auto-upgrade to/from share-safe https://www.mercurial-scm.org/repo/hg/rev/566066826e7c changeset: 49193:566066826e7c user:Pierre-Yves David date:Mon Apr 04 19:30:32 2022 +0200 summary: upgrade: split some logic from UpgradeOperation https://www.mercurial-scm.org/repo/hg/rev/e4b31016e194 changeset: 49194:e4b31016e194 user:Pierre-Yves David date:Tue Apr 05 05:20:05 2022 +0200 summary: auto-upgrade: introduce a way to auto-upgrade to/from tracked-hint https://www.mercurial-scm.org/repo/hg/rev/411d591e0a27 changeset: 49195:411d591e0a27 user:Pierre-Yves David date:Tue Mar 22 14:14:52 2022 +0100 summary: auto-upgrade: introduce a way to auto-upgrade to/from dirstate-v2 https://www.mercurial-scm.org/repo/hg/rev/1c233af99316 changeset: 49196:1c233af99316 user:Pierre-Yves David date:Tue Apr 05 03:36:31 2022 +0200 summary: auto-upgrade: add a test case with no permission to lock the repository https://www.mercurial-scm.org/repo/hg/rev/883be4c74d54 changeset: 49197:883be4c74d54 user:Pierre-Yves David date:Tue Apr 05 04:41:09 2022 +0200 summary: debuglock: make the command more useful in non-interactive mode https://www.mercurial-scm.org/repo/hg/rev/a68b37524d50 changeset: 49198:a68b37524d50 user:Pierre-Yves David date:Tue Apr 05 04:43:34 2022 +0200 summary: wait-on-file: properly wait on any files and symlink https://www.mercurial-scm.org/repo/hg/rev/575f3dedb69a changeset: 49199:575f3dedb69a user:Pierre-Yves David date:Tue Apr 05 04:45:48 2022 +0200 summary: auto-upgrade: add a test case where the repository is already locked https://www.mercurial-scm.org/repo/hg/rev/71774d799de7 changeset: 49200:71774d799de7 bookmark:@ user:Pierre-Yves David date:Tue Apr 05 05:01:58 2022 +0200 summary: auto-upgrade: skip the operation if the repository cannot be locked https://www.mercurial-scm.org/repo/hg/rev/c29e79d11b01 changeset: 49201:c29e79d11b01 branch: stable parent: 49181:477b5145e1a0 user:Arseniy Alekseyev date:Tue May 17 14:59:25 2022 +0100 summary: test-dirstate: actually test the append code path in dirstate v2 https://www.mercurial-scm.org/repo/hg/rev/2d0e22171ef9 changeset: 49202:2d0e22171ef9 branch: stable tag: tip user:Arseniy Alekseyev date:Thu May 19 12:23:38 2022 +0100 summary: rhg: align the dirstate v2 writing algorithm with python -- Repository URL: https://www.mercurial-scm.org/repo/hg ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@49190: 2 new changesets
2 new changesets in mercurial: https://www.mercurial-scm.org/repo/hg/rev/237855525f64 changeset: 49189:237855525f64 user:Anton Shestakov date:Mon May 02 16:27:14 2022 +0400 summary: tests: make sure .js files stay in ASCII encoding (issue6559) https://www.mercurial-scm.org/repo/hg/rev/4ff4e23de7df changeset: 49190:4ff4e23de7df bookmark:@ tag: tip user:Arseniy Alekseyev date:Tue May 10 20:30:26 2022 +0100 summary: clone: use better names for temp files -- Repository URL: https://www.mercurial-scm.org/repo/hg ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH stable] rust: relax im-rc dependency to allow minor updates
# HG changeset patch # User Mads Kiilerich # Date 1653395384 -7200 # Tue May 24 14:29:44 2022 +0200 # Branch stable # Node ID 32de89308ead51d42d631c208ab17c866218e1c5 # Parent 1d257c4c15683ee998edcc7dd6caf5a4cb52c820 rust: relax im-rc dependency to allow minor updates This "15.0.*" requirement came from 0d99778af68a and is now replaced with plain "15.0". AFAICS, it really should allow (but not necessarily require) im-rc 15.1 . Narrow requirement requirements with wildcard in the version is not used in other places. diff --git a/rust/hg-core/Cargo.toml b/rust/hg-core/Cargo.toml --- a/rust/hg-core/Cargo.toml +++ b/rust/hg-core/Cargo.toml @@ -14,7 +14,7 @@ bytes-cast = "0.2" byteorder = "1.3.4" derive_more = "0.99" home = "0.5" -im-rc = "15.0.*" +im-rc = "15.0" itertools = "0.9" lazy_static = "1.4.0" libc = "0.2" ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
