httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rpl...@apache.org
Subject svn commit: r706921 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Date Wed, 22 Oct 2008 09:34:21 GMT
Author: rpluem
Date: Wed Oct 22 02:34:21 2008
New Revision: 706921

URL: http://svn.apache.org/viewvc?rev=706921&view=rev
Log:
* Improve the way to detect whether buckets in the filter chain need to be
  flushed by using the main requests bytes_count field instead of the
  subrequest field.

* Do not reset conn->need_flush. This prevents SegFaults from not flushing
  buckets in the filter chain.

PR: 45792

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/proxy/proxy_util.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=706921&r1=706920&r2=706921&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Oct 22 02:34:21 2008
@@ -2,6 +2,8 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) mod_proxy: Prevent segmentation faults by correctly flushing all buckets
+     from the proxy backend. PR 45792 [Ruediger Pluem]
 
   *) mod_dir: Support "DirectoryIndex None" 
      Suggested By André Warnier <aw ice-sa.com> [Eric Covener]

Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=706921&r1=706920&r2=706921&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Wed Oct 22 02:34:21 2008
@@ -1652,54 +1652,109 @@
 #endif
 
     r = conn->r;
-    if (conn->need_flush && r && (r->bytes_sent || r->eos_sent))
{
+    if (conn->need_flush && r) {
+        request_rec *main_request;
+
         /*
-         * 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 where 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 buckets 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.
+         * We need to get to the main request as subrequests do not count any
+         * sent bytes.
          */
-        c = r->connection;
-        bb = apr_brigade_create(r->pool, c->bucket_alloc);
-        if (r->eos_sent) {
+        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;
+
             /*
-             * 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.
+             * 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.
              */
-            ap_fflush(c->output_filters, bb);
-        }
-        else {
-            ap_fflush(r->output_filters, bb);
+            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;
         }
-        apr_brigade_destroy(bb);
-        conn->r = NULL;
-        conn->need_flush = 0;
     }
 
     /* determine if the connection need to be closed */



Mime
View raw message