Patchwork [7,of,8,V4] worker: add a SIGCHLD handler to collect worker immediately

login
register
mail settings
Submitter Jun Wu
Date Nov. 12, 2016, 3:11 a.m.
Message ID <7492186538a69cdfad28.1478920298@x1c>
Download mbox | patch
Permalink /patch/17513/
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 1478920042 0
#      Sat Nov 12 03:07:22 2016 +0000
# Node ID 7492186538a69cdfad28cb93093d7f7077942ca3
# Parent  596cee9d5c12dcfa2d272b5441d5060a26a6440c
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 7492186538a6
worker: add a SIGCHLD handler to collect worker immediately

As planned by previous patches, add a SIGCHLD handler to get notifications
about worker exits, and deals with worker failure immediately.
Yuya Nishihara - Nov. 14, 2016, 1:15 p.m.
On Sat, 12 Nov 2016 03:11:38 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1478920042 0
> #      Sat Nov 12 03:07:22 2016 +0000
> # Node ID 7492186538a69cdfad28cb93093d7f7077942ca3
> # Parent  596cee9d5c12dcfa2d272b5441d5060a26a6440c
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 7492186538a6
> worker: add a SIGCHLD handler to collect worker immediately
> 
> As planned by previous patches, add a SIGCHLD handler to get notifications
> about worker exits, and deals with worker failure immediately.
> 
> diff --git a/mercurial/worker.py b/mercurial/worker.py
> --- a/mercurial/worker.py
> +++ b/mercurial/worker.py
> @@ -126,8 +126,12 @@ def _posixworker(ui, func, staticargs, a
>                  problem[0] = st
>                  killworkers()
> +    def sigchldhandler(signum, frame):
> +        waitforworkers(blocking=False)
> +    oldchldhandler = signal.signal(signal.SIGCHLD, sigchldhandler)
>      for pargs in partition(args, workers):
>          pid = os.fork()
>          if pid == 0:
>              signal.signal(signal.SIGINT, oldhandler)
> +            signal.signal(signal.SIGCHLD, oldchldhandler)
>              try:
>                  os.close(rfd)
> @@ -147,4 +151,5 @@ def _posixworker(ui, func, staticargs, a
>          signal.signal(signal.SIGINT, oldhandler)
>          t.join()
> +        signal.signal(signal.SIGCHLD, oldchldhandler)

I got strange error:

% hg up --trace
Traceback (most recent call last):
  ...
  File "mercurial/merge.py", line 1187, in applyupdates
    for i, item in prog:
  File "mercurial/worker.py", line 159, in _posixworker
    for line in fp:
IOError: [Errno 0] Error
abort: Error

Maybe something goes wrong in the buffered iterator?
iter(fp.readline, '') worked fine.
Jun Wu - Nov. 14, 2016, 2:18 p.m.
Interesting. Thanks for the very careful review (and also for the
setproctitle series)!

I did some simple manual tests but didn't cover all corners here. I will
investigate.

Excerpts from Yuya Nishihara's message of 2016-11-14 22:15:24 +0900:
> On Sat, 12 Nov 2016 03:11:38 +0000, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1478920042 0
> > #      Sat Nov 12 03:07:22 2016 +0000
> > # Node ID 7492186538a69cdfad28cb93093d7f7077942ca3
> > # Parent  596cee9d5c12dcfa2d272b5441d5060a26a6440c
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 7492186538a6
> > worker: add a SIGCHLD handler to collect worker immediately
> > 
> > As planned by previous patches, add a SIGCHLD handler to get notifications
> > about worker exits, and deals with worker failure immediately.
> > 
> > diff --git a/mercurial/worker.py b/mercurial/worker.py
> > --- a/mercurial/worker.py
> > +++ b/mercurial/worker.py
> > @@ -126,8 +126,12 @@ def _posixworker(ui, func, staticargs, a
> >                  problem[0] = st
> >                  killworkers()
> > +    def sigchldhandler(signum, frame):
> > +        waitforworkers(blocking=False)
> > +    oldchldhandler = signal.signal(signal.SIGCHLD, sigchldhandler)
> >      for pargs in partition(args, workers):
> >          pid = os.fork()
> >          if pid == 0:
> >              signal.signal(signal.SIGINT, oldhandler)
> > +            signal.signal(signal.SIGCHLD, oldchldhandler)
> >              try:
> >                  os.close(rfd)
> > @@ -147,4 +151,5 @@ def _posixworker(ui, func, staticargs, a
> >          signal.signal(signal.SIGINT, oldhandler)
> >          t.join()
> > +        signal.signal(signal.SIGCHLD, oldchldhandler)
> 
> I got strange error:
> 
> % hg up --trace
> Traceback (most recent call last):
>   ...
>   File "mercurial/merge.py", line 1187, in applyupdates
>     for i, item in prog:
>   File "mercurial/worker.py", line 159, in _posixworker
>     for line in fp:
> IOError: [Errno 0] Error
> abort: Error
> 
> Maybe something goes wrong in the buffered iterator?
> iter(fp.readline, '') worked fine.

Patch

diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -126,8 +126,12 @@  def _posixworker(ui, func, staticargs, a
                 problem[0] = st
                 killworkers()
+    def sigchldhandler(signum, frame):
+        waitforworkers(blocking=False)
+    oldchldhandler = signal.signal(signal.SIGCHLD, sigchldhandler)
     for pargs in partition(args, workers):
         pid = os.fork()
         if pid == 0:
             signal.signal(signal.SIGINT, oldhandler)
+            signal.signal(signal.SIGCHLD, oldchldhandler)
             try:
                 os.close(rfd)
@@ -147,4 +151,5 @@  def _posixworker(ui, func, staticargs, a
         signal.signal(signal.SIGINT, oldhandler)
         t.join()
+        signal.signal(signal.SIGCHLD, oldchldhandler)
         status = problem[0]
         if status: