* 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.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@604447 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 2bfe1fc22410bc23728f8ec3070e68c66eadc368..ad19c227450759408e20feb758ecdcbf823bfea0 100644 (file)
--- 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 84c6b261c7358bbc6387fa2afd4c39a47b6ade21..9a6a6eb10b49e4f4c4da31ccf89c5f6a027da960 100644 (file)
--- 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 f7e951d280e0b3f853cb1759c334d3f6ab62e9ec..6d4c63a8c4194c7b2be17b5acf874fc537a42a23 100644 (file)
--- 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 92eb8f34c0e3344c838368a80906eb8947cabeb0..e6e2e5ef3bb51a6dd071f5eb8345a33b1cf67889 100644 (file)
--- 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