You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Justin Erenkrantz <ju...@erenkrantz.com> on 2011/05/12 11:24:25 UTC
Re: 1.7 Performance via HTTP
[ adding dev@httpd ]
On Thu, May 12, 2011 at 12:25 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Thu, May 12, 2011 at 11:18, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
>> On Thu, May 12, 2011 at 12:16 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> Unfortunately huge memory leak in mod_deflate/mod_dav prevents
>>> enabling gzip on compression on production servers, especially on
>>> Windows where single process mode is used:
>>> http://svn.haxx.se/dev/archive-2009-08/0274.shtml
>>
>> *shrug*
>>
>> Sounds like that would be a good thing to work on in either Dublin or
>> Berlin. Again, the fix is pretty straightforward... -- justin
>>
> It would be great! Since I'm not familiar with this part of codebase
> and cannot fix it myself.
This patch should fix the memory leak. As I mentioned in a follow-up,
we can discuss in Dublin or Berlin if there is a better way to solve
this dangling filter reference...but, this will do the trick and
should be back-portable to 2.2 (and 2.0 too, I guess). -- justin
Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c (revision 1102210)
+++ modules/filters/mod_deflate.c (working copy)
@@ -303,6 +303,7 @@ typedef struct deflate_ctx_t
unsigned char *validation_buffer;
apr_size_t validation_buffer_length;
int inflate_init;
+ int filter_init;
} deflate_ctx;
/* Number of validation bytes (CRC and length) after the compressed data */
@@ -445,6 +446,8 @@ static apr_status_t deflate_out_filter(ap_filter_t
char *token;
const char *encoding;
+ ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
+
/*
* Only work on main request, not subrequests,
* that are not a 204 response with no content
@@ -563,7 +566,7 @@ static apr_status_t deflate_out_filter(ap_filter_t
*/
if (r->status != HTTP_NOT_MODIFIED) {
- ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
+ ctx->filter_init = 1;
ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
ctx->buffer = apr_palloc(r->pool, c->bufferSize);
ctx->libz_end_func = deflateEnd;
@@ -583,6 +586,7 @@ static apr_status_t deflate_out_filter(ap_filter_t
* We are not able to init libz and pass data down the chain
* uncompressed.
*/
+ ctx->filter_init = 0;
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, bb);
}
@@ -629,6 +633,11 @@ static apr_status_t deflate_out_filter(ap_filter_t
/* initialize deflate output buffer */
ctx->stream.next_out = ctx->buffer;
ctx->stream.avail_out = c->bufferSize;
+ } else if (!ctx->filter_init) {
+ /* Hmm. We've run through the filter init before as we have a ctx,
+ * but we never initialized. We probably have a dangling ref. Bail.
+ */
+ return ap_pass_brigade(f->next, bb);
}
while (!APR_BRIGADE_EMPTY(bb))
Re: 1.7 Performance via HTTP
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, May 12, 2011 at 9:24 AM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> This patch should fix the memory leak. As I mentioned in a follow-up,
> we can discuss in Dublin or Berlin if there is a better way to solve
> this dangling filter reference...but, this will do the trick and
> should be back-portable to 2.2 (and 2.0 too, I guess). -- justin
Committed a slight variant of this to httpd trunk in r1103315. -- justin
Re: 1.7 Performance via HTTP
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, May 12, 2011 at 9:24 AM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> This patch should fix the memory leak. As I mentioned in a follow-up,
> we can discuss in Dublin or Berlin if there is a better way to solve
> this dangling filter reference...but, this will do the trick and
> should be back-portable to 2.2 (and 2.0 too, I guess). -- justin
Committed a slight variant of this to httpd trunk in r1103315. -- justin