[PATCH 3 of 3] commandserver: backport handling of forking server from chgserver

Jun Wu quark at fb.com
Mon Jul 4 13:43:04 EDT 2016


This series looks good to me. Looking forward to next steps.

Note: Sorry for the delay. I was planning to review this last week but got
addicted to improve my email tooling [1] (and it's a bit interesting now).
Finally I have the one-key-to-apply-all-patches-in-current-thread feature,
which should help me with reviewing in the future :)

[1]: https://github.com/quark-zju/sup

Excerpts from Yuya Nishihara's message of 2016-06-30 23:26:55 +0900:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1463811801 -32400
> #      Sat May 21 15:23:21 2016 +0900
> # Node ID 0afc4036f1732699683814cb2a90376e7bccae2a
> # Parent  b00830fdf008c83d361b05491b023778275dfd61
> commandserver: backport handling of forking server from chgserver
> 
> This is common between chg and vanilla forking server, so move it to
> commandserver and unify handle().
> 
> It would be debatable whether we really need gc.collect() or not, but that
> is beyond the scope of this series. Maybe we can remove gc.collect() once
> all resource deallocations are switched to context manager.
> 
> diff --git a/hgext/chgserver.py b/hgext/chgserver.py
> --- a/hgext/chgserver.py
> +++ b/hgext/chgserver.py
> @@ -41,18 +41,15 @@ Config
>  from __future__ import absolute_import
>  
>  import errno
> -import gc
>  import hashlib
>  import inspect
>  import os
> -import random
>  import re
>  import signal
>  import struct
>  import sys
>  import threading
>  import time
> -import traceback
>  
>  from mercurial.i18n import _
>  
> @@ -531,46 +528,7 @@ class chgcmdserver(commandserver.server)
>                           'setenv': setenv,
>                           'setumask': setumask})
>  
> -# copied from mercurial/commandserver.py
> -class _requesthandler(socketserver.StreamRequestHandler):
> -    def handle(self):
> -        # use a different process group from the master process, making this
> -        # process pass kernel "is_current_pgrp_orphaned" check so signals like
> -        # SIGTSTP, SIGTTIN, SIGTTOU are not ignored.
> -        os.setpgid(0, 0)
> -        # change random state otherwise forked request handlers would have a
> -        # same state inherited from parent.
> -        random.seed()
> -        ui = self.server.ui
> -        sv = None
> -        try:
> -            sv = self._createcmdserver()
> -            try:
> -                sv.serve()
> -            # handle exceptions that may be raised by command server. most of
> -            # known exceptions are caught by dispatch.
> -            except error.Abort as inst:
> -                ui.warn(_('abort: %s\n') % inst)
> -            except IOError as inst:
> -                if inst.errno != errno.EPIPE:
> -                    raise
> -            except KeyboardInterrupt:
> -                pass
> -            finally:
> -                sv.cleanup()
> -        except: # re-raises
> -            # also write traceback to error channel. otherwise client cannot
> -            # see it because it is written to server's stderr by default.
> -            if sv:
> -                cerr = sv.cerr
> -            else:
> -                cerr = commandserver.channeledoutput(self.wfile, 'e')
> -            traceback.print_exc(file=cerr)
> -            raise
> -        finally:
> -            # trigger __del__ since ForkingMixIn uses os._exit
> -            gc.collect()
> -
> +class _requesthandler(commandserver._requesthandler):
>      def _createcmdserver(self):
>          ui = self.server.ui
>          repo = self.server.repo
> diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
> --- a/mercurial/commandserver.py
> +++ b/mercurial/commandserver.py
> @@ -8,7 +8,9 @@
>  from __future__ import absolute_import
>  
>  import errno
> +import gc
>  import os
> +import random
>  import struct
>  import sys
>  import traceback
> @@ -338,6 +340,13 @@ class pipeservice(object):
>  
>  class _requesthandler(socketserver.StreamRequestHandler):
>      def handle(self):
> +        # use a different process group from the master process, making this
> +        # process pass kernel "is_current_pgrp_orphaned" check so signals like
> +        # SIGTSTP, SIGTTIN, SIGTTOU are not ignored.
> +        os.setpgid(0, 0)
> +        # change random state otherwise forked request handlers would have a
> +        # same state inherited from parent.
> +        random.seed()
>          ui = self.server.ui
>          sv = None
>          try:
> @@ -364,6 +373,9 @@ class _requesthandler(socketserver.Strea
>                  cerr = channeledoutput(self.wfile, 'e')
>              traceback.print_exc(file=cerr)
>              raise
> +        finally:
> +            # trigger __del__ since ForkingMixIn uses os._exit
> +            gc.collect()
>  
>      def _createcmdserver(self):
>          ui = self.server.ui


More information about the Mercurial-devel mailing list