You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@attglobal.net> on 2001/07/31 21:14:35 UTC
[PATCH] fix storage leak on persistent connection
The core output filter uses c->pool to store a brigade representing
response data from a request which won't be sent for a while. The
storage for that temporary brigade is not completely freed until
c->pool is cleaned up. This represents a storage leak which will
become a problem if a bazillion requests are issued on that
connection.
It may be that the registered cleanup for the brigade is the entire
leak. I dunno...
With this patch we create a subpool of c->pool to use for the
temporary brigade storage. The subpool will be cleared when we go
through the filter without saving anything in a temporary brigade.
All I'll claim regarding the patch is that it attempts to solve a real
problem and that it apparently removes a storage leak symptom when I
request a static file a bazillion times on a single connection
(no claims of neatness or minimality or anything else :) )
Comments?
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.31
diff -u -r1.31 core.c
--- server/core.c 2001/07/30 18:51:57 1.31
+++ server/core.c 2001/07/31 19:08:15
@@ -3052,6 +3052,10 @@
*/
typedef struct CORE_OUTPUT_FILTER_CTX {
apr_bucket_brigade *b;
+ apr_pool_t *subpool; /* subpool of c->pool used for data saved after a
+ * request is finished
+ */
+ int has_stuff; /* anything in the subpool? */
} core_output_filter_ctx_t;
#define MAX_IOVEC_TO_WRITE 16
@@ -3168,6 +3172,11 @@
if ((!fd && !more &&
(nbytes + flen < AP_MIN_BYTES_TO_WRITE) && !APR_BUCKET_IS_FLUSH(e))
|| (nbytes + flen < AP_MIN_BYTES_TO_WRITE && APR_BUCKET_IS_EOS(e) && c->keepalive)) {
+
+ if (ctx->subpool == NULL) {
+ apr_pool_create(&ctx->subpool, f->c->pool);
+ }
+
/* NEVER save an EOS in here. If we are saving a brigade with
* an EOS bucket, then we are doing keepalive connections, and
* we want to process to second request fully.
@@ -3179,7 +3188,7 @@
* after the request_pool is cleared.
*/
if (ctx->b == NULL) {
- ctx->b = apr_brigade_create(f->c->pool);
+ ctx->b = apr_brigade_create(ctx->subpool);
}
APR_BRIGADE_FOREACH(bucket, b) {
@@ -3204,11 +3213,13 @@
return rv;
}
apr_brigade_write(ctx->b, NULL, NULL, str, n);
+ ctx->has_stuff = 1;
}
apr_brigade_destroy(b);
}
else {
- ap_save_brigade(f, &ctx->b, &b, c->pool);
+ ap_save_brigade(f, &ctx->b, &b, ctx->subpool);
+ ctx->has_stuff = 1;
}
return APR_SUCCESS;
}
@@ -3281,6 +3292,11 @@
b = more;
more = NULL;
} /* end while () */
+
+ if (ctx->subpool && ctx->has_stuff) {
+ apr_pool_clear(ctx->subpool);
+ ctx->has_stuff = 0;
+ }
return APR_SUCCESS;
}
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix storage leak on persistent connection
Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:
> The core output filter uses c->pool to store a brigade representing
> response data from a request which won't be sent for a while. The
> storage for that temporary brigade is not completely freed until
> c->pool is cleaned up. This represents a storage leak which will
> become a problem if a bazillion requests are issued on that
> connection.
I'll plan on committing this after the next tarball. Is anybody
opposed? Does anybody mind if I don't try to keep up with whether or
not anything is in the subpool and simply call apr_pool_clear() in the
normal exit path (i.e., get rid of has_stuff)? I'm afraid that future
updates to the function may not do the right thing w.r.t. has_stuff.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...