Re: [PATCH 5/8] reconfiguration leaks: objects tied to http_port

On 14/06/2014 8:07 a.m., Alex Rousskov wrote:
> On 04/25/2014 02:59 AM, Amos Jeffries wrote:
>> On 25/04/2014 12:55 p.m., Alex Rousskov wrote:
>>> Do not leak [SSL] objects tied to http_port and https_port on reconfigure.
>>>
>>> PortCfg objects were not destroyed at all (no delete call) and were
>>> incorrectly stored (excessive cbdata locking). This change adds
>>> destruction and removes excessive locking to allow the destructed
>>> object to be freed. It also cleans up forgotten(?) clientca and crlfile
>>> PortCfg members.
>>>
>>> This change fixes a serious leak but also carries an elevated risk:
>>> There is a lot of code throughout Squid that does not check the pointers
>>> to the objects that are now properly destroyed. It is possible that some
>>> of that code will crash some time after reconfigure. It is not possible
>>> to ensure that this does not happen without rewriting/fixing the
>>> offending code to use refcounting. Such a rewrite would be a relatively
>>> large change outside this patch scope. We may decide that it is better
>>> to leak than to take this additional risk.
>>>
>>> Alex.
>>>
>>
>> -0.
>>
>> I have a patch moving the SSL config options into a standalone
>> ref-counted object. That can be polished up and references added to each
>> ConnStateData fairly easily.
>
> Amos, what is the status of that patch? Any ETA? Do you expect your
> changes to be easily portable to v3.3?

Stalled behind the larger works. If it is urgent I can did it out and
polish it up.

It could be back-ported to 3.3 if you like. The design is a new
Ref-Countable class to hold all the SSL options (and generated state)
leaving just a Pointer to it in the main config class.
 * Ports which needed a clone operation took a copy of the pointer and
share the context.
 * client/server context initialization functions take a Pointer to the
class and update its state content.

Amos
Received on Sat Jun 14 2014 - 01:25:32 MDT