[PATCH STABLE V2] worker: ignore meaningless exit status indication returned by os.waitpid()
Jun Wu
quark at fb.com
Fri Feb 24 20:08:22 EST 2017
Looks good. Thanks again for discovering the subtle issue.
Excerpts from FUJIWARA Katsunori's message of 2017-02-25 01:11:11 +0900:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1487952472 -32400
> # Sat Feb 25 01:07:52 2017 +0900
> # Branch stable
> # Node ID 18fb3cf572b49ad25a0f0e62ce1f737c7cef4ec1
> # Parent aa25989b0658dcefbd4c1bce7c389f006f22af30
> worker: ignore meaningless exit status indication returned by os.waitpid()
>
> Before this patch, worker implementation assumes that os.waitpid()
> with os.WNOHANG returns '(0, 0)' for still running child process. This
> is explicitly specified as below in Python API document.
>
> os.WNOHANG
> The option for waitpid() to return immediately if no child
> process status is available immediately. The function returns
> (0, 0) in this case.
>
> On the other hand, POSIX specification doesn't define the "stat_loc"
> value returned by waitpid() with WNOHANG for such child process.
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/waitpid.html
>
> CPython implementation for os.waitpid() on POSIX doesn't take any care
> of this gap, and this may cause unexpected "exit status indication"
> even on POSIX conformance platform.
>
> For example, os.waitpid() with os.WNOHANG returns non-zero "exit
> status indication" on FreeBSD. This implies os.kill() with own pid or
> sys.exit() with non-zero exit code, even if no child process fails.
>
> To ignore meaningless exit status indication returned by os.waitpid(),
> this patch skips subsequent steps forcibly, if os.waitpid() returns 0
> as pid.
>
> This patch also arranges examination of 'p' value for readability.
>
> FYI, there are some issues below about this behavior reported for
> CPython.
>
> https://bugs.python.org/issue21791
> https://bugs.python.org/issue27808
>
> diff --git a/mercurial/worker.py b/mercurial/worker.py
> --- a/mercurial/worker.py
> +++ b/mercurial/worker.py
> @@ -120,9 +120,12 @@ def _posixworker(ui, func, staticargs, a
> break
> else:
> raise
> - if p:
> - pids.discard(p)
> - st = _exitstatus(st)
> + if not p:
Maybe make it more explicit: if p <= 0
> + # skip subsequent steps, because child process should
> + # be still running in this case
> + continue
> + pids.discard(p)
> + st = _exitstatus(st)
> if st and not problem[0]:
> problem[0] = st
> def sigchldhandler(signum, frame):
More information about the Mercurial-devel
mailing list