[PATCH stable] commandserver: handle backlog before exiting

Yuya Nishihara yuya at tcha.org
Tue Jan 31 10:48:26 EST 2017


On Tue, 31 Jan 2017 15:24:42 +0000, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-01-31 23:36:46 +0900:
> > > This introduces another race condition that unlinks an innocent socket file.
> > > I will send a V2 later.
> 
> (This actually seems to be a regression after the socketserver refactoring.
> It probably requires sub-classing "unixforkingservice" in chg to solve
> properly. It's not too trivial and I'll send fixes after the freeze.)

I don't think SocketServer does anything special for flushing backlog.

> > I have no idea if we can reliably make the socket stop queuing new connections
> > without destroying the backlog.
> > 
> > Instead, maybe the client can handle very first ECONNRESET gracefully?
> 
> After "unlink", the client cannot "connect" by a filesystem address. And chg
> client does connect by a filesystem address. So it should be reliable (not
> considering Linux-specific black magic /proc/random-pid/fd/n).

Yeah, but you found there's another race.

> I think the "industry standard" about shutting down is to handle all
> remaining requests while rejecting new ones. The pattern is seen in nginx,
> squid, thin (a Ruby on Rails server), OpenStack APIs etc.

If they do, we can copy. Can you investigate how they solve the problem?
I tried if shutdown(SHUT_RD) could be used, but it didn't help as expected.

> It looks better
> than retrying in the client.

Yes, but we have retry for connect(), so adding retry for the first recv()
wouldn't make things much worse.


More information about the Mercurial-devel mailing list