[PATCH 8 of 8 V5] worker: stop using a separate thread waiting for children

Jun Wu quark at fb.com
Wed Nov 16 14:49:24 EST 2016


Excerpts from Martin von Zweigbergk's message of 2016-11-16 11:17:03 -0800:
> On Mon, Nov 14, 2016 at 6:39 PM, Jun Wu <quark at fb.com> wrote:
> > # HG changeset patch
> > # User Jun Wu <quark at fb.com>
> > # Date 1478919967 0
> > #      Sat Nov 12 03:06:07 2016 +0000
> > # Node ID 977775ff43115c001579973ef09581f102b1b842
> > # Parent  3a463b68088ed4721b0b0a33504143b7eff65ade
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 977775ff4311
> > worker: stop using a separate thread waiting for children
> 
> Just to make sure you didn't miss 9955fc5ee24b (worker: handle worker
> failures more aggressively, 2013-02-20), which has this in the
> message:
> 
> "We now wait for worker processes in a separate thread, so that we can
> spot failures in a timely way, wihout waiting for the progress pipe
> to drain."

That is not an issue - We get "timely" notification (SIGCHLD signal) when a
child crashes when the parent is "waiting for the progress pipe to drain",
and the SIGCHLD handler will kill the workers immediately.

Beware that without "[PATCH V3] util: improve iterfile so it chooses code
path wisely", Python < 2.7.4 could be broken.

This series should be a net win. The old code could have serious issues when
other extensions spawns child processes on their own.

> I haven't followed the discussion on this series, so I'm sorry if this
> has already been brought up. I also don't know much about how
> worker.py works, I just happened to see that commit message and
> thought it sounded like you're undoing that and wanted to make sure
> you had seen that commit. I have no opinion about it myself; if you
> tell me it's not an issue, I will happily take your word for it.
> 
> >
> > Now that we have a SIGCHLD hander, and it could get executed when waiting
> > for I/O. It's no longer necessary to have a separated waitpid thread. So
> > just remove it.
> >
> > diff --git a/mercurial/worker.py b/mercurial/worker.py
> > --- a/mercurial/worker.py
> > +++ b/mercurial/worker.py
> > @@ -13,5 +13,4 @@ import os
> >  import signal
> >  import sys
> > -import threading
> >
> >  from .i18n import _
> > @@ -144,9 +143,7 @@ def _posixworker(ui, func, staticargs, a
> >      os.close(wfd)
> >      fp = os.fdopen(rfd, 'rb', 0)
> > -    t = threading.Thread(target=waitforworkers)
> > -    t.start()
> >      def cleanup():
> >          signal.signal(signal.SIGINT, oldhandler)
> > -        t.join()
> > +        waitforworkers()
> >          signal.signal(signal.SIGCHLD, oldchldhandler)
> >          status = problem[0]
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel 


More information about the Mercurial-devel mailing list