facebook/proxygen

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.

}

afrind commented

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?