ServerIdleSessionConroller:getIdleSession() returns session already in thread
mikekliger opened this issue · 5 comments
Hi! Im dealing with another subtle bug that I think is leaking sockets/cnxs in prod tests.
I am migrating to proxygen conn pool with thread transferring capabilties.
The issue is when I transfer from another thread via getIdleSession, it returns a session that is somehow already in the evb thread of the calling session pool. This causes an issue since detachThreadLocals() is already called on the session. From the examples given, there is no handling of this case. I believe this is causing my program to leak connections, since the connection will not be attached to the calling thread in this case.
Two questions:
Why is getIdleSession() returning a session in the same thread.
How can I handle this case? Should I be attaching it to the calling session pool no matter what?
Here is a crude breakdown of the business logic which is adapted from the examples posted here and in other issues
session = getIdleSession()
if(session)
evb = evb thread of session
if(!evb or !evb->isinEvbThread) ->>> this seems to be failing almost every time
{
... attach session to the current threadlocal session pool and add connection to TLUS connection manager
}
else {
do nothing... this no op seems to result in a leak.
}
Are you calling SessionPool::getTransaction first? It should track all the idle sessions in the current thread and give one to you before you go to the ServerIdleSessionController.
https://github.com/facebook/proxygen/blob/main/proxygen/lib/http/connpool/SessionPool.cpp#L101
Yes always
I am seeing this connection error in mass after some time fwiw, which may just be a symptom of the leaking. I can confirm that attachThreadLocals() isn't getting called (while detach must be getting called)
AsyncSocketException: failed to create socket (peer=<uninitialized address>, local=<uninitialized address>), type = Internal error, errno = 97 (Address family not supported by protocol)
So I can confirm HTTPUpstreamSession::AttachThreadLocals() is being called, but nothing after it is being logged. I am doing a local build to see if AttachThreadLocals() is perhaps silently failing
It seems to be failing on the codec_.forEach(filterFn)
Is it improper to pass nullptr to filterFn?