httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: Do pools lead to bad programming?
Date Thu, 12 Dec 2013 00:15:49 GMT
On 12 Dec 2013, at 2:00 AM, Kean Johnston <kean.johnston@gmail.com> wrote:

> So I've been spending a fair bit of time inside Apache recently and I've seen a pattern.
Consider the following code (from mod_proxy_fcgi.c):
> 
>    apr_uri_t *uri = apr_palloc(r->pool, sizeof(*uri));
> 
>    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01076)
>                  "url: %s proxyname: %s proxyport: %d",
>                 url, proxyname, proxyport);
> 
>    if (strncasecmp(url, "fcgi:", 5) != 0) {
>        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01077) "declining URL %s",
url);
>        return DECLINED;
>    }
> 
> That allocation of uri can be moved down (further than the code shown above) until right
before it is used. I've seen this in a number of places and it "feels" like it is considered
OK because the pool is relatively short lived and in most cases I've seen so far the allocation
is pretty small. But as a concept, this strikes me as bad. If that code was using a traditional
malloc/free the above would be a memory leak. I am aware that pools provide protection against
such leaks, but nonetheless, that is wasted memory allocated and although quick, also a waste
of time.

The idea behind pools is that you allocate an arbitrary and unpredictable set of memory, and
then free the whole set at some future point in time. This means you don't need to keep track
of each and every escape path out of a system and hope you've cleaned up everything, you allocate
at will confident that it will all be freed.

Obviously allocating too early and then throwing away the results of the allocation is a waste
as you've pointed out, and should ideally be smoked out and fixed.

> Am I being too obsessive? If not, would you like patches to correct these as I find them,
and if so, should I open a bug about this or just post patches here (they are all likely to
be a simple move of 1 or 2 lines)?

I'd love to see these things fixed, because they add up. If you post them here they are likely
to be reviewed very quickly, as they'll no doubt be simple to review.

Regards,
Graham
--


Mime
View raw message