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

[Christos Tsantilas]

On 02/09/2016 07:31 PM, Amos Jeffries wrote:
> 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.

I think it is simple to implement it.

The GnuTls API allow to attach user data to the gnutls_session_t 
objects. Someone can use the gnutls_session_set_ptr 
()/gnutls_session_get_ptr () library funtions for this.
We can attach a counter to gnutls_session_set_ptr which increased in 
Security::LockingPointer::resetAndLock() method and decreased in free 
function passed to Security::LockingPointer.


>
>
>>    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
>