From: Ruediger Pluem Date: Tue, 11 Dec 2007 13:27:21 +0000 (+0000) Subject: * Use a separate subpool to manage the data for the socket and the connection X-Git-Tag: 2.3.0~1153 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0ae93ad6f9bd3cea1ec0ff506279e685fb25dc23;p=thirdparty%2Fapache%2Fhttpd.git * 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 77510e63ae8..ad62ffa3ca7 100644 --- 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 ] diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 24346cb0843..2bfe1fc2241 100644 --- 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 27ac89f2d9b..84c6b261c73 100644 --- 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 91f83852539..f6f3fd08464 100644 --- 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);