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

login
register
mail settings
Submitter Jun Wu
Date Nov. 15, 2016, 2:39 a.m.
Message ID <9dbb3532b173a980f341.1479177548@x1c>
Download mbox | patch
Permalink /patch/17581/
State Accepted
Headers show

Comments

Jun Wu - Nov. 15, 2016, 2:39 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1479175936 0
#      Tue Nov 15 02:12:16 2016 +0000
# Node ID 9dbb3532b173a980f341e41d9e96338a386364e5
# Parent  2ab9239f357f1739b2a230bee69ad5eaa81493a6
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 9dbb3532b173
worker: make waitforworkers reentrant

We are going to use it in the 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 (no child to wait).
This patch handles these errors: ECHILD is ignored. EINTR needs a retry.

The "pids" set is designed to be only modifiable by "waitforworkers". And we
only remove items after a successful waitpid. Since a child process can only
be "waitpid"-ed once. It's guaranteed that "pids.remove(p)" won't be called
with duplicated "p"s. And once a "p" is removed from "pids", that "p" does
not need to be killed or waited any more.

Patch

diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -99,7 +99,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.ECHILD:
+                        break # ignore ECHILD
+                    else:
+                        raise
             if p:
+                pids.remove(p)
                 st = _exitstatus(st)
             if st and not problem[0]: