From: Ruediger Pluem Date: Thu, 23 Dec 2010 16:43:43 +0000 (+0000) Subject: * The concept of the cleaned flag is flawed: Once we returned the connection X-Git-Tag: 2.3.11~352 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cbe247d915ff6d144f6f113135a02754718b61fa;p=apache * The concept of the cleaned flag is flawed: Once we returned the connection to the pool we cannot longer rely on it as another thread could have leased the connection in the meantime and might have modified it. BUT: We only use this flag once we returned the connection to the pool. So signal that we returned the connection to the pool by something that is local to the thread, in this case set backend to NULL if we already have returende the connection. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1052314 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/include/ap_mmn.h b/include/ap_mmn.h index aab46e5278..2f7de4bee8 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -287,12 +287,13 @@ * 20101113.1 (2.3.9-dev) Add ap_set_flag_slot_char() * 20101113.2 (2.3.9-dev) Add ap_expr_exec_re() * 20101204.0 (2.3.10-dev) Add _t to ap_expr's typedef names + * 20101223.0 (2.3.11-dev) Remove cleaned from proxy_conn_rec. */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ #ifndef MODULE_MAGIC_NUMBER_MAJOR -#define MODULE_MAGIC_NUMBER_MAJOR 20101204 +#define MODULE_MAGIC_NUMBER_MAJOR 20101223 #endif #define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */ diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 6da213cbca..a993556d00 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -230,7 +230,6 @@ typedef struct { #if APR_HAS_THREADS int inreslist:1; /* connection in apr_reslist? */ #endif - int cleaned:1; /* connection cleaned? */ } proxy_conn_rec; typedef struct { diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index d523e16d09..e661f34e45 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1383,7 +1383,7 @@ apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, request_rec static apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, - proxy_conn_rec *backend, + proxy_conn_rec **backend_ptr, proxy_worker *worker, proxy_server_conf *conf, char *server_portstr) { @@ -1409,6 +1409,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, int proxy_status = OK; const char *original_status_line = r->status_line; const char *proxy_status_line = NULL; + proxy_conn_rec *backend = *backend_ptr; conn_rec *origin = backend->connection; apr_interval_time_t old_timeout = 0; proxy_dir_conf *dconf; @@ -1939,6 +1940,8 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, */ ap_proxy_release_connection(backend->worker->scheme, backend, r->server); + /* Ensure that the backend is not reused */ + backend_ptr = NULL; } @@ -1946,7 +1949,12 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, if (ap_pass_brigade(r->output_filters, pass_bb) != APR_SUCCESS || c->aborted) { /* Ack! Phbtt! Die! User aborted! */ - if (!backend->cleaned) { + /* Only close backend if we haven't got all from the + * backend. Furthermore if backend_ptr is NULL it is no + * longer save to fiddle around with backend as it might + * be already in use by another thread. + */ + if (backend_ptr) { backend->close = 1; /* this causes socket close below */ } finish = TRUE; @@ -1972,6 +1980,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, */ ap_proxy_release_connection(backend->worker->scheme, backend, r->server); + backend_ptr = NULL; /* Pass EOS bucket down the filter chain. */ e = apr_bucket_eos_create(c->bucket_alloc); @@ -2151,7 +2160,7 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, } /* Step Five: Receive the Response... Fall thru to cleanup */ - status = ap_proxy_http_process_response(p, r, backend, worker, + status = ap_proxy_http_process_response(p, r, &backend, worker, conf, server_portstr); break; @@ -2160,7 +2169,7 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, /* Step Six: Clean Up */ cleanup: if (backend) { - if ((status != OK) && (!backend->cleaned)) + if (status != OK) backend->close = 1; ap_proxy_http_cleanup(proxy_function, r, backend); } diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 59ded45de1..85c52b3f7d 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -2105,7 +2105,6 @@ PROXY_DECLARE(int) ap_proxy_acquire_connection(const char *proxy_function, #if APR_HAS_THREADS (*conn)->inreslist = 0; #endif - (*conn)->cleaned = 0; return OK; } @@ -2114,13 +2113,10 @@ PROXY_DECLARE(int) ap_proxy_release_connection(const char *proxy_function, proxy_conn_rec *conn, server_rec *s) { - if (!conn->cleaned) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "proxy: %s: has released connection for (%s)", proxy_function, conn->worker->hostname); - connection_cleanup(conn); - conn->cleaned = 1; - } + connection_cleanup(conn); return OK; }