Re: request_header_add adds a memory leak

On 20/07/2012 12:34 p.m., Alex Rousskov wrote:
> Hello,
>
> I believe the current trunk is profusely leaking memory. There may
> be several reasons, but my current suspect is a circular RefCounted
> reference that allows neither HttpRequest nor AccessLogEntry to be freed:
>
> HttpRequest -> AccessLogEntry -> HttpRequest
>
> The circle was created by the request_header_add feature during the
> review process, when it was decided that we should reuse the Format API
> to expand macros in the added header values. That API needs
> AccessLogEntry. This is good news -- there is not much code relying on
> the circle existence!
>
> The bad news is that breaking the circle is not trivial. Copying
> AccessLogEntry to HttpRequest (instead of linking the two) is not a good
> option because the AccessLogEntry object needs to be populated with
> values collected throughout the master transaction lifetime -- up until
> httpHdrAdd() is called on the server side (and much later when we start
> supporting macros in mangled responses).

However, when the transaction reaches termination and logging is
completed it becomes no longer useful (next transaction must re-fill
with new details specific to that transaction).

A clear() method on the entry would be good for resetting all its
pointers after logging completion.

>
> The mess stems from the fact that AccessLogEntry is not generally
> available on the server side. HttpRequest is available there but it
> cannot be used for AccessLogEntry smuggling (due to the above loop).
>
> Unless I hear better ideas, I will extend the FwdState::fwdStart() API
> to supply an AccessLogEntry (and store it in Server/etc). This may even
> help with future plans of converting ALE into some kind of MasterXaction
> class.

Both would be good loong-term, but up to you which gets chosen. I'm
inclined to leave the API changes until xaction project can be started
properly.

Amos
Received on Fri Jul 20 2012 - 02:40:51 MDT