Re: [PATCH 2 of 2 V2] commandserver: handle backlog before exiting
On Fri, 10 Feb 2017 05:14:20 -0800, Jun Wu wrote: > Excerpts from Yuya Nishihara's message of 2017-02-10 22:11:26 +0900: > > Missed setting self._socketunlinked. Perhaps you want self._unlinksocket(). > > > > Should I fix it in flight? > > Sorry! It should be "self._unlinksocket()". Thanks for finding it out! Fixed and pushed. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 V2] commandserver: handle backlog before exiting
Excerpts from Yuya Nishihara's message of 2017-02-10 22:11:26 +0900: > Missed setting self._socketunlinked. Perhaps you want self._unlinksocket(). > > Should I fix it in flight? Sorry! It should be "self._unlinksocket()". Thanks for finding it out! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 V2] commandserver: handle backlog before exiting
On Wed, 8 Feb 2017 14:52:31 -0800, Jun Wu wrote: > # HG changeset patch > # User Jun Wu > # Date 1486593930 28800 > # Wed Feb 08 14:45:30 2017 -0800 > # Node ID cb56fce57eceef2cf4cd9893d387b9fe2b3cecd6 > # Parent 5fc577761fb78168fcbd7ec93d911a1b7b4989c9 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > cb56fce57ece > 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 stop > queuing new connections and processing all pending connections before exit. > > diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py > --- a/mercurial/commandserver.py > +++ b/mercurial/commandserver.py > @@ -478,9 +478,21 @@ class unixforkingservice(object): > > def _mainloop(self): > +exiting = False > h = self._servicehandler > -while not h.shouldexit(): > +while True: > +if not exiting and h.shouldexit(): > +# clients can no longer connect() to the domain socket, so > +# we stop queuing new requests. > +# for requests that are queued (connect()-ed, but haven't > been > +# accept()-ed), handle them before exit. otherwise, clients > +# waiting for recv() will receive ECONNRESET. > +self._servicehandler.unlinksocket(self.address) Missed setting self._socketunlinked. Perhaps you want self._unlinksocket(). Should I fix it in flight? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 V2] commandserver: handle backlog before exiting
On Wed, Feb 08, 2017 at 02:52:31PM -0800, Jun Wu wrote: > # HG changeset patch > # User Jun Wu > # Date 1486593930 28800 > # Wed Feb 08 14:45:30 2017 -0800 > # Node ID cb56fce57eceef2cf4cd9893d387b9fe2b3cecd6 > # Parent 5fc577761fb78168fcbd7ec93d911a1b7b4989c9 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > cb56fce57ece > commandserver: handle backlog before exiting Queued these, thanks. > > 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 stop > queuing new connections and processing all pending connections before exit. > > diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py > --- a/mercurial/commandserver.py > +++ b/mercurial/commandserver.py > @@ -478,9 +478,21 @@ class unixforkingservice(object): > > def _mainloop(self): > +exiting = False > h = self._servicehandler > -while not h.shouldexit(): > +while True: > +if not exiting and h.shouldexit(): > +# clients can no longer connect() to the domain socket, so > +# we stop queuing new requests. > +# for requests that are queued (connect()-ed, but haven't > been > +# accept()-ed), handle them before exit. otherwise, clients > +# waiting for recv() will receive ECONNRESET. > +self._servicehandler.unlinksocket(self.address) > +exiting = True > try: > ready = select.select([self._sock], [], [], h.pollinterval)[0] > if not ready: > +# only exit if we completed all queued requests > +if exiting: > +break > continue > conn, _addr = self._sock.accept() > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 V2] commandserver: handle backlog before exiting
# HG changeset patch # User Jun Wu # Date 1486593930 28800 # Wed Feb 08 14:45:30 2017 -0800 # Node ID cb56fce57eceef2cf4cd9893d387b9fe2b3cecd6 # Parent 5fc577761fb78168fcbd7ec93d911a1b7b4989c9 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r cb56fce57ece 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 stop queuing new connections and processing all pending connections before exit. diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py --- a/mercurial/commandserver.py +++ b/mercurial/commandserver.py @@ -478,9 +478,21 @@ class unixforkingservice(object): def _mainloop(self): +exiting = False h = self._servicehandler -while not h.shouldexit(): +while True: +if not exiting and h.shouldexit(): +# clients can no longer connect() to the domain socket, so +# we stop queuing new requests. +# for requests that are queued (connect()-ed, but haven't been +# accept()-ed), handle them before exit. otherwise, clients +# waiting for recv() will receive ECONNRESET. +self._servicehandler.unlinksocket(self.address) +exiting = True try: ready = select.select([self._sock], [], [], h.pollinterval)[0] if not ready: +# only exit if we completed all queued requests +if exiting: +break continue conn, _addr = self._sock.accept() ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
