On 07/16/2014 02:38 PM, Rainer Weikusat wrote: > Alex Rousskov <rouss...@measurement-factory.com> writes: >> On 07/16/2014 11:11 AM, Rainer Weikusat wrote: >>> This is broken because it re-enables readable notifications even if no >>> connection was accepted.
>> In other words, it re-enables a check for new connections when >> >> * we know that we currently cannot accept any new connections, >> * Squid is under stress already, and >> * our code does not handle multiple deferences of the same TcpAcceptor >> object well. > There's an important 4th point: The check is re-enabled when it is known > that it will immediately return true again because the connection which > was not accepted is still pending (hence, the socket is still readable), > more details below. Agreed! > I changed the TcpAcceptor/ AcceptLimiter code to act as if > only a single file descriptor was available for client connections and > created two connections via netcat. What it should do in this case is to > ignore the second connection until the first one was closed. Instead, it > went into an endless 'defer this connection' loop, as mentioned above, > because the pending 2nd connection meant the socket remained readable. OK, I think I now better understand what you meant by "one connection at the same time". And even if I do not, we seem to agree on the fix to solve the problem, which is even more important :-). > updated patch: > > --- mad-squid/src/comm/TcpAcceptor.cc 8 Jan 2013 17:36:44 -0000 1.1.1.2 > +++ mad-squid/src/comm/TcpAcceptor.cc 16 Jul 2014 20:36:35 -0000 > @@ -204,7 +204,6 @@ > } else { > afd->acceptNext(); > } > - SetSelect(fd, COMM_SELECT_READ, Comm::TcpAcceptor::doAccept, afd, 0); > > } catch (const std::exception &e) { > fatalf("FATAL: error while accepting new client connection: %s\n", > e.what()); > @@ -262,6 +261,8 @@ > debugs(5, 5, HERE << "Listener: " << conn << > " accepted new connection " << newConnDetails << > " handler Subscription: " << theCallSub); > + > + if (!isLimited) SetSelect(conn->fd, COMM_SELECT_READ, > Comm::TcpAcceptor::doAccept, this, 0); > notify(flag, newConnDetails); > } I am happy to commit the above (after splitting the new if-statement into two lines), but since this is not an emergency, I would prefer to wait a little for any objections or second opinions. Thank you, Alex.