Re: [PATCH] Bug 2976: invalid URL on intercepted requests during reconfigure

 On Wed, 09 Mar 2011 11:13:54 -0700, Alex Rousskov wrote:
> On 03/02/2011 02:19 PM, Amos Jeffries wrote:
>>
>> While investigating bug 2976 (URLs suddenly becoming (null)://foo/
>> on
>> intercepted traffic for a few seconds) it has become clear that the
>> listening ports are badly abusing cbdata.
>>
>> They *are* passed as cbdata parameters from IPC to the connection
>> accepting routines. However the parser is not aware of their
>> cbdata'ness
>> and is using new/delete directly on the config pointers.
>
> Ouch. For the record, could you please paste the offending code that
> "is
> using new/delete directly on the config pointers" if you still
> remember
> where those offenders were?

 in src/cache_cf.cc parse_http_port_list / free_http_port_list
  - noting that the http_port_list struct is a CBDATA_CLASS2()
    with locks set by all active ConnStatedata at the point
 parseDoConfigure() calls releaseConfig() -> free_http_port_list()

>
>> Long term I think the right solution is to RefCount the
>> http_port_list
>> object. Since their main property seems to be that the ConnStateData
>> retain a long-term pointers to them despite SquidConfig
>> disappearing.
>
> It would be nice to come up with a unified approach to
> [re]configuration
> for the long-term fix. A lot of code, from adaptation to storage,
> does
> not support [re]configuration [well] because of similar problems.

 The HotConf project proposal fixed that with its starting/.../stopped
 reconfigure calls, but we never got it widely implemented in the
 existing config code. Though I notice happily that some of your recent
 features have used that model to reconfigure.

>
> I think we should
> (a) make SquidConfig refcountable and its small pieces copy-able;
> (b) make SquidConfig or any separately stored pieces constant;
> (c) remove any state-changing code from the configuration code.
>
> The configuration code should only be concerned with
> creating/parsing,
> reporting, manipulating, and cleaning configuration options. One
> should
> be able to copy small and efficiently share large configuration
> objects,
> without worrying about reconfiguration wiping out or modifying those
> objects. One should also be able to create several complete
> configuration objects without any effect on Squid state (other than
> RAM
> usage).
>

 Amos
Received on Wed Mar 09 2011 - 22:28:43 MST