* Use a separate subpool to manage the data for the socket and the connection

  member of the proxy_conn_rec struct as we destroy this data more frequently
  than other data in the proxy_conn_rec struct like hostname and addr (at least
  in the case where we have keepalive connections that timed out and were
  closed by the backend).
  This fixes a memory leak with short lived and broken connections.

PR: 44026

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@603237 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 77510e63ae883f251c59a94ee20beee2b8e7c581..ad62ffa3ca70fc1253a5692c99cd924864fd221f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) mod_proxy: Lower memory consumption for short lived connections.
+     PR 44026. [Ruediger Pluem]
+
   *) core: Lower memory consumption of ap_r* functions by reusing the brigade
      instead of recreating it during each filter pass.
      [Stefan Fritsch <sf sfritsch.de>]

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 24346cb0843c76ef80332b1b505ba127f2ee39ee..2bfe1fc22410bc23728f8ec3070e68c66eadc368 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -144,6 +144,7 @@
  * 20071108.3 (2.3.0-dev)  Add API guarantee for adding connection filters
  *                         with non-NULL request_rec pointer (ap_add_*_filter*)
  * 20071108.4 (2.3.0-dev)  Add ap_proxy_ssl_connection_cleanup
+ * 20071108.5 (2.3.0-dev)  Add *scpool to proxy_conn_rec structure
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -151,7 +152,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20071108
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 4                    /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 5                    /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h
index 27ac89f2d9b1123bc16a5089f87054cf2347a725..84c6b261c7358bbc6387fa2afd4c39a47b6ade21 100644 (file)
--- a/modules/proxy/mod_proxy.h
+++ b/modules/proxy/mod_proxy.h
@@ -230,7 +230,7 @@ typedef struct {
     const char   *hostname;
     apr_port_t   port;
     int          is_ssl;
-    apr_pool_t   *pool;     /* Subpool used for creating socket */
+    apr_pool_t   *pool;     /* Subpool for hostname and addr data */
     apr_socket_t *sock;     /* Connection socket */
     apr_sockaddr_t *addr;   /* Preparsed remote address info */
     apr_uint32_t flags;     /* Conection flags */
@@ -240,6 +240,7 @@ typedef struct {
 #if APR_HAS_THREADS
     int          inreslist; /* connection in apr_reslist? */
 #endif
+    apr_pool_t   *scpool;   /* Subpool used for socket and connection data */
 } proxy_conn_rec;
 
 typedef struct {

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 91f83852539505c20806bebc623ca475f5bb5c06..f6f3fd084642807c9efa644ba34286f3a8b731e5 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -1600,12 +1600,13 @@ static apr_status_t connection_cleanup(void *theconn)
     if (conn->close) {
         apr_pool_t *p = conn->pool;
         if (conn->connection) {
-            apr_pool_cleanup_kill(conn->pool, conn, connection_cleanup);
+            apr_pool_cleanup_kill(p, conn, connection_cleanup);
         }
-        apr_pool_clear(conn->pool);
+        apr_pool_clear(p);
         memset(conn, 0, sizeof(proxy_conn_rec));
         conn->pool = p;
         conn->worker = worker;
+        apr_pool_create(&(conn->scpool), p);
     }
 #if APR_HAS_THREADS
     if (worker->hmax && worker->cp->res) {
@@ -1622,17 +1623,11 @@ static apr_status_t connection_cleanup(void *theconn)
     return APR_SUCCESS;
 }
 
-static apr_status_t socket_cleanup(proxy_conn_rec *conn)
+static void socket_cleanup(proxy_conn_rec *conn)
 {
-    if (conn->sock) {
-        apr_socket_close(conn->sock);
-        conn->sock = NULL;
-    }
-    if (conn->connection) {
-        apr_pool_cleanup_kill(conn->pool, conn, connection_cleanup);
-        conn->connection = NULL;
-    }
-    return APR_SUCCESS;
+    conn->sock = NULL;
+    conn->connection = NULL;
+    apr_pool_clear(conn->scpool);
 }
 
 PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup(proxy_conn_rec *conn,
@@ -1675,6 +1670,7 @@ static apr_status_t connection_constructor(void **resource, void *params,
                                            apr_pool_t *pool)
 {
     apr_pool_t *ctx;
+    apr_pool_t *scpool;
     proxy_conn_rec *conn;
     proxy_worker *worker = (proxy_worker *)params;
 
@@ -1684,9 +1680,18 @@ static apr_status_t connection_constructor(void **resource, void *params,
      * when disconnecting from backend.
      */
     apr_pool_create(&ctx, pool);
+    /*
+     * Create another subpool that manages the data for the
+     * socket and the connection member of the proxy_conn_rec struct as we
+     * destroy this data more frequently than other data in the proxy_conn_rec
+     * struct like hostname and addr (at least in the case where we have
+     * keepalive connections that timed out).
+     */
+    apr_pool_create(&scpool, ctx);
     conn = apr_pcalloc(pool, sizeof(proxy_conn_rec));
 
     conn->pool   = ctx;
+    conn->scpool = scpool;
     conn->worker = worker;
 #if APR_HAS_THREADS
     conn->inreslist = 1;
@@ -2164,11 +2169,6 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function,
         (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
 
     if (conn->sock) {
-        /*
-         * This increases the connection pool size
-         * but the number of dropped connections is
-         * relatively small compared to connection lifetime
-         */
         if (!(connected = is_socket_connected(conn->sock))) {
             socket_cleanup(conn);
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
@@ -2179,7 +2179,7 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function,
     while (backend_addr && !connected) {
         if ((rv = apr_socket_create(&newsock, backend_addr->family,
                                 SOCK_STREAM, APR_PROTO_TCP,
-                                conn->pool)) != APR_SUCCESS) {
+                                conn->scpool)) != APR_SUCCESS) {
             loglevel = backend_addr->next ? APLOG_DEBUG : APLOG_ERR;
             ap_log_error(APLOG_MARK, loglevel, rv, s,
                          "proxy: %s: error creating fam %d socket for target %s",
@@ -2290,11 +2290,11 @@ PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function,
         return OK;
     }
 
-    bucket_alloc = apr_bucket_alloc_create(conn->pool);
+    bucket_alloc = apr_bucket_alloc_create(conn->scpool);
     /*
      * The socket is now open, create a new backend server connection
      */
-    conn->connection = ap_run_create_connection(conn->pool, s, conn->sock,
+    conn->connection = ap_run_create_connection(conn->scpool, s, conn->sock,
                                                 0, NULL,
                                                 bucket_alloc);