You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2010/12/23 17:43:44 UTC
svn commit: r1052314 - in /httpd/httpd/trunk: include/ap_mmn.h
modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c
modules/proxy/proxy_util.c
Author: rpluem
Date: Thu Dec 23 16:43:43 2010
New Revision: 1052314
URL: http://svn.apache.org/viewvc?rev=1052314&view=rev
Log:
* 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.
Modified:
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/modules/proxy/mod_proxy.h
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1052314&r1=1052313&r2=1052314&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Thu Dec 23 16:43:43 2010
@@ -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 */
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1052314&r1=1052313&r2=1052314&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Thu Dec 23 16:43:43 2010
@@ -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 {
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1052314&r1=1052313&r2=1052314&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Thu Dec 23 16:43:43 2010
@@ -1383,7 +1383,7 @@ apr_status_t ap_proxygetline(apr_bucket_
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_respo
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_respo
*/
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_respo
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_respo
*/
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_re
}
/* 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_re
/* 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);
}
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1052314&r1=1052313&r2=1052314&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Dec 23 16:43:43 2010
@@ -2105,7 +2105,6 @@ PROXY_DECLARE(int) ap_proxy_acquire_conn
#if APR_HAS_THREADS
(*conn)->inreslist = 0;
#endif
- (*conn)->cleaned = 0;
return OK;
}
@@ -2114,13 +2113,10 @@ PROXY_DECLARE(int) ap_proxy_release_conn
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;
}
Re: svn commit: r1052314 - in /httpd/httpd/trunk: include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
Posted by Jim Jagielski <ji...@apache.org>.
On Dec 23, 2010, at 11:43 AM, rpluem@apache.org wrote:
> Author: rpluem
> Date: Thu Dec 23 16:43:43 2010
> New Revision: 1052314
>
> URL: http://svn.apache.org/viewvc?rev=1052314&view=rev
> Log:
> * 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.
>
v. cool...