diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 2bfe1fc224..ad19c22745 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -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 diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 84c6b261c7..9a6a6eb10b 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -241,6 +241,10 @@ typedef struct { 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 { diff --git a/modules/proxy/mod_proxy_ftp.c b/modules/proxy/mod_proxy_ftp.c index f7e951d280..6d4c63a8c4 100644 --- a/modules/proxy/mod_proxy_ftp.c +++ b/modules/proxy/mod_proxy_ftp.c @@ -959,6 +959,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, } /* 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); } diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 92eb8f34c0..e6e2e5ef3b 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1576,6 +1576,9 @@ static apr_status_t connection_cleanup(void *theconn) { 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 @@ static apr_status_t connection_cleanup(void *theconn) } #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 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, 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 @@ PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function, 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