Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Sender: trawick@rdu26-58-158.nc.rr.com
To: new-httpd@apache.org
Subject: [PATCH] fix storage leak on persistent connection
From: Jeff Trawick <trawick@attglobal.net>
Date: 31 Jul 2001 15:14:35 -0400
Message-ID: <m366c8vrl0.fsf@rdu26-58-158.nc.rr.com>
Lines: 92
User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.3
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

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...