[PATCH stable] commandserver: handle backlog before exiting

Jun Wu quark at fb.com
Tue Jan 31 04:52:51 EST 2017


Excerpts from Jun Wu's message of 2017-01-30 16:52:46 +0000:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1485794838 0
> #      Mon Jan 30 16:47:18 2017 +0000
> # Node ID 0f9c5c49ad7ac321f6fecb4ca90121969bb038d4
> # Parent  ea5353feeec3c2eddd7e4acfd398baddaf37b3e4
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 0f9c5c49ad7a
> commandserver: handle backlog before exiting
> 
> Previously, when a chg server is exiting, it does not handle connected
> clients so clients may get ECONNRESET and crash:
> 
>   1. client connect() # success
>   2. server shouldexit = True and exit
>   3. client recv() # ECONNRESET
> 
> d7875bfbfccb makes this race condition easier to reproduce if a lot of short
> chg commands are started in parallel.
> 
> This patch fixes the above issue by unlinking the socket path to prevent new
> connections and handling all pending connections before exiting.
> 
> diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
> --- a/mercurial/commandserver.py
> +++ b/mercurial/commandserver.py
> @@ -412,5 +412,9 @@ class unixservicehandler(object):
>  
>      def unlinksocket(self, address):
> -        os.unlink(address)
> +        try:
> +            os.unlink(address)
> +        except OSError as ex:
> +            if ex.errno != errno.ENOENT:
> +                raise

This introduces another race condition that unlinks an innocent socket file.
I will send a V2 later.

>  
>      def printbanner(self, address):
> @@ -471,9 +475,17 @@ class unixforkingservice(object):
>  
>      def _mainloop(self):
> +        exiting = False
>          h = self._servicehandler
> -        while not h.shouldexit():
> +        while True:
> +            if not exiting and h.shouldexit():
> +                # prevent accepting new connections
> +                self._servicehandler.unlinksocket(self.address)
> +                exiting = True
>              try:
>                  ready = select.select([self._sock], [], [], h.pollinterval)[0]
>                  if not ready:
> +                    # only exit if we have no more connections to handle
> +                    if exiting:
> +                        break
>                      continue
>                  conn, _addr = self._sock.accept()


More information about the Mercurial-devel mailing list