[squid-dev] Security::SessionPointer and Security::LockingPointer

[Amos Jeffries]

On 10/02/2016 5:53 a.m., Christos Tsantilas wrote:
> Hi all,
> 
> The short question:
> The Security::SessionPointer is a TidyPointer. Is it acceptable to
> convert it to a LockingPointer?

Only with a reliable reference counting/locking mechanism.

We get away with it on contexts only because contexts are anchored in
permanent globals (ie PortCfg list, ::Config etc) and the deinit()
function does its reference counting relative to active sessions when we
do a -k reconfigure change.

With session_t we dont have any lifesaving anchor and have to use a
slightly different free()-like call that risks totally releasing memory
too early.

I added the fd_table adjustment recently to get us a some stability
anchor, but its not enough to be fully lock-safe in relation to
outstanding Calls or connection close handlers. We still need the code
to pass around the Pointer& instead of Ptr and avoid having more than
one Pointer for each session_t existing at any time.

> 
> My sense is that the GnuTLS does not support locking pointers?
> However I have an idea about how we can add locking support for
> gnutls_session_t if required.
> 

I have not been able to find any exposed refcount/locks we can use in
GnuTLS, and been procrastinating on contacting Nikos about it. GnuTLS
seems to handle threads and SMP internally well enough. But not async
events like our Calls.

I am *very* interested in that locking idea of yours.


>   The Security::SessionPointer is the old Ssl::SSL_Pointer and used to
> be a TidiPointer.

Yes.

> However while fixing the memory leak bug reported and analysed under the
> "[squid-dev] NotePairs, SSL and Cert Validation memory leaks" mail
> thread, I made a patch which converted this pointer to a LockingPointer.
> 
> If we can not convert it to locking pointer I need to reimplement the
> patch. However using a locking pointer for SSL may help us in many other
> cases too.
> 

Yes I noticed that. What I've done as a halfway measure is to make the
API of both the Locking and TidyPointers identical. So as a last resort
we could make the OpenSSL one a LockingPointer and the GnuTLS a
TidyPointer. But that does not solve the leak issue for GnuTLS builds.

Amos