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 2007/12/15 17:15:04 UTC
svn commit: r604447 - in /httpd/httpd/trunk: include/ap_mmn.h
modules/proxy/mod_proxy.h modules/proxy/mod_proxy_ftp.c
modules/proxy/proxy_util.c
Author: rpluem
Date: Sat Dec 15 08:15:04 2007
New Revision: 604447
URL: http://svn.apache.org/viewvc?rev=604447&view=rev
Log:
* Fix a SEGFAULT by ensuring that buckets that may have been buffered in the
network filters get flushed to the network. This is needed since
these buckets have been created with the bucket allocator of the
backend connection. This allocator either gets destroyed if
conn->close is set or the worker address is not reusable which
causes the connection to the backend to be closed or it will be used
again by another frontend connection that wants to recycle the
backend connection.
In this case we could run into nasty race conditions (e.g. if the
next user of the backend connection destroys the allocator before we
sent the buckets to the network).
Remark 1: Doing a setaside does not help here as the buckets remain
created by the wrong allocator in this case.
Remark 2: Yes, this creates a possible performance penalty in the case
of pipelined requests as we may send only a small amount of data over
the wire.
Modified:
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/modules/proxy/mod_proxy.h
httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.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=604447&r1=604446&r2=604447&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Sat Dec 15 08:15:04 2007
@@ -145,6 +145,7 @@
* 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
+ * 20071108.6 (2.3.0-dev) Add *r and need_flush to proxy_conn_rec structure
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -152,7 +153,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20071108
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 5 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 6 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=604447&r1=604446&r2=604447&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Sat Dec 15 08:15:04 2007
@@ -241,6 +241,10 @@
int inreslist; /* connection in apr_reslist? */
#endif
apr_pool_t *scpool; /* Subpool used for socket and connection data */
+ request_rec *r; /* Request record of the frontend request
+ * which the backend currently answers. */
+ int need_flush;/* Flag to decide whether we need to flush the
+ * filter chain or not */
} proxy_conn_rec;
typedef struct {
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c?rev=604447&r1=604446&r2=604447&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c Sat Dec 15 08:15:04 2007
@@ -959,6 +959,7 @@
}
/* TODO: see if ftp could use determine_connection */
backend->addr = connect_addr;
+ backend->r = r;
ap_set_module_config(c->conn_config, &proxy_ftp_module, 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=604447&r1=604446&r2=604447&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Sat Dec 15 08:15:04 2007
@@ -1576,6 +1576,9 @@
{
proxy_conn_rec *conn = (proxy_conn_rec *)theconn;
proxy_worker *worker = conn->worker;
+ apr_bucket_brigade *bb;
+ conn_rec *c;
+ request_rec *r;
/*
* If the connection pool is NULL the worker
@@ -1596,8 +1599,48 @@
}
#endif
+ r = conn->r;
+ if (conn->need_flush && r) {
+ /*
+ * We need to ensure that buckets that may have been buffered in the
+ * network filters get flushed to the network. This is needed since
+ * these buckets have been created with the bucket allocator of the
+ * backend connection. This allocator either gets destroyed if
+ * conn->close is set or the worker address is not reusable which
+ * causes the connection to the backend to be closed or it will be used
+ * again by another frontend connection that wants to recycle the
+ * backend connection.
+ * In this case we could run into nasty race conditions (e.g. if the
+ * next user of the backend connection destroys the allocator before we
+ * sent the buckets to the network).
+ *
+ * Remark 1: Doing a setaside does not help here as the buckets remain
+ * created by the wrong allocator in this case.
+ *
+ * Remark 2: Yes, this creates a possible performance penalty in the case
+ * of pipelined requests as we may send only a small amount of data over
+ * the wire.
+ */
+ c = r->connection;
+ bb = apr_brigade_create(r->pool, c->bucket_alloc);
+ if (r->eos_sent) {
+ /*
+ * If we have already sent an EOS bucket send directly to the
+ * connection based filters. We just want to flush the buckets
+ * if something hasn't been sent to the network yet.
+ */
+ ap_fflush(c->output_filters, bb);
+ }
+ else {
+ ap_fflush(r->output_filters, bb);
+ }
+ apr_brigade_destroy(bb);
+ conn->r = NULL;
+ conn->need_flush = 0;
+ }
+
/* determine if the connection need to be closed */
- if (conn->close) {
+ if (conn->close || !worker->is_address_reusable) {
apr_pool_t *p = conn->pool;
apr_pool_clear(p);
memset(conn, 0, sizeof(proxy_conn_rec));
@@ -1969,6 +2012,8 @@
apr_status_t err = APR_SUCCESS;
apr_status_t uerr = APR_SUCCESS;
+ conn->r = r;
+
/*
* Break up the URL to determine the host to connect to
*/
@@ -2287,6 +2332,12 @@
return OK;
}
+ /*
+ * We need to flush the buckets before we return the connection to the
+ * connection pool. See comment in connection_cleanup for why this is
+ * needed.
+ */
+ conn->need_flush = 1;
bucket_alloc = apr_bucket_alloc_create(conn->scpool);
/*
* The socket is now open, create a new backend server connection