Patchwork [5,of,8,V4] worker: make waitforworkers reentrant

login
register
mail settings
Submitter Jun Wu
Date Nov. 12, 2016, 3:11 a.m.
Message ID <ca128e326027530210f6.1478920296@x1c>
Download mbox | patch
Permalink /patch/17514/
State Changes Requested
Headers show

Comments

Jun Wu - Nov. 12, 2016, 3:11 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1469737019 -3600
#      Thu Jul 28 21:16:59 2016 +0100
# Node ID ca128e326027530210f6509e3ec8b28b97bdce12
# Parent  10d4102eab371e6b6fcbfef92af6fe9457aabcc4
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r ca128e326027
worker: make waitforworkers reentrant

We are going to use it in SIGCHLD handler. The handler will be executed in the
main thread with the non-blocking version of waitpid, while the waitforworkers
thread runs the blocking version. It's possible that one of them collects a
worker and makes the other error out. This patch handles these error: ECHILD
is ignored. EINTR needs a retry.
Yuya Nishihara - Nov. 14, 2016, 1:04 p.m.
On Sat, 12 Nov 2016 03:11:36 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1469737019 -3600
> #      Thu Jul 28 21:16:59 2016 +0100
> # Node ID ca128e326027530210f6509e3ec8b28b97bdce12
> # Parent  10d4102eab371e6b6fcbfef92af6fe9457aabcc4
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r ca128e326027
> worker: make waitforworkers reentrant
> 
> We are going to use it in SIGCHLD handler. The handler will be executed in the
> main thread with the non-blocking version of waitpid, while the waitforworkers
> thread runs the blocking version. It's possible that one of them collects a
> worker and makes the other error out. This patch handles these error: ECHILD
> is ignored. EINTR needs a retry.
> 
> diff --git a/mercurial/worker.py b/mercurial/worker.py
> --- a/mercurial/worker.py
> +++ b/mercurial/worker.py
> @@ -96,7 +96,18 @@ def _posixworker(ui, func, staticargs, a
>                      raise
>      def waitforworkers(blocking=True):
> -        for pid in pids:
> -            p, st = os.waitpid(pid, 0 if blocking else os.WNOHANG)
> +        for pid in pids.copy():
> +            p = st = 0
> +            while True:
> +                try:
> +                    p, st = os.waitpid(pid, 0 if blocking else os.WNOHANG)
> +                except OSError as e:
> +                    if e.errno == errno.EINTR:
> +                        continue
> +                    elif e.errno == errno.ECHLD:
> +                        break # ignore ECHLD

s/ECHLD/ECHILD/

(worker.py isn't well covered by the automated test.)

Patch

diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -96,7 +96,18 @@  def _posixworker(ui, func, staticargs, a
                     raise
     def waitforworkers(blocking=True):
-        for pid in pids:
-            p, st = os.waitpid(pid, 0 if blocking else os.WNOHANG)
+        for pid in pids.copy():
+            p = st = 0
+            while True:
+                try:
+                    p, st = os.waitpid(pid, 0 if blocking else os.WNOHANG)
+                except OSError as e:
+                    if e.errno == errno.EINTR:
+                        continue
+                    elif e.errno == errno.ECHLD:
+                        break # ignore ECHLD
+                    else:
+                        raise
             if p:
+                pids.remove(p)
                 st = _exitstatus(st)
             if st and not problem[0]: