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 2008/11/08 12:09:39 UTC

svn commit: r712375 - in /httpd/httpd/trunk: include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Author: rpluem
Date: Sat Nov  8 03:09:38 2008
New Revision: 712375

URL: http://svn.apache.org/viewvc?rev=712375&view=rev
Log:
* Rip out the old flushing approach for solving lifetime issues between the
  backend connection bucket allocator and front end connection bucket allocator.
  Instead copy the buckets from the backend over to ones that have been created
  using the front end bucket allocator. For metabucket this is done by recreating
  them, for data buckets this is done by reading them and putting the read data
  in a transient bucket.

PR: 45792

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/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=712375&r1=712374&r2=712375&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Sat Nov  8 03:09:38 2008
@@ -174,13 +174,16 @@
  * 20081101.0 (2.3.0-dev)  Remove unused AUTHZ_GROUP_NOTE define.
  * 20081102.0 (2.3.0-dev)  Remove authz_provider_list, authz_request_state,
  *                         and AUTHZ_ACCESS_PASSED_NOTE.
+ * 20081104.0 (2.3.0-dev)  Remove r and need_flush fields from proxy_conn_rec
+ *                         as they are no longer used and add
+ *                         ap_proxy_buckets_lifetime_transform to mod_proxy.h
  *
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20081102
+#define MODULE_MAGIC_NUMBER_MAJOR 20081104
 #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=712375&r1=712374&r2=712375&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Sat Nov  8 03:09:38 2008
@@ -741,6 +741,29 @@
 PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r,
                                            apr_bucket_brigade *brigade);
 
+/**
+ * Transform buckets from one bucket allocator to another one by creating a
+ * transient bucket for each data bucket and let it use the data read from
+ * the old bucket. Metabuckets are transformed by just recreating them.
+ * Attention: Currently only the following bucket types are handled:
+ *
+ * All data buckets
+ * FLUSH
+ * EOS
+ *
+ * If an other bucket type is found its type is logged as a debug message
+ * and APR_EGENERAL is returned.
+ * @param r    current request record of client request. Only used for logging
+ *             purposes
+ * @param from the brigade that contains the buckets to transform
+ * @param to   the brigade that will receive the transformed buckets
+ * @return     APR_SUCCESS if all buckets could be transformed APR_EGENERAL
+ *             otherwise
+ */
+PROXY_DECLARE(apr_status_t)
+ap_proxy_buckets_lifetime_transform(request_rec *r, apr_bucket_brigade *from,
+                                        apr_bucket_brigade *to);
+
 #define PROXY_LBMETHOD "proxylbmethod"
 
 /* The number of dynamic workers that can be added when reconfiguring.

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=712375&r1=712374&r2=712375&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c Sat Nov  8 03:09:38 2008
@@ -969,7 +969,6 @@
         }
         /* 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/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=712375&r1=712374&r2=712375&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Sat Nov  8 03:09:38 2008
@@ -1335,6 +1335,7 @@
     request_rec *rp;
     apr_bucket *e;
     apr_bucket_brigade *bb, *tmp_bb;
+    apr_bucket_brigade *pass_bb;
     int len, backasswards;
     int interim_response = 0; /* non-zero whilst interim 1xx responses
                                * are being read. */
@@ -1347,6 +1348,7 @@
     const char *te = NULL;
 
     bb = apr_brigade_create(p, c->bucket_alloc);
+    pass_bb = apr_brigade_create(p, c->bucket_alloc);
 
     /* Get response from the remote server, and pass it up the
      * filter chain
@@ -1765,6 +1767,9 @@
                         break;
                     }
 
+                    /* Switch the allocator lifetime of the buckets */
+                    ap_proxy_buckets_lifetime_transform(r, bb, pass_bb);
+
                     /* found the last brigade? */
                     if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
                         /* signal that we must leave */
@@ -1772,7 +1777,7 @@
                     }
 
                     /* try send what we read */
-                    if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS
+                    if (ap_pass_brigade(r->output_filters, pass_bb) != APR_SUCCESS
                         || c->aborted) {
                         /* Ack! Phbtt! Die! User aborted! */
                         backend->close = 1;  /* this causes socket close below */
@@ -1781,6 +1786,7 @@
 
                     /* make sure we always clean up after ourselves */
                     apr_brigade_cleanup(bb);
+                    apr_brigade_cleanup(pass_bb);
 
                 } while (!finish);
             }

Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=712375&r1=712374&r2=712375&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Sat Nov  8 03:09:38 2008
@@ -1628,9 +1628,6 @@
 {
     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
@@ -1651,112 +1648,6 @@
     }
 #endif
 
-    r = conn->r;
-    if (conn->need_flush && r) {
-        request_rec *main_request;
-
-        /*
-         * We need to get to the main request as subrequests do not count any
-         * sent bytes.
-         */
-        main_request = r;
-        while (main_request->main) {
-            main_request = main_request->main;
-        }
-        if (main_request->bytes_sent || r->eos_sent) {
-            ap_filter_t *flush_filter;
-
-            /*
-             * 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: Only do this if buckets were sent down the chain before
-             * that could still be buffered in the network filter. This is the
-             * case if we have sent an EOS bucket or if we actually sent buckets
-             * with data down the chain. In all other cases we either have not
-             * sent any buckets at all down the chain or we only sent meta
-             * buckets that are not EOS buckets down the chain. The only meta
-             * bucket that remains in this case is the flush bucket which would
-             * have removed all possibly buffered buckets in the network filter.
-             * If we sent a flush bucket in the case where not ANY buckets were
-             * sent down the chain, we break error handling which happens AFTER
-             * us.
-             *
-             * Remark 2: Doing a setaside does not help here as the bucketsi
-             * remain created by the wrong allocator in this case.
-             *
-             * Remark 3: 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.
-             *
-             * XXX: There remains a nasty edge case with detecting whether data
-             * was sent down the chain: If the data sent down the chain was
-             * set aside by some filter in the chain r->bytes_sent will still
-             * be zero albeit there are buckets in the chain that we would
-             * need to flush. Adding our own filter at the begining of the
-             * chain for detecting the passage of data buckets does not help
-             * as well because calling ap_set_content_type can cause such
-             * filters to be added *before* our filter in the chain and thus
-             * have it cut off from this information.
-             */
-            c = r->connection;
-            bb = apr_brigade_create(r->pool, c->bucket_alloc);
-            if (r->eos_sent) {
-                if (r->main) {
-                    /*
-                     * If we are a subrequest the EOS bucket went up the filter
-                     * chain up to the SUBREQ_CORE filter which drops it
-                     * silently. So we need to sent our flush bucket through
-                     * all the filters which haven't seen the EOS bucket. These
-                     * are the filters after the SUBREQ_CORE filter.
-                     * All filters after the SUBREQ_CORE filter are
-                     *
-                     * Either not connection oriented, so filter->r == NULL or
-                     *
-                     * filter->r != r since they belong to a different request
-                     * (our parent request).
-                     */
-                    flush_filter = r->output_filters;
-                    /*
-                     * We do not need to check if flush_filter->next != NULL
-                     * because there is at least the core output filter which
-                     * is a network filter and thus flush_filter->r == NULL
-                     * which is != r.
-                     */
-                    while (flush_filter->r == r) {
-                        flush_filter = flush_filter->next;
-                    }
-                }
-                else {
-                    /*
-                     * If we are a main request and 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 and
-                     * the request based filters in the chain may not work
-                     * any longer once they have seen an EOS bucket.
-                     */
-                    flush_filter = c->output_filters;
-                }
-            }
-            else {
-                flush_filter = r->output_filters;
-            }
-            ap_fflush(flush_filter, bb);
-            apr_brigade_destroy(bb);
-            conn->r = NULL;
-        }
-    }
-
     /* determine if the connection need to be closed */
     if (conn->close || !worker->is_address_reusable || worker->disablereuse) {
         apr_pool_t *p = conn->pool;
@@ -2138,8 +2029,6 @@
     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
      */
@@ -2478,12 +2367,6 @@
         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
@@ -2576,3 +2459,54 @@
     e = apr_bucket_eos_create(c->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(brigade, e);
 }
+
+/*
+ * Transform buckets from one bucket allocator to another one by creating a
+ * transient bucket for each data bucket and let it use the data read from
+ * the old bucket. Metabuckets are transformed by just recreating them.
+ * Attention: Currently only the following bucket types are handled:
+ *
+ * All data buckets
+ * FLUSH
+ * EOS
+ *
+ * If an other bucket type is found its type is logged as a debug message
+ * and APR_EGENERAL is returned.
+ */
+PROXY_DECLARE(apr_status_t)
+ap_proxy_buckets_lifetime_transform(request_rec *r, apr_bucket_brigade *from,
+                                    apr_bucket_brigade *to)
+{
+    apr_bucket *e;
+    apr_bucket *new;
+    const char *data;
+    apr_size_t bytes;
+    apr_status_t rv = APR_SUCCESS;
+
+    apr_brigade_cleanup(to);
+    for (e = APR_BRIGADE_FIRST(from);
+         e != APR_BRIGADE_SENTINEL(from);
+         e = APR_BUCKET_NEXT(e)) {
+        if (!APR_BUCKET_IS_METADATA(e)) {
+            apr_bucket_read(e, &data, &bytes, APR_BLOCK_READ);
+            new = apr_bucket_transient_create(data, bytes, r->connection->bucket_alloc);
+            APR_BRIGADE_INSERT_TAIL(to, new);
+        }
+        else if (APR_BUCKET_IS_FLUSH(e)) {
+            new = apr_bucket_flush_create(r->connection->bucket_alloc);
+            APR_BRIGADE_INSERT_TAIL(to, new);
+        }
+        else if (APR_BUCKET_IS_EOS(e)) {
+            new = apr_bucket_eos_create(r->connection->bucket_alloc);
+            APR_BRIGADE_INSERT_TAIL(to, new);
+        }
+        else {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                          "proxy: Unhandled bucket type of type %s in"
+                          " ap_proxy_buckets_lifetime_transform", e->type->name);
+            rv = APR_EGENERAL;
+        }
+    }
+    return rv;
+}
+