On 05/08/2014 11:49 AM, Amos Jeffries wrote: >>> Something is still calling serverConn->close() directly
In your particular trace, HttpStateData::continueAfterParsingHeader() does that, but there are many other methods doing the same thing. AFAICT, the idea is that the close handlers will clean up during connection closure, including the FwdState close handler. While the whole FwdState/Server integration is an ugly mess, that close() calling code in ServerStateData and its kids is not at fault here. > instead of via the FwdState::closeServer() method. I am guessing you are thinking about FwdState::closeServerConnection(). It is a private method encapsulating repeated code internal to FwdState. It was not meant to be, cannot, and should not be used outside FwdState itself "as is". >> Do you know how a connection close callback (fwdServerClosedWrapper) can >> be called with fd set to -1? It feels like there is a shared Connection >> object that two different jobs are updating/closing independently, >> stepping on each other's toes. Actually, I bet CommCloseCbParams.fd is just never set. commCallCloseHandlers() is supposed to set it, but it does not. This is a bug, but I suspect most close handlers do not care about the descriptor value so they continue to work fine. r13388 (standy connections) was not that lucky. I added the following check to update the pconn stats for the closing connection: > if (serverConnection()->fd == fd) // should be, but not critical to assert > fwdPconnPool->noteUses(fd_table[fd].pconn.uses); That new check does not work as intended because both fds are -1 in this case: the first one is reset by http.cc code that shares Connection object with FwdState and the second is never set by Comm. The above noteUses() call was moved from Comm (where it did not belong in the first place and caused other troubles after we added non-static pconn pools). Here is that old Comm code calling noteUses() for static pconn pools: > commCallCloseHandlers(fd); > > - if (F->pconn.uses && F->pconn.pool) > - F->pconn.pool->noteUses(F->pconn.uses); I have committed a short-term fix for this bug (r13404). Here is updated FwdState::serverClosed(): > - if (serverConnection()->fd == fd) // should be, but not critical to > assert > + if (fd >= 0 && serverConnection()->fd == fd) // XXX: fd is often -1 here > fwdPconnPool->noteUses(fd_table[fd].pconn.uses); This change should prevent serverClosed() segfaults, but it is wrong. For the correct solution, I am considering moving pconn.uses from fd_table to Connection. What do you think? Thank you, Alex.
