Hello, I have proposed a patch for PR39673 but I'm not sure it would be accepted for mainline httpd, so here I am.
The patch adds the possibility (for a module) to acquire a connection aside from the worker's reslist, so that it won't be acquired from the reslist nor put back to it once released (nor reused for another client connection, unless the module does it on its own). The connection is still "bound" to a worker, hence it will be established/closed according to the worker's configuration (hostname, port, SSL, is_address_reusable, TTL...), and the module can still (and should) call ap_proxy_determine_connection(), ap_proxy_connect_backend() and ap_proxy_release_connection() to do that work. The new function to acquire an aside connection is : PROXY_DECLARE(int) ap_proxy_acquire_connection_ex(const char *proxy_function, proxy_conn_rec **conn, proxy_worker *worker, server_rec *s, void *baton, ap_proxy_acquire_fn acquire, ap_proxy_release_fn release); When called with both baton and acquire set to NULL, ap_proxy_acquire_connection_ex() is the same as ap_proxy_acquire_connection(), and will acquire connections from the reslist. When acquire is not NULL, it is used as the constructor for *conn (given the baton), so it's up to the caller to create a new (aside) connection (with the new function ap_proxy_aside_connection() also provided) or reuse an existing one according to its criteria (and the baton). If release is not NULL, it will be called upon ap_proxy_release_connection() with the same baton. When acquire is NULL but baton isn't, a built-in acquire function is used to select an existing or create a new connection associated to a conn_rec (and still the worker). The baton is assumed to be a conn_rec (eg. the one of the client's connection), and this mode is hence a return back to the days of httpd <= 2.0. This is the trick to respond to PR39673 (NTLM forwarding), or any issue due to a session associated to a TCP connection (some load-balancers configured to do so expect that), which mod_proxy can't forward currently. The patch then uses the new ap_proxy_acquire_connection_ex() function (latter mode) in mod_proxy_http and mod_proxy_ajp (which both have the ability to reuse connections), but only when the environment var "proxy-aside-c" is defined. This allows for the administrator to use SetEnv[If] or a RewriteRule to enable the functionality, according to specific conditions, like (PR39673) : RewriteCond %{HTTP:Authorization} ^NTLM RewriteRule ^ - [E=proxy-aside-c] The patch is attached, and I tried to make it as generic as I could so that it is not PR39673 (NTLM) specific, aside connections looks like a nice to have for modules. This version is slightly different from the one proposed in the PR, in that it will always close aside connections associated with a non-reusable worker upon release (the PR's one failed to do that), and the default release callback (when none is given) for aside connections is now to apply the worker's configured TTL (if any). Do you think this can/should (not) be applied to httpd? Regards, Yann.
Index: modules/proxy/mod_proxy_ajp.c =================================================================== --- modules/proxy/mod_proxy_ajp.c (revision 1628385) +++ modules/proxy/mod_proxy_ajp.c (working copy) @@ -730,6 +730,12 @@ static int proxy_ajp_handler(request_rec *r, proxy ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00895) "serving URL %s", url); /* create space for state information */ + if (apr_table_get(r->subprocess_env, "proxy-aside-c")) { + status = ap_proxy_acquire_connection_ex(scheme, &backend, worker, + r->server, r->connection, + NULL, NULL); + } + else status = ap_proxy_acquire_connection(scheme, &backend, worker, r->server); if (status != OK) { Index: modules/proxy/proxy_util.c =================================================================== --- modules/proxy/proxy_util.c (revision 1628385) +++ modules/proxy/proxy_util.c (working copy) @@ -72,6 +72,13 @@ static struct wstat { {0x0, '\0', NULL} }; +/* Opaque aside connection record */ +struct ap_proxy_aside_rec { + void *baton; + ap_proxy_release_fn release; + apr_time_t expiry; /* idle connection expiry */ +}; + /* Global balancer counter */ int PROXY_DECLARE_DATA proxy_lb_workers = 0; static int lb_workers_limit = 0; @@ -1351,6 +1358,14 @@ PROXY_DECLARE(int) ap_proxy_connection_reusable(pr return ! (conn->close || !worker->s->is_address_reusable || worker->s->disablereuse); } +static void socket_cleanup(proxy_conn_rec *conn) +{ + conn->sock = NULL; + conn->connection = NULL; + conn->ssl_hostname = NULL; + apr_pool_clear(conn->scpool); +} + static apr_status_t connection_cleanup(void *theconn) { proxy_conn_rec *conn = (proxy_conn_rec *)theconn; @@ -1360,7 +1375,7 @@ static apr_status_t connection_cleanup(void *theco * If the connection pool is NULL the worker * cleanup has been run. Just return. */ - if (!worker->cp) { + if (!worker->cp->pool) { return APR_SUCCESS; } @@ -1379,17 +1394,45 @@ static apr_status_t connection_cleanup(void *theco } /* determine if the connection need to be closed */ - if (!ap_proxy_connection_reusable(conn)) { + if (!worker->s->is_address_reusable || worker->s->disablereuse) { + ap_proxy_aside_rec aside; + int is_aside = (conn->aside != NULL); apr_pool_t *p = conn->pool; + if (is_aside) { + aside = *conn->aside; + } apr_pool_clear(p); conn = apr_pcalloc(p, sizeof(proxy_conn_rec)); conn->pool = p; conn->worker = worker; apr_pool_create(&(conn->scpool), p); apr_pool_tag(conn->scpool, "proxy_conn_scpool"); + if (is_aside) { + conn->aside = apr_pmemdup(p, &aside, sizeof aside); + } } + else if (conn->close) { + socket_cleanup(conn); + conn->close = 0; + } - if (worker->s->hmax && worker->cp->res) { + /* If the connection is aside (not acquired from the reslist), + * let it be released by the callback if any, or live with its + * parent pool otherwise. + */ + if (conn->aside) { + if (conn->aside->release) { + conn->aside->release(conn->aside->baton, conn); + } + else if (conn->sock && worker->s->ttl) { + conn->aside->expiry = apr_time_now() + + apr_time_from_sec(worker->s->ttl); + } + else { + conn->aside->expiry = 0; + } + } + else if (worker->s->hmax && worker->cp->res) { conn->inreslist = 1; apr_reslist_release(worker->cp->res, (void *)conn); } @@ -1402,14 +1445,6 @@ static apr_status_t connection_cleanup(void *theco return APR_SUCCESS; } -static void socket_cleanup(proxy_conn_rec *conn) -{ - conn->sock = NULL; - conn->connection = NULL; - conn->ssl_hostname = NULL; - apr_pool_clear(conn->scpool); -} - PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup(proxy_conn_rec *conn, request_rec *r) { @@ -1485,10 +1520,11 @@ static apr_status_t connection_constructor(void ** static apr_status_t connection_destructor(void *resource, void *params, apr_pool_t *pool) { - proxy_conn_rec *conn = (proxy_conn_rec *)resource; + proxy_worker *worker = (proxy_worker *)params; /* Destroy the pool only if not called from reslist_destroy */ - if (conn->worker->cp->pool) { + if (worker->cp->pool) { + proxy_conn_rec *conn = (proxy_conn_rec *)resource; apr_pool_destroy(conn->pool); } @@ -2151,11 +2187,66 @@ PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr return connected ? 0 : 1; } -PROXY_DECLARE(int) ap_proxy_acquire_connection(const char *proxy_function, - proxy_conn_rec **conn, - proxy_worker *worker, - server_rec *s) +PROXY_DECLARE(apr_status_t) ap_proxy_aside_connection(proxy_conn_rec **conn, + proxy_worker *worker, + apr_pool_t *pool) { + apr_status_t rv = connection_constructor((void **)conn, worker, pool); + if (rv == APR_SUCCESS) { + (*conn)->aside = apr_pcalloc((*conn)->pool, sizeof *(*conn)->aside); + (*conn)->inreslist = 0; + } + return rv; +} + +PROXY_DECLARE(apr_status_t) ap_proxy_destroy_connection(proxy_conn_rec *conn) +{ + proxy_worker *worker = conn->worker; + if (conn->aside) { + return connection_destructor(conn, worker, NULL); + } + else { + return apr_reslist_invalidate(worker->cp->res, conn); + } +} + +/* For the given conn_rec (baton), handle a connection per worker */ +static apr_status_t proxy_acquire_from_c(void *baton, proxy_conn_rec **conn, + proxy_worker *worker, server_rec *s) +{ + apr_hash_t *conns; + conn_rec *c = baton; + + conns = ap_get_module_config(c->conn_config, &proxy_module); + if (!conns) { + conns = apr_hash_make(c->pool); + ap_set_module_config(c->conn_config, &proxy_module, conns); + *conn = NULL; + } + else { + *conn = apr_hash_get(conns, &worker->s, sizeof worker->s); + } + if (!*conn) { + ap_proxy_aside_connection(conn, worker, c->pool); + apr_hash_set(conns, &worker->s, sizeof worker->s, *conn); + } + else if ((*conn)->aside->expiry) { + if (apr_time_now() >= (*conn)->aside->expiry) { + socket_cleanup(*conn); + } + (*conn)->aside->expiry = 0; + } + + return APR_SUCCESS; +} + +PROXY_DECLARE(int) ap_proxy_acquire_connection_ex(const char *proxy_function, + proxy_conn_rec **conn, + proxy_worker *worker, + server_rec *s, void *baton, + ap_proxy_acquire_fn acquire, + ap_proxy_release_fn release) +{ apr_status_t rv; if (!PROXY_WORKER_IS_USABLE(worker)) { @@ -2170,7 +2261,41 @@ PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr } } - if (worker->s->hmax && worker->cp->res) { + /* Aside connection case, use the given functions. + * When a baton is given with no acquire function, it is + * assumed to be a conn_rec (to acquire from), hence the + * corresponding functions are installed. + */ + if (acquire || (baton && (acquire = proxy_acquire_from_c))) { + rv = acquire(baton, conn, worker, s); + if (rv == APR_SUCCESS) { + if (!(*conn)->aside) { + (*conn)->aside = apr_pcalloc((*conn)->pool, + sizeof *(*conn)->aside); + } + if (release && acquire != proxy_acquire_from_c) { + (*conn)->aside->release = release; + } + else { + (*conn)->aside->release = NULL; + } + (*conn)->aside->baton = baton; + } + else if (rv != APR_NOTFOUND) { + ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO() + "%s: failed to acquire aside connection for (%s)", + proxy_function, worker->s->hostname); + return HTTP_SERVICE_UNAVAILABLE; + } + } + else { + rv = APR_NOTFOUND; + } + + if (rv == APR_SUCCESS) { + /* got/use aside connection */ + } + else if (worker->s->hmax && worker->cp->res) { rv = apr_reslist_acquire(worker->cp->res, (void **)conn); } else { @@ -2192,8 +2317,9 @@ PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr return HTTP_SERVICE_UNAVAILABLE; } ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00942) - "%s: has acquired connection for (%s)", - proxy_function, worker->s->hostname); + "%s: has acquired %sconnection for (%s)", + proxy_function, (*conn)->aside ? "aside " : "", + worker->s->hostname); (*conn)->worker = worker; (*conn)->close = 0; @@ -2202,13 +2328,23 @@ PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr return OK; } +PROXY_DECLARE(int) ap_proxy_acquire_connection(const char *proxy_function, + proxy_conn_rec **conn, + proxy_worker *worker, + server_rec *s) +{ + return ap_proxy_acquire_connection_ex(proxy_function, conn, + worker, s, NULL, NULL, NULL); +} + PROXY_DECLARE(int) ap_proxy_release_connection(const char *proxy_function, proxy_conn_rec *conn, server_rec *s) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00943) - "%s: has released connection for (%s)", - proxy_function, conn->worker->s->hostname); + "%s: has released %sconnection for (%s)", + proxy_function, conn->aside ? "aside " : "", + conn->worker->s->hostname); connection_cleanup(conn); return OK; Index: modules/proxy/mod_proxy_http.c =================================================================== --- modules/proxy/mod_proxy_http.c (revision 1628385) +++ modules/proxy/mod_proxy_http.c (working copy) @@ -1962,8 +1962,16 @@ static int proxy_http_handler(request_rec *r, prox /* create space for state information */ - if ((status = ap_proxy_acquire_connection(proxy_function, &backend, - worker, r->server)) != OK) + if (apr_table_get(r->subprocess_env, "proxy-aside-c")) { + status = ap_proxy_acquire_connection_ex(proxy_function, &backend, + worker, r->server, c, + NULL, NULL); + } + else { + status = ap_proxy_acquire_connection(proxy_function, &backend, + worker, r->server); + } + if (status != OK) goto cleanup; Index: modules/proxy/mod_proxy.h =================================================================== --- modules/proxy/mod_proxy.h (revision 1628385) +++ modules/proxy/mod_proxy.h (working copy) @@ -234,6 +234,9 @@ typedef struct { apr_array_header_t* cookie_domains; } proxy_req_conf; +/* Opaque aside connection record */ +typedef struct ap_proxy_aside_rec ap_proxy_aside_rec; + typedef struct { conn_rec *connection; request_rec *r; /* Request record of the backend request @@ -255,6 +258,7 @@ typedef struct { unsigned int inreslist:1; /* connection in apr_reslist? */ const char *uds_path; /* Unix domain socket path */ const char *ssl_hostname;/* Hostname (SNI) in use by SSL connection */ + ap_proxy_aside_rec *aside; /* Aside context */ } proxy_conn_rec; typedef struct { @@ -868,7 +872,39 @@ PROXY_DECLARE(int) ap_proxy_acquire_connection(con proxy_conn_rec **conn, proxy_worker *worker, server_rec *s); + /** + * Function types usable by ap_proxy_acquire_connection_ex(). + */ +typedef apr_status_t (*ap_proxy_acquire_fn)(void *baton, proxy_conn_rec **conn, + proxy_worker *worker, + server_rec *s); +typedef apr_status_t (*ap_proxy_release_fn)(void *baton, proxy_conn_rec *conn); + +/** + * Acquire an aside connection using the given @acquire function and register + * a @release callback for it on cleanup. + * @param proxy_function calling proxy scheme (http, ajp, ...) + * @param conn acquired connection + * @param worker worker used for obtaining connection + * @param s current server record + * @param baton associated baton (or conn_rec if @acquire is NULL) + * @param acquire function (if not NULL) used to acquire aside connections, + * given @baton, @conn, @worker, and @s + * @param release function (if not NULL) used to release acquired connection, + * given @baton and @conn + * @return OK or HTTP_XXX error + * @note If the given @baton is not NULL but @acquire is NULL, the baton is + * considered to be a conn_rec to which the connection is attached (by worker). + */ +PROXY_DECLARE(int) ap_proxy_acquire_connection_ex(const char *proxy_function, + proxy_conn_rec **conn, + proxy_worker *worker, + server_rec *s, void *baton, + ap_proxy_acquire_fn acquire, + ap_proxy_release_fn release); + +/** * Release a connection back to worker connection pool * @param proxy_function calling proxy scheme (http, ajp, ...) * @param conn acquired connection