You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2015/10/04 12:10:52 UTC

svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Author: minfrin
Date: Sun Oct  4 10:10:51 2015
New Revision: 1706669

URL: http://svn.apache.org/viewvc?rev=1706669&view=rev
Log:
core: Extend support for asynchronous write completion from the
network filter to any connection or request filter.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/include/ap_mmn.h
    httpd/httpd/trunk/include/http_core.h
    httpd/httpd/trunk/include/http_request.h
    httpd/httpd/trunk/include/httpd.h
    httpd/httpd/trunk/include/util_filter.h
    httpd/httpd/trunk/modules/http/http_core.c
    httpd/httpd/trunk/modules/http/http_request.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
    httpd/httpd/trunk/server/core.c
    httpd/httpd/trunk/server/core_filters.c
    httpd/httpd/trunk/server/mpm/event/event.c
    httpd/httpd/trunk/server/mpm/motorz/motorz.c
    httpd/httpd/trunk/server/mpm/simple/simple_io.c
    httpd/httpd/trunk/server/request.c
    httpd/httpd/trunk/server/util_filter.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sun Oct  4 10:10:51 2015
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) core: Extend support for asynchronous write completion from the
+     network filter to any connection or request filter. [Graham Leggett]
+
   *) mpm_event: Free memory earlier when shutting down processes.
      [Stefan Fritsch]
 

Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Sun Oct  4 10:10:51 2015
@@ -489,6 +489,11 @@
  *                         protocol_switch and protocol_get. Add
  *                         ap_select_protocol(), ap_switch_protocol(),
  *                         ap_get_protocol(). Add HTTP_MISDIRECTED_REQUEST.
+ * 20150222.5 (2.5.0-dev)  Add ap_request_core_filter(),
+ *                         ap_filter_setaside_brigade(),
+ *                         ap_filter_reinstate_brigade() and
+ *                         ap_filter_should_yield(). Add empty and filters to
+ *                         conn_rec.
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -496,7 +501,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20150222
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 4                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 5                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

Modified: httpd/httpd/trunk/include/http_core.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_core.h (original)
+++ httpd/httpd/trunk/include/http_core.h Sun Oct  4 10:10:51 2015
@@ -792,6 +792,7 @@ AP_DECLARE_DATA extern ap_filter_rec_t *
 AP_DECLARE_DATA extern ap_filter_rec_t *ap_core_output_filter_handle;
 AP_DECLARE_DATA extern ap_filter_rec_t *ap_content_length_filter_handle;
 AP_DECLARE_DATA extern ap_filter_rec_t *ap_core_input_filter_handle;
+AP_DECLARE_DATA extern ap_filter_rec_t *ap_request_core_filter_handle;
 
 /**
  * This hook provdes a way for modules to provide metrics/statistics about

Modified: httpd/httpd/trunk/include/http_request.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_request.h?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_request.h (original)
+++ httpd/httpd/trunk/include/http_request.h Sun Oct  4 10:10:51 2015
@@ -149,6 +149,18 @@ AP_DECLARE(int) ap_run_sub_req(request_r
  */
 AP_DECLARE(void) ap_destroy_sub_req(request_rec *r);
 
+/**
+ * An output filter to ensure that we avoid passing morphing buckets to
+ * connection filters and in so doing defeat async write completion when
+ * they are set aside. This should be inserted at the end of a request
+ * filter stack.
+ * @param f The current filter
+ * @param bb The brigade to filter
+ * @return status code
+ */
+AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
+                                                            apr_bucket_brigade *bb);
+
 /*
  * Then there's the case that you want some other request to be served
  * as the top-level request INSTEAD of what the client requested directly.

Modified: httpd/httpd/trunk/include/httpd.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/include/httpd.h (original)
+++ httpd/httpd/trunk/include/httpd.h Sun Oct  4 10:10:51 2015
@@ -55,6 +55,7 @@
 #include "apr_buckets.h"
 #include "apr_poll.h"
 #include "apr_thread_proc.h"
+#include "apr_hash.h"
 
 #include "os.h"
 
@@ -1136,7 +1137,7 @@ struct conn_rec {
     conn_state_t *cs;
     /** Is there data pending in the input filters? */
     int data_in_input_filters;
-    /** Is there data pending in the output filters? */
+    /** No longer used, replaced with ap_filter_should_yield() */
     int data_in_output_filters;
 
     /** Are there any filters that clogg/buffer the input stream, breaking
@@ -1191,6 +1192,12 @@ struct conn_rec {
 
     /** Array of requests being handled under this connection. */
     apr_array_header_t *requests;
+
+    /** Empty bucket brigade */
+    apr_bucket_brigade *empty;
+
+    /** Hashtable of filters with setaside buckets for write completion */
+    apr_hash_t *filters;
 };
 
 struct conn_slave_rec {

Modified: httpd/httpd/trunk/include/util_filter.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_filter.h?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/include/util_filter.h (original)
+++ httpd/httpd/trunk/include/util_filter.h Sun Oct  4 10:10:51 2015
@@ -278,6 +278,13 @@ struct ap_filter_t {
      *  to the request_rec, except that it is used for connection filters.
      */
     conn_rec *c;
+
+    /** Buffered data associated with the current filter. */
+    apr_bucket_brigade *bb;
+
+    /** Dedicated pool to use for deferred writes. */
+    apr_pool_t *deferred_pool;
+
 };
 
 /**
@@ -519,8 +526,11 @@ AP_DECLARE(apr_status_t) ap_remove_outpu
  */
 
 /**
- * prepare a bucket brigade to be setaside.  If a different brigade was
+ * Prepare a bucket brigade to be setaside.  If a different brigade was
  * set-aside earlier, then the two brigades are concatenated together.
+ *
+ * If *save_to is NULL, the brigade will be created, and a cleanup registered
+ * to clear the brigade address when the pool is destroyed.
  * @param f The current filter
  * @param save_to The brigade that was previously set-aside.  Regardless, the
  *             new bucket brigade is returned in this location.
@@ -533,6 +543,53 @@ AP_DECLARE(apr_status_t) ap_save_brigade
                                          apr_bucket_brigade **b, apr_pool_t *p);
 
 /**
+ * Prepare a bucket brigade to be setaside, creating a dedicated pool if
+ * necessary within the filter to handle the lifetime of the setaside brigade.
+ * @param f The current filter
+ * @param bb The bucket brigade to set aside.  This brigade is always empty
+ *          on return
+ */
+AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
+                                                    apr_bucket_brigade *bb);
+
+/**
+ * Reinstate a brigade setaside earlier, and calculate the amount of data we
+ * should write based on the presence of flush buckets, size limits on in
+ * memory buckets, and the number of outstanding requests in the pipeline.
+ * This is a safety mechanism to protect against a module that might try
+ * generate data too quickly for downstream to handle without yielding as
+ * it should.
+ *
+ * If the brigade passed in is empty, we reinstate the brigade and return
+ * immediately on the assumption that any buckets needing to be flushed were
+ * flushed before being passed to ap_filter_setaside_brigade().
+ *
+ * @param f The current filter
+ * @param bb The bucket brigade to restore to.
+ * @param flush_upto Work out the bucket we need to flush up to, based on the
+ *                   presence of a flush bucket, size limits on in-memory
+ *                   buckets, size limits on the number of requests outstanding
+ *                   in the pipeline.
+ * @return APR_SUCCESS.
+ */
+AP_DECLARE(apr_status_t) ap_filter_reinstate_brigade(ap_filter_t *f,
+                                                     apr_bucket_brigade *bb,
+                                                     apr_bucket **flush_upto);
+
+/**
+ * This function calculates whether there are any as yet unsent
+ * buffered brigades in downstream filters, and returns non zero
+ * if so.
+ *
+ * A filter should use this to determine whether the passing of data
+ * downstream might block, and so defer the passing of brigades
+ * downstream with ap_filter_setaside_brigade().
+ *
+ * This function can be called safely from a handler.
+ */
+AP_DECLARE(int) ap_filter_should_yield(ap_filter_t *f);
+
+/**
  * Flush function for apr_brigade_* calls.  This calls ap_pass_brigade
  * to flush the brigade if the brigade buffer overflows.
  * @param bb The brigade to flush

Modified: httpd/httpd/trunk/modules/http/http_core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_core.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_core.c (original)
+++ httpd/httpd/trunk/modules/http/http_core.c Sun Oct  4 10:10:51 2015
@@ -263,6 +263,8 @@ static int http_create_request(request_r
                                     NULL, r, r->connection);
         ap_add_output_filter_handle(ap_http_outerror_filter_handle,
                                     NULL, r, r->connection);
+        ap_add_output_filter_handle(ap_request_core_filter_handle,
+                                    NULL, r, r->connection);
     }
 
     return OK;

Modified: httpd/httpd/trunk/modules/http/http_request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_request.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_request.c (original)
+++ httpd/httpd/trunk/modules/http/http_request.c Sun Oct  4 10:10:51 2015
@@ -256,6 +256,14 @@ AP_DECLARE(void) ap_process_request_afte
     apr_bucket *b;
     conn_rec *c = r->connection;
 
+    /* Find the last request, taking into account internal
+     * redirects. We want to send the EOR bucket at the end of
+     * all the buckets so it does not jump the queue.
+     */
+    while (r->next) {
+        r = r->next;
+    }
+
     /* Send an EOR bucket through the output filter chain.  When
      * this bucket is destroyed, the request will be logged and
      * its pool will be freed
@@ -264,8 +272,8 @@ AP_DECLARE(void) ap_process_request_afte
     b = ap_bucket_eor_create(c->bucket_alloc, r);
     APR_BRIGADE_INSERT_HEAD(bb, b);
 
-    ap_pass_brigade(c->output_filters, bb);
-    
+    ap_pass_brigade(r->output_filters, bb);
+
     /* The EOR bucket has either been handled by an output filter (eg.
      * deleted or moved to a buffered_bb => no more in bb), or an error
      * occured before that (eg. c->aborted => still in bb) and we ought

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Sun Oct  4 10:10:51 2015
@@ -1682,6 +1682,7 @@ static apr_status_t ssl_io_filter_output
     ssl_filter_ctx_t *filter_ctx = f->ctx;
     bio_filter_in_ctx_t *inctx;
     bio_filter_out_ctx_t *outctx;
+    apr_bucket *flush_upto = NULL;
     apr_read_type_e rblock = APR_NONBLOCK_READ;
 
     if (f->c->aborted) {
@@ -1689,6 +1690,9 @@ static apr_status_t ssl_io_filter_output
         return APR_ECONNABORTED;
     }
 
+    /* Reinstate any buffered content */
+    ap_filter_reinstate_brigade(f, bb, &flush_upto);
+
     if (!filter_ctx->pssl) {
         /* ssl_filter_io_shutdown was called */
         return ap_pass_brigade(f->next, bb);
@@ -1711,6 +1715,16 @@ static apr_status_t ssl_io_filter_output
     while (!APR_BRIGADE_EMPTY(bb) && status == APR_SUCCESS) {
         apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
 
+        /* if the core has set aside data, back off and try later */
+        if (!flush_upto) {
+            if (ap_filter_should_yield(f)) {
+                break;
+            }
+        }
+        else if (flush_upto == bucket) {
+            flush_upto = NULL;
+        }
+
         if (APR_BUCKET_IS_METADATA(bucket)) {
             /* Pass through metadata buckets untouched.  EOC is
              * special; terminate the SSL layer first. */
@@ -1762,6 +1776,10 @@ static apr_status_t ssl_io_filter_output
 
     }
 
+    if (APR_STATUS_IS_EOF(status) || (status == APR_SUCCESS)) {
+        return ap_filter_setaside_brigade(f, bb);
+    }
+
     return status;
 }
 

Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Sun Oct  4 10:10:51 2015
@@ -112,6 +112,7 @@ AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t
 
 /* Handles for core filters */
 AP_DECLARE_DATA ap_filter_rec_t *ap_subreq_core_filter_handle;
+AP_DECLARE_DATA ap_filter_rec_t *ap_request_core_filter_handle;
 AP_DECLARE_DATA ap_filter_rec_t *ap_core_output_filter_handle;
 AP_DECLARE_DATA ap_filter_rec_t *ap_content_length_filter_handle;
 AP_DECLARE_DATA ap_filter_rec_t *ap_core_input_filter_handle;
@@ -5007,6 +5008,8 @@ static conn_rec *core_create_conn(apr_po
 
     c->id = id;
     c->bucket_alloc = alloc;
+    c->empty = apr_brigade_create(c->pool, c->bucket_alloc);
+    c->filters = apr_hash_make(c->pool);
 
     c->clogging_input_filters = 0;
 
@@ -5395,6 +5398,9 @@ static void register_hooks(apr_pool_t *p
     ap_core_output_filter_handle =
         ap_register_output_filter("CORE", ap_core_output_filter,
                                   NULL, AP_FTYPE_NETWORK);
+    ap_request_core_filter_handle =
+        ap_register_output_filter("REQ_CORE", ap_request_core_filter,
+                                  NULL, AP_FTYPE_TRANSCODE);
     ap_subreq_core_filter_handle =
         ap_register_output_filter("SUBREQ_CORE", ap_sub_req_output_filter,
                                   NULL, AP_FTYPE_CONTENT_SET);

Modified: httpd/httpd/trunk/server/core_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core_filters.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core_filters.c (original)
+++ httpd/httpd/trunk/server/core_filters.c Sun Oct  4 10:10:51 2015
@@ -78,9 +78,7 @@ do { \
 #define APLOG_MODULE_INDEX AP_CORE_MODULE_INDEX
 
 struct core_output_filter_ctx {
-    apr_bucket_brigade *buffered_bb;
     apr_bucket_brigade *tmp_flush_bb;
-    apr_pool_t *deferred_write_pool;
     apr_size_t bytes_written;
 };
 
@@ -328,11 +326,6 @@ apr_status_t ap_core_input_filter(ap_fil
     return APR_SUCCESS;
 }
 
-static void setaside_remaining_output(ap_filter_t *f,
-                                      core_output_filter_ctx_t *ctx,
-                                      apr_bucket_brigade *bb,
-                                      conn_rec *c);
-
 static apr_status_t send_brigade_nonblocking(apr_socket_t *s,
                                              apr_bucket_brigade *bb,
                                              apr_size_t *bytes_written,
@@ -358,33 +351,23 @@ static apr_status_t sendfile_nonblocking
                                          conn_rec *c);
 #endif
 
-/* XXX: Should these be configurable parameters? */
-#define THRESHOLD_MIN_WRITE 4096
-#define THRESHOLD_MAX_BUFFER 65536
-#define MAX_REQUESTS_IN_PIPELINE 5
-
 /* Optional function coming from mod_logio, used for logging of output
  * traffic
  */
 extern APR_OPTIONAL_FN_TYPE(ap_logio_add_bytes_out) *ap__logio_add_bytes_out;
 
-apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
+apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *bb)
 {
     conn_rec *c = f->c;
     core_net_rec *net = f->ctx;
     core_output_filter_ctx_t *ctx = net->out_ctx;
-    apr_bucket_brigade *bb = NULL;
-    apr_bucket *bucket, *next, *flush_upto = NULL;
-    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
-    int eor_buckets_in_brigade, morphing_bucket_in_brigade;
+    apr_bucket *flush_upto = NULL;
     apr_status_t rv;
     int loglevel = ap_get_conn_module_loglevel(c, APLOG_MODULE_INDEX);
 
     /* Fail quickly if the connection has already been aborted. */
     if (c->aborted) {
-        if (new_bb != NULL) {
-            apr_brigade_cleanup(new_bb);
-        }
+        apr_brigade_cleanup(bb);
         return APR_ECONNABORTED;
     }
 
@@ -397,33 +380,14 @@ apr_status_t ap_core_output_filter(ap_fi
          * allocated from bb->pool which might be wrong.
          */
         ctx->tmp_flush_bb = apr_brigade_create(c->pool, c->bucket_alloc);
-        /* same for buffered_bb and ap_save_brigade */
-        ctx->buffered_bb = apr_brigade_create(c->pool, c->bucket_alloc);
-    }
-
-    if (new_bb != NULL)
-        bb = new_bb;
-
-    if ((ctx->buffered_bb != NULL) &&
-        !APR_BRIGADE_EMPTY(ctx->buffered_bb)) {
-        if (new_bb != NULL) {
-            APR_BRIGADE_PREPEND(bb, ctx->buffered_bb);
-        }
-        else {
-            bb = ctx->buffered_bb;
-        }
-        c->data_in_output_filters = 0;
-    }
-    else if (new_bb == NULL) {
-        return APR_SUCCESS;
     }
 
     /* Scan through the brigade and decide whether to attempt a write,
      * and how much to write, based on the following rules:
      *
-     *  1) The new_bb is null: Do a nonblocking write of as much as
+     *  1) The bb is empty: Do a nonblocking write of as much as
      *     possible: do a nonblocking write of as much data as possible,
-     *     then save the rest in ctx->buffered_bb.  (If new_bb == NULL,
+     *     then save the rest in ctx->buffered_bb.  (If bb is empty,
      *     it probably means that the MPM is doing asynchronous write
      *     completion and has just determined that this connection
      *     is writable.)
@@ -459,91 +423,12 @@ apr_status_t ap_core_output_filter(ap_fi
      *  3) Actually do the blocking write up to the last bucket determined
      *     by rules 2a-d. The point of doing only one flush is to make as
      *     few calls to writev() as possible.
-     *
-     *  4) If the brigade contains at least THRESHOLD_MIN_WRITE
-     *     bytes: Do a nonblocking write of as much data as possible,
-     *     then save the rest in ctx->buffered_bb.
      */
 
-    if (new_bb == NULL) {
-        rv = send_brigade_nonblocking(net->client_socket, bb,
-                                      &(ctx->bytes_written), c);
-        if (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv)) {
-            /* The client has aborted the connection */
-            ap_log_cerror(APLOG_MARK, APLOG_TRACE1, rv, c,
-                          "core_output_filter: writing data to the network");
-            apr_brigade_cleanup(bb);
-            c->aborted = 1;
-            return rv;
-        }
-        setaside_remaining_output(f, ctx, bb, c);
-        return APR_SUCCESS;
-    }
-
-    bytes_in_brigade = 0;
-    non_file_bytes_in_brigade = 0;
-    eor_buckets_in_brigade = 0;
-    morphing_bucket_in_brigade = 0;
-
-    for (bucket = APR_BRIGADE_FIRST(bb); bucket != APR_BRIGADE_SENTINEL(bb);
-         bucket = next) {
-        next = APR_BUCKET_NEXT(bucket);
-
-        if (!APR_BUCKET_IS_METADATA(bucket)) {
-            if (bucket->length == (apr_size_t)-1) {
-                /*
-                 * A setaside of morphing buckets would read everything into
-                 * memory. Instead, we will flush everything up to and
-                 * including this bucket.
-                 */
-                morphing_bucket_in_brigade = 1;
-            }
-            else {
-                bytes_in_brigade += bucket->length;
-                if (!APR_BUCKET_IS_FILE(bucket))
-                    non_file_bytes_in_brigade += bucket->length;
-            }
-        }
-        else if (AP_BUCKET_IS_EOR(bucket)) {
-            eor_buckets_in_brigade++;
-        }
-
-        if (APR_BUCKET_IS_FLUSH(bucket)
-            || non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER
-            || morphing_bucket_in_brigade
-            || eor_buckets_in_brigade > MAX_REQUESTS_IN_PIPELINE) {
-            /* this segment of the brigade MUST be sent before returning. */
-
-            if (loglevel >= APLOG_TRACE6) {
-                char *reason = APR_BUCKET_IS_FLUSH(bucket) ?
-                               "FLUSH bucket" :
-                               (non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER) ?
-                               "THRESHOLD_MAX_BUFFER" :
-                               morphing_bucket_in_brigade ? "morphing bucket" :
-                               "MAX_REQUESTS_IN_PIPELINE";
-                ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, c,
-                              "will flush because of %s", reason);
-                ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, c,
-                              "seen in brigade%s: bytes: %" APR_SIZE_T_FMT
-                              ", non-file bytes: %" APR_SIZE_T_FMT ", eor "
-                              "buckets: %d, morphing buckets: %d",
-                              flush_upto == NULL ? " so far"
-                                                 : " since last flush point",
-                              bytes_in_brigade,
-                              non_file_bytes_in_brigade,
-                              eor_buckets_in_brigade,
-                              morphing_bucket_in_brigade);
-            }
-            /*
-             * Defer the actual blocking write to avoid doing many writes.
-             */
-            flush_upto = next;
+    ap_filter_reinstate_brigade(f, bb, &flush_upto);
 
-            bytes_in_brigade = 0;
-            non_file_bytes_in_brigade = 0;
-            eor_buckets_in_brigade = 0;
-            morphing_bucket_in_brigade = 0;
-        }
+    if (APR_BRIGADE_EMPTY(bb)) {
+        return APR_SUCCESS;
     }
 
     if (flush_upto != NULL) {
@@ -571,71 +456,30 @@ apr_status_t ap_core_output_filter(ap_fi
         APR_BRIGADE_CONCAT(bb, ctx->tmp_flush_bb);
     }
 
+    rv = send_brigade_nonblocking(net->client_socket, bb, &(ctx->bytes_written),
+            c);
+    if ((rv != APR_SUCCESS) && (!APR_STATUS_IS_EAGAIN(rv))) {
+        /* The client has aborted the connection */
+        ap_log_cerror(
+                APLOG_MARK, APLOG_TRACE1, rv, c,
+                "core_output_filter: writing data to the network");
+        apr_brigade_cleanup(bb);
+        c->aborted = 1;
+        return rv;
+    }
     if (loglevel >= APLOG_TRACE8) {
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, c,
-                      "brigade contains: bytes: %" APR_SIZE_T_FMT
-                      ", non-file bytes: %" APR_SIZE_T_FMT
-                      ", eor buckets: %d, morphing buckets: %d",
-                      bytes_in_brigade, non_file_bytes_in_brigade,
-                      eor_buckets_in_brigade, morphing_bucket_in_brigade);
+        ap_log_cerror(
+                APLOG_MARK, APLOG_TRACE8, 0, c,
+                "tried nonblocking write, total bytes "
+                "written: %" APR_SIZE_T_FMT, ctx->bytes_written);
     }
 
-    if (bytes_in_brigade >= THRESHOLD_MIN_WRITE) {
-        rv = send_brigade_nonblocking(net->client_socket, bb,
-                                      &(ctx->bytes_written), c);
-        if ((rv != APR_SUCCESS) && (!APR_STATUS_IS_EAGAIN(rv))) {
-            /* The client has aborted the connection */
-            ap_log_cerror(APLOG_MARK, APLOG_TRACE1, rv, c,
-                          "core_output_filter: writing data to the network");
-            apr_brigade_cleanup(bb);
-            c->aborted = 1;
-            return rv;
-        }
-        if (loglevel >= APLOG_TRACE8) {
-                ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, c,
-                              "tried nonblocking write, total bytes "
-                              "written: %" APR_SIZE_T_FMT,
-                              ctx->bytes_written);
-        }
-    }
+    remove_empty_buckets(bb);
+    ap_filter_setaside_brigade(f, bb);
 
-    setaside_remaining_output(f, ctx, bb, c);
     return APR_SUCCESS;
 }
 
-/*
- * This function assumes that either ctx->buffered_bb == NULL, or
- * ctx->buffered_bb is empty, or ctx->buffered_bb == bb
- */
-static void setaside_remaining_output(ap_filter_t *f,
-                                      core_output_filter_ctx_t *ctx,
-                                      apr_bucket_brigade *bb,
-                                      conn_rec *c)
-{
-    if (bb == NULL) {
-        return;
-    }
-    remove_empty_buckets(bb);
-    if (!APR_BRIGADE_EMPTY(bb)) {
-        c->data_in_output_filters = 1;
-        if (bb != ctx->buffered_bb) {
-            if (!ctx->deferred_write_pool) {
-                apr_pool_create(&ctx->deferred_write_pool, c->pool);
-                apr_pool_tag(ctx->deferred_write_pool, "deferred_write");
-            }
-            ap_save_brigade(f, &(ctx->buffered_bb), &bb,
-                            ctx->deferred_write_pool);
-        }
-    }
-    else if (ctx->deferred_write_pool) {
-        /*
-         * There are no more requests in the pipeline. We can just clear the
-         * pool.
-         */
-        apr_pool_clear(ctx->deferred_write_pool);
-    }
-}
-
 #ifndef APR_MAX_IOVEC_SIZE
 #define MAX_IOVEC_TO_WRITE 16
 #else

Modified: httpd/httpd/trunk/server/mpm/event/event.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/event/event.c (original)
+++ httpd/httpd/trunk/server/mpm/event/event.c Sun Oct  4 10:10:51 2015
@@ -1146,19 +1146,38 @@ read_request:
     }
 
     if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
-        ap_filter_t *output_filter = c->output_filters;
-        apr_status_t rv;
+        apr_hash_index_t *rindex;
+        apr_status_t rv = APR_SUCCESS;
+        int data_in_output_filters = 0;
         ap_update_child_status_from_conn(sbh, SERVER_BUSY_WRITE, c);
-        while (output_filter->next != NULL) {
-            output_filter = output_filter->next;
+
+        rindex = apr_hash_first(NULL, c->filters);
+        while (rindex) {
+            ap_filter_t *f = apr_hash_this_val(rindex);
+
+            if (!APR_BRIGADE_EMPTY(f->bb)) {
+
+                rv = ap_pass_brigade(f, c->empty);
+                apr_brigade_cleanup(c->empty);
+                if (APR_SUCCESS != rv) {
+                    ap_log_cerror(
+                            APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(00470)
+                            "write failure in '%s' output filter", f->frec->name);
+                    break;
+                }
+
+                if (ap_filter_should_yield(f)) {
+                    data_in_output_filters = 1;
+                }
+            }
+
+            rindex = apr_hash_next(rindex);
         }
-        rv = output_filter->frec->filter_func.out_func(output_filter, NULL);
+
         if (rv != APR_SUCCESS) {
-            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(00470)
-                          "network write failure in core output filter");
             cs->pub.state = CONN_STATE_LINGER;
         }
-        else if (c->data_in_output_filters) {
+        else if (data_in_output_filters) {
             /* Still in WRITE_COMPLETION_STATE:
              * Set a write timeout for this connection, and let the
              * event thread poll for writeability.

Modified: httpd/httpd/trunk/server/mpm/motorz/motorz.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/motorz/motorz.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/motorz/motorz.c (original)
+++ httpd/httpd/trunk/server/mpm/motorz/motorz.c Sun Oct  4 10:10:51 2015
@@ -359,21 +359,38 @@ static apr_status_t motorz_io_process(mo
         }
 
         if (scon->cs.state == CONN_STATE_WRITE_COMPLETION) {
-            ap_filter_t *output_filter = c->output_filters;
-            ap_update_child_status_from_conn(scon->sbh, SERVER_BUSY_WRITE, c);
-            while (output_filter->next != NULL) {
-                output_filter = output_filter->next;
-            }
+            apr_hash_index_t *rindex;
+            apr_status_t rv = APR_SUCCESS;
+            int data_in_output_filters = 0;
+            ap_update_child_status_from_conn(sbh, SERVER_BUSY_WRITE, c);
+
+            rindex = apr_hash_first(NULL, c->filters);
+            while (rindex) {
+                ap_filter_t *f = apr_hash_this_val(rindex);
+
+                if (!APR_BRIGADE_EMPTY(f->bb)) {
 
-            rv = output_filter->frec->filter_func.out_func(output_filter,
-                                                           NULL);
+                    rv = ap_pass_brigade(f, c->empty);
+                    apr_brigade_cleanup(c->empty);
+                    if (APR_SUCCESS != rv) {
+                        ap_log_cerror(
+                                APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(02848)
+                                "write failure in '%s' output filter", f->frec->name);
+                        break;
+                    }
+
+                    if (ap_filter_should_yield(f)) {
+                        data_in_output_filters = 1;
+                    }
+                }
+
+                rindex = apr_hash_next(rindex);
+            }
 
             if (rv != APR_SUCCESS) {
-                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, ap_server_conf, APLOGNO(02848)
-                             "network write failure in core output filter");
                 scon->cs.state = CONN_STATE_LINGER;
             }
-            else if (c->data_in_output_filters) {
+            else if (data_in_output_filters) {
                 /* Still in WRITE_COMPLETION_STATE:
                  * Set a write timeout for this connection, and let the
                  * event thread poll for writeability.

Modified: httpd/httpd/trunk/server/mpm/simple/simple_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/simple/simple_io.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/simple/simple_io.c (original)
+++ httpd/httpd/trunk/server/mpm/simple/simple_io.c Sun Oct  4 10:10:51 2015
@@ -92,20 +92,37 @@ static apr_status_t simple_io_process(si
         }
 
         if (scon->cs.state == CONN_STATE_WRITE_COMPLETION) {
-            ap_filter_t *output_filter = c->output_filters;
-            while (output_filter->next != NULL) {
-                output_filter = output_filter->next;
-            }
+            apr_hash_index_t *rindex;
+            apr_status_t rv = APR_SUCCESS;
+            int data_in_output_filters = 0;
+
+            rindex = apr_hash_first(NULL, c->filters);
+            while (rindex) {
+                ap_filter_t *f = apr_hash_this_val(rindex);
+
+                if (!APR_BRIGADE_EMPTY(f->bb)) {
 
-            rv = output_filter->frec->filter_func.out_func(output_filter,
-                                                           NULL);
+                    rv = ap_pass_brigade(f, c->empty);
+                    apr_brigade_cleanup(c->empty);
+                    if (APR_SUCCESS != rv) {
+                        ap_log_cerror(
+                                APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(00249)
+                                "write failure in '%s' output filter", f->frec->name);
+                        break;
+                    }
+
+                    if (ap_filter_should_yield(f)) {
+                        data_in_output_filters = 1;
+                    }
+                }
+
+                rindex = apr_hash_next(rindex);
+            }
 
             if (rv != APR_SUCCESS) {
-                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, ap_server_conf, APLOGNO(00249)
-                             "network write failure in core output filter");
                 scon->cs.state = CONN_STATE_LINGER;
             }
-            else if (c->data_in_output_filters) {
+            else if (data_in_output_filters) {
                 /* Still in WRITE_COMPLETION_STATE:
                  * Set a write timeout for this connection, and let the
                  * event thread poll for writeability.

Modified: httpd/httpd/trunk/server/request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/request.c (original)
+++ httpd/httpd/trunk/server/request.c Sun Oct  4 10:10:51 2015
@@ -2036,6 +2036,64 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
     return APR_SUCCESS;
 }
 
+AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
+                                                            apr_bucket_brigade *bb)
+{
+    apr_bucket *flush_upto = NULL;
+    apr_status_t status = APR_SUCCESS;
+    apr_bucket_brigade *tmp_bb = f->ctx;
+
+    if (!tmp_bb) {
+        tmp_bb = f->ctx = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
+    }
+
+    /* Reinstate any buffered content */
+    ap_filter_reinstate_brigade(f, bb, &flush_upto);
+
+    while (!APR_BRIGADE_EMPTY(bb)) {
+        apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
+
+        /* if the core has set aside data, back off and try later */
+        if (!flush_upto) {
+            if (ap_filter_should_yield(f)) {
+                break;
+            }
+        }
+        else if (flush_upto == bucket) {
+            flush_upto = NULL;
+        }
+
+        /* have we found a morphing bucket? if so, force it to morph into something
+         * safe to pass down to the connection filters without needing to be set
+         * aside.
+         */
+        if (!APR_BUCKET_IS_METADATA(bucket)) {
+            if (bucket->length == (apr_size_t) - 1) {
+                const char *data;
+                apr_size_t size;
+                if (APR_SUCCESS
+                        != (status = apr_bucket_read(bucket, &data, &size,
+                                APR_BLOCK_READ))) {
+                    return status;
+                }
+            }
+        }
+
+        /* pass each bucket down the chain */
+        APR_BUCKET_REMOVE(bucket);
+        APR_BRIGADE_INSERT_TAIL(tmp_bb, bucket);
+
+        status = ap_pass_brigade(f->next, tmp_bb);
+        if (!APR_STATUS_IS_EOF(status) && (status != APR_SUCCESS)) {
+            return status;
+        }
+
+    }
+
+    ap_filter_setaside_brigade(f, bb);
+    return status;
+}
+
 extern APR_OPTIONAL_FN_TYPE(authz_some_auth_required) *ap__authz_ap_some_auth_required;
 
 AP_DECLARE(int) ap_some_auth_required(request_rec *r)

Modified: httpd/httpd/trunk/server/util_filter.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?rev=1706669&r1=1706668&r2=1706669&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_filter.c (original)
+++ httpd/httpd/trunk/server/util_filter.c Sun Oct  4 10:10:51 2015
@@ -24,6 +24,7 @@
 #include "http_config.h"
 #include "http_core.h"
 #include "http_log.h"
+#include "http_request.h"
 #include "util_filter.h"
 
 /* NOTE: Apache's current design doesn't allow a pool to be passed thru,
@@ -32,6 +33,10 @@
 #define FILTER_POOL     apr_hook_global_pool
 #include "ap_hooks.h"   /* for apr_hook_global_pool */
 
+/* XXX: Should these be configurable parameters? */
+#define THRESHOLD_MAX_BUFFER 65536
+#define MAX_REQUESTS_IN_PIPELINE 5
+
 /*
 ** This macro returns true/false if a given filter should be inserted BEFORE
 ** another filter. This will happen when one of: 1) there isn't another
@@ -319,6 +324,8 @@ static ap_filter_t *add_any_filter_handl
     f->r = frec->ftype < AP_FTYPE_CONNECTION ? r : NULL;
     f->c = c;
     f->next = NULL;
+    f->bb = NULL;
+    f->deferred_pool = NULL;
 
     if (INSERT_BEFORE(f, *outf)) {
         f->next = *outf;
@@ -474,6 +481,16 @@ AP_DECLARE(void) ap_remove_input_filter(
 
 AP_DECLARE(void) ap_remove_output_filter(ap_filter_t *f)
 {
+
+    if ((f->bb) && !APR_BRIGADE_EMPTY(f->bb)) {
+        apr_brigade_cleanup(f->bb);
+    }
+
+    if (f->deferred_pool) {
+        apr_pool_destroy(f->deferred_pool);
+        f->deferred_pool = NULL;
+    }
+
     remove_any_filter(f, f->r ? &f->r->output_filters : NULL,
                       f->r ? &f->r->proto_output_filters : NULL,
                       &f->c->output_filters);
@@ -566,6 +583,7 @@ AP_DECLARE(apr_status_t) ap_pass_brigade
 {
     if (next) {
         apr_bucket *e;
+
         if ((e = APR_BRIGADE_LAST(bb)) && APR_BUCKET_IS_EOS(e) && next->r) {
             /* This is only safe because HTTP_HEADER filter is always in
              * the filter stack.   This ensures that there is ALWAYS a
@@ -635,7 +653,8 @@ AP_DECLARE(apr_status_t) ap_save_brigade
     apr_status_t rv, srv = APR_SUCCESS;
 
     /* If have never stored any data in the filter, then we had better
-     * create an empty bucket brigade so that we can concat.
+     * create an empty bucket brigade so that we can concat. Register
+     * a cleanup to zero out the pointer if the pool is cleared.
      */
     if (!(*saveto)) {
         *saveto = apr_brigade_create(p, f->c->bucket_alloc);
@@ -673,6 +692,248 @@ AP_DECLARE(apr_status_t) ap_save_brigade
     return srv;
 }
 
+static apr_status_t filters_cleanup(void *data)
+{
+    ap_filter_t **key = data;
+
+    apr_hash_set((*key)->c->filters, key, sizeof(ap_filter_t **), NULL);
+
+    return APR_SUCCESS;
+}
+
+AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
+        apr_bucket_brigade *bb)
+{
+    int loglevel = ap_get_conn_module_loglevel(f->c, APLOG_MODULE_INDEX);
+
+    if (loglevel >= APLOG_TRACE6) {
+        ap_log_cerror(
+            APLOG_MARK, APLOG_TRACE6, 0, f->c,
+            "setaside %s brigade to %s brigade in '%s' output filter",
+            (APR_BRIGADE_EMPTY(bb) ? "empty" : "full"),
+            (!f->bb || APR_BRIGADE_EMPTY(f->bb) ? "empty" : "full"), f->frec->name);
+    }
+
+    if (!APR_BRIGADE_EMPTY(bb)) {
+        apr_pool_t *pool;
+        /*
+         * Set aside the brigade bb within f->bb.
+         */
+        if (!f->bb) {
+            ap_filter_t **key;
+
+            pool = f->r ? f->r->pool : f->c->pool;
+
+            key = apr_palloc(pool, sizeof(ap_filter_t **));
+            *key = f;
+            apr_hash_set(f->c->filters, key, sizeof(ap_filter_t **), f);
+
+            f->bb = apr_brigade_create(pool, f->c->bucket_alloc);
+
+            apr_pool_pre_cleanup_register(pool, key, filters_cleanup);
+
+        }
+
+        /* decide what pool we setaside to, request pool or deferred pool? */
+        if (f->r) {
+            pool = f->r->pool;
+            APR_BRIGADE_CONCAT(f->bb, bb);
+        }
+        else {
+            if (!f->deferred_pool) {
+                apr_pool_create(&f->deferred_pool, f->c->pool);
+                apr_pool_tag(f->deferred_pool, "deferred_pool");
+            }
+            pool = f->deferred_pool;
+            return ap_save_brigade(f, &f->bb, &bb, pool);
+        }
+
+    }
+    else if (f->deferred_pool) {
+        /*
+         * There are no more requests in the pipeline. We can just clear the
+         * pool.
+         */
+        apr_brigade_cleanup(f->bb);
+        apr_pool_clear(f->deferred_pool);
+    }
+    return APR_SUCCESS;
+}
+
+AP_DECLARE(apr_status_t) ap_filter_reinstate_brigade(ap_filter_t *f,
+                                                     apr_bucket_brigade *bb,
+                                                     apr_bucket **flush_upto)
+{
+    apr_bucket *bucket, *next;
+    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
+    int eor_buckets_in_brigade, morphing_bucket_in_brigade;
+    int loglevel = ap_get_conn_module_loglevel(f->c, APLOG_MODULE_INDEX);
+
+    if (loglevel >= APLOG_TRACE6) {
+        ap_log_cerror(
+            APLOG_MARK, APLOG_TRACE6, 0, f->c,
+            "reinstate %s brigade to %s brigade in '%s' output filter",
+            (!f->bb || APR_BRIGADE_EMPTY(f->bb) ? "empty" : "full"),
+            (APR_BRIGADE_EMPTY(bb) ? "empty" : "full"), f->frec->name);
+    }
+
+    if (f->bb && !APR_BRIGADE_EMPTY(f->bb)) {
+        APR_BRIGADE_PREPEND(bb, f->bb);
+    }
+
+    /*
+     * Determine if and up to which bucket we need to do a blocking write:
+     *
+     *  a) The brigade contains a flush bucket: Do a blocking write
+     *     of everything up that point.
+     *
+     *  b) The request is in CONN_STATE_HANDLER state, and the brigade
+     *     contains at least THRESHOLD_MAX_BUFFER bytes in non-file
+     *     buckets: Do blocking writes until the amount of data in the
+     *     buffer is less than THRESHOLD_MAX_BUFFER.  (The point of this
+     *     rule is to provide flow control, in case a handler is
+     *     streaming out lots of data faster than the data can be
+     *     sent to the client.)
+     *
+     *  c) The request is in CONN_STATE_HANDLER state, and the brigade
+     *     contains at least MAX_REQUESTS_IN_PIPELINE EOR buckets:
+     *     Do blocking writes until less than MAX_REQUESTS_IN_PIPELINE EOR
+     *     buckets are left. (The point of this rule is to prevent too many
+     *     FDs being kept open by pipelined requests, possibly allowing a
+     *     DoS).
+     *
+     *  d) The request is being served by a connection filter and the
+     *     brigade contains a morphing bucket: If there was no other
+     *     reason to do a blocking write yet, try reading the bucket. If its
+     *     contents fit into memory before THRESHOLD_MAX_BUFFER is reached,
+     *     everything is fine. Otherwise we need to do a blocking write the
+     *     up to and including the morphing bucket, because ap_save_brigade()
+     *     would read the whole bucket into memory later on.
+     */
+
+    *flush_upto = NULL;
+
+    bytes_in_brigade = 0;
+    non_file_bytes_in_brigade = 0;
+    eor_buckets_in_brigade = 0;
+    morphing_bucket_in_brigade = 0;
+
+    for (bucket = APR_BRIGADE_FIRST(bb); bucket != APR_BRIGADE_SENTINEL(bb);
+         bucket = next) {
+        next = APR_BUCKET_NEXT(bucket);
+
+        if (!APR_BUCKET_IS_METADATA(bucket)) {
+            if (bucket->length == (apr_size_t)-1) {
+                /*
+                 * A setaside of morphing buckets would read everything into
+                 * memory. Instead, we will flush everything up to and
+                 * including this bucket.
+                 */
+                morphing_bucket_in_brigade = 1;
+            }
+            else {
+                bytes_in_brigade += bucket->length;
+                if (!APR_BUCKET_IS_FILE(bucket))
+                    non_file_bytes_in_brigade += bucket->length;
+            }
+        }
+        else if (AP_BUCKET_IS_EOR(bucket)) {
+            eor_buckets_in_brigade++;
+        }
+
+        if (APR_BUCKET_IS_FLUSH(bucket)
+            || non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER
+            || (!f->r && morphing_bucket_in_brigade)
+            || eor_buckets_in_brigade > MAX_REQUESTS_IN_PIPELINE) {
+            /* this segment of the brigade MUST be sent before returning. */
+
+            if (loglevel >= APLOG_TRACE6) {
+                char *reason = APR_BUCKET_IS_FLUSH(bucket) ?
+                               "FLUSH bucket" :
+                               (non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER) ?
+                               "THRESHOLD_MAX_BUFFER" :
+                               (!f->r && morphing_bucket_in_brigade) ? "morphing bucket" :
+                               "MAX_REQUESTS_IN_PIPELINE";
+                ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, f->c,
+                              "will flush because of %s", reason);
+                ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, f->c,
+                              "seen in brigade%s: bytes: %" APR_SIZE_T_FMT
+                              ", non-file bytes: %" APR_SIZE_T_FMT ", eor "
+                              "buckets: %d, morphing buckets: %d",
+                              flush_upto == NULL ? " so far"
+                                                 : " since last flush point",
+                              bytes_in_brigade,
+                              non_file_bytes_in_brigade,
+                              eor_buckets_in_brigade,
+                              morphing_bucket_in_brigade);
+            }
+            /*
+             * Defer the actual blocking write to avoid doing many writes.
+             */
+            *flush_upto = next;
+
+            bytes_in_brigade = 0;
+            non_file_bytes_in_brigade = 0;
+            eor_buckets_in_brigade = 0;
+            morphing_bucket_in_brigade = 0;
+        }
+    }
+
+    if (loglevel >= APLOG_TRACE8) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, f->c,
+                      "brigade contains: bytes: %" APR_SIZE_T_FMT
+                      ", non-file bytes: %" APR_SIZE_T_FMT
+                      ", eor buckets: %d, morphing buckets: %d",
+                      bytes_in_brigade, non_file_bytes_in_brigade,
+                      eor_buckets_in_brigade, morphing_bucket_in_brigade);
+    }
+
+    return APR_SUCCESS;
+}
+
+AP_DECLARE(int) ap_filter_should_yield(ap_filter_t *f)
+{
+    /*
+     * This function decides whether a filter should yield due to buffered
+     * data in a downstream filter. If a downstream filter buffers we
+     * must back off so we don't overwhelm the server. If this function
+     * returns true, the filter should call ap_filter_setaside_brigade()
+     * to save unprocessed buckets, and then reinstate those buckets on
+     * the next call with ap_filter_reinstate_brigade() and continue
+     * where it left off.
+     *
+     * If this function is forced to return zero, we return back to
+     * synchronous filter behaviour.
+     *
+     * Subrequests present us with a problem - we don't know how much data
+     * they will produce and therefore how much buffering we'll need, and
+     * if a subrequest had to trigger buffering, but next subrequest wouldn't
+     * know when the previous one had finished sending data and buckets
+     * could be sent out of order.
+     *
+     * In the case of subrequests, deny the ability to yield. When the data
+     * reaches the filters from the main request, they will be setaside
+     * there in the right order and the request will be given the
+     * opportunity to yield.
+     */
+    if (f->r && f->r->main) {
+        return 0;
+    }
+
+    /*
+     * This is either a main request or internal redirect, or it is a
+     * connection filter. Yield if there is any buffered data downstream
+     * from us.
+     */
+    while (f) {
+        if (f->bb && !APR_BRIGADE_EMPTY(f->bb)) {
+            return 1;
+        }
+        f = f->next;
+    }
+    return 0;
+}
+
 AP_DECLARE_NONSTD(apr_status_t) ap_filter_flush(apr_bucket_brigade *bb,
                                                 void *ctx)
 {




Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Christophe JAILLET <ch...@wanadoo.fr>.
Le 04/10/2015 12:10, minfrin@apache.org a écrit :
> Author: minfrin
> Date: Sun Oct  4 10:10:51 2015
> New Revision: 1706669
>
> URL: http://svn.apache.org/viewvc?rev=1706669&view=rev
> Log:
> core: Extend support for asynchronous write completion from the
> network filter to any connection or request filter.
[...]
> Modified: httpd/httpd/trunk/server/util_filter.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?rev=1706669&r1=1706668&r2=1706669&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_filter.c (original)
> +++ httpd/httpd/trunk/server/util_filter.c Sun Oct  4 10:10:51 2015
[...]
> +AP_DECLARE(apr_status_t) ap_filter_reinstate_brigade(ap_filter_t *f,
> +                                                     apr_bucket_brigade *bb,
> +                                                     apr_bucket **flush_upto)
> +{
[...]
> +
> +    *flush_upto = NULL;
> +
[...]
> +                ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, f->c,
> +                              "seen in brigade%s: bytes: %" APR_SIZE_T_FMT
> +                              ", non-file bytes: %" APR_SIZE_T_FMT ", eor "
> +                              "buckets: %d, morphing buckets: %d",
> +                              flush_upto == NULL ? " so far"				<----------------------
> +                                                 : " since last flush point",
> +                              bytes_in_brigade,
> +                              non_file_bytes_in_brigade,
> +                              eor_buckets_in_brigade,
> +                              morphing_bucket_in_brigade);
> +            }
> +            /*
> +             * Defer the actual blocking write to avoid doing many writes.
> +             */
> +            *flush_upto = next;

Shouldn't the 'flush_upto' indicated here be a '*flush_upto'?
Obviously there is no impact, just TRACE8 message accuracy.

(spotted by smatch)

CJ

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 3:32 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Just wanted to run the tests framework on trunk for a local change but
> it seems that this commit (bisected to) makes many tests to segfault.

Forgot to mention, same thing with follow up r1706670 applied.

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 3:51 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Just did that, same thing.
> I was using mpm_worker, but now tried mpm_event with same segfaults.

FWIW, I'm running an (old) Debian 6.0.10 (squeeze, 64bit).

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 7:00 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Oct 6, 2015 at 6:29 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> all tests pass now.
>
> I spoke too soon...
>
> New segfault is in mod_substitute:

Fixed in r1707091.
Now all tests really pass :p

I wonder how we should address the possible (third-party-)breakage due
to ap_pass_brigade(c->ouput_filters, EOR) =>
ap_pass_brigade(r->ouput_filters, EOR) in
ap_process_request_after_handler(), should this be backported in
2.4.x...

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Oct 6, 2015, at 1:29 PM, Graham Leggett <mi...@sharp.fm> wrote:
> 
> On 06 Oct 2015, at 7:00 PM, Yann Ylavic <yl...@gmail.com> wrote:
> 
>> Yet another filter which does not remove itself after the EOS, and
>> destroys the EOR bucket while still using *r after.
> 
> I am wondering if the EOR bucket is a suboptimal way to clean up after ourselves.
> 
> The MPM itself is in a position to know when the filter buffers are all empty and it is safe to delete a request pool. I am imagining a “shutdown set”, which contains the pools which have been marked as ready to shut down. If there is a pool in the shutdown set, walk the outstanding filters and look for any filters whose pools are descendants of that pool, and check if the filters are empty. If so, apr_pool_destroy() and we’re done. If the “shutdown set” is empty, do nothing.
> 
> Instead of creating an EOR bucket, we call an MPM API that says “add this pool to the shutdown set”, and then forget about it.
> 
> Legacy MPMs fall back to EOR behaviour, but we deprecate it.
> 
> I think the above is a far safer approach than assuming filters will “do the right thing”.
> 

Basic idea seems sound... +1


Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Graham Leggett <mi...@sharp.fm>.
On 06 Oct 2015, at 7:29 PM, Graham Leggett <mi...@sharp.fm> wrote:

> I am wondering if the EOR bucket is a suboptimal way to clean up after ourselves.
> 
> The MPM itself is in a position to know when the filter buffers are all empty and it is safe to delete a request pool. I am imagining a “shutdown set”, which contains the pools which have been marked as ready to shut down. If there is a pool in the shutdown set, walk the outstanding filters and look for any filters whose pools are descendants of that pool, and check if the filters are empty. If so, apr_pool_destroy() and we’re done. If the “shutdown set” is empty, do nothing.
> 
> Instead of creating an EOR bucket, we call an MPM API that says “add this pool to the shutdown set”, and then forget about it.
> 
> Legacy MPMs fall back to EOR behaviour, but we deprecate it.
> 
> I think the above is a far safer approach than assuming filters will “do the right thing”.

Having done some further digging that isn’t that workable. While the presence of a filter with setaside data in it tells us definitely that we shouldn’t destroy the brigade yet, it is very difficult to determine when we should.

I think we’ll need to compromise and provide an async switch that in v2.4 defaults to “connection” (restrict async behaviour to connection filters only), and can be optionally set to “request” (async behaviour for connection and request filters) if the filters in use allow it.

Regards,
Graham
—


Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Graham Leggett <mi...@sharp.fm>.
On 06 Oct 2015, at 7:00 PM, Yann Ylavic <yl...@gmail.com> wrote:

> Yet another filter which does not remove itself after the EOS, and
> destroys the EOR bucket while still using *r after.

I am wondering if the EOR bucket is a suboptimal way to clean up after ourselves.

The MPM itself is in a position to know when the filter buffers are all empty and it is safe to delete a request pool. I am imagining a “shutdown set”, which contains the pools which have been marked as ready to shut down. If there is a pool in the shutdown set, walk the outstanding filters and look for any filters whose pools are descendants of that pool, and check if the filters are empty. If so, apr_pool_destroy() and we’re done. If the “shutdown set” is empty, do nothing.

Instead of creating an EOR bucket, we call an MPM API that says “add this pool to the shutdown set”, and then forget about it.

Legacy MPMs fall back to EOR behaviour, but we deprecate it.

I think the above is a far safer approach than assuming filters will “do the right thing”.

Regards,
Graham
—


Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 6:29 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> all tests pass now.

I spoke too soon...

New segfault is in mod_substitute:

Program terminated with signal 11, Segmentation fault.

(gdb) bt
#0  0x00007fa02fbaec2b in allocator_free (allocator=0x1cedcb0,
node=0x0) at memory/unix/apr_pools.c:421
#1  0x00007fa02fbaf3a2 in apr_pool_clear (pool=0x1d104e8) at
memory/unix/apr_pools.c:947
#2  0x00007fa02b2f1b79 in substitute_filter (f=0x1d0bdb8,
bb=0x1cf27e8) at mod_substitute.c:552
#3  0x000000000043ff57 in ap_pass_brigade (next=0x1d0bdb8,
bb=0x1cf27e8) at util_filter.c:608
#4  0x0000000000488ce4 in ap_process_request_after_handler
(r=0x1cf3e50) at http_request.c:275
#5  0x0000000000488ffc in ap_process_async_request (r=0x1cf3e50) at
http_request.c:371
#6  0x000000000048513d in ap_process_http_async_connection
(c=0x1cee0b8) at http_core.c:154
#7  0x0000000000485360 in ap_process_http_connection (c=0x1cee0b8) at
http_core.c:248
#8  0x0000000000478cc5 in ap_run_process_connection (c=0x1cee0b8) at
connection.c:41
#9  0x00007fa025d0b7e1 in process_socket (thd=0x1b2fcd8, p=0x1cedda8,
sock=0x1cede20, cs=0x1cee028, my_child_num=0, my_thread_num=0) at
event.c:1136
#10 0x00007fa025d0e334 in worker_thread (thd=0x1b2fcd8,
dummy=0x1c98c20) at event.c:2247
#11 0x00007fa02fbbe6af in dummy_worker (opaque=0x1b2fcd8) at
threadproc/unix/thread.c:145
#12 0x00007fa02f50f8ca in start_thread (arg=<optimized out>) at
pthread_create.c:300
#13 0x00007fa02ee4a08d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#14 0x0000000000000000 in ?? ()

Yet another filter which does not remove itself after the EOS, and
destroys the EOR bucket while still using *r after.

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 5:53 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Oct 6, 2015 at 5:34 PM, Graham Leggett <mi...@sharp.fm> wrote:
>>
>> apr_bucket_simple_copy() looks wrong - in theory we should have a proper copy function that does the right thing with the second copy, for example by not copying the pool. If we blindly copy the pool (or the request containing the pool) I see nothing that would prevent an attempt to free the pool twice.
>
> Agreed, we probably need something like this:

Committed in r1707084, all tests pass now.

AW: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Gesendet: Dienstag, 6. Oktober 2015 17:54
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/
> modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/
> server/mpm/simple/
> 
> On Tue, Oct 6, 2015 at 5:34 PM, Graham Leggett <mi...@sharp.fm> wrote:
> >
> > apr_bucket_simple_copy() looks wrong - in theory we should have a
> proper copy function that does the right thing with the second copy, for
> example by not copying the pool. If we blindly copy the pool (or the
> request containing the pool) I see nothing that would prevent an attempt
> to free the pool twice.
> 
> Agreed, we probably need something like this:
> 
> Index: server/eor_bucket.c
> ===================================================================
> --- server/eor_bucket.c    (revision 1707064)
> +++ server/eor_bucket.c    (working copy)
> @@ -91,6 +91,17 @@ static void eor_bucket_destroy(void *data)
>      }
>  }
> 
> +static apr_status_t eor_bucket_copy(apr_bucket *a, apr_bucket **b)
> +{
> +    *b = apr_bucket_alloc(sizeof(**b), a->list); /* XXX: check for
> failure? */
> +    **b = *a;
> +

We could use apr_bucket_simple_copy(a, b) instead of the above.

> +    /* we don't wan't request to be destroyed twice */
> +    (*b)->data = NULL;

Hm. Shouldn't the last EOR bucket of a particular request destroyed call
eor_bucket_cleanup? That would require some kind of a reference like refcount buckets provide.

> +
> +    return APR_SUCCESS;
> +}
> +
>  AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
>      "EOR", 5, APR_BUCKET_METADATA,
>      eor_bucket_destroy,
> @@ -97,6 +108,6 @@ AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_


Regards

Rüdiger

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Jim Jagielski <ji...@jaguNET.com>.
+1 (concept)

> On Oct 6, 2015, at 11:53 AM, Yann Ylavic <yl...@gmail.com> wrote:
> 
> On Tue, Oct 6, 2015 at 5:34 PM, Graham Leggett <mi...@sharp.fm> wrote:
>> 
>> apr_bucket_simple_copy() looks wrong - in theory we should have a proper copy function that does the right thing with the second copy, for example by not copying the pool. If we blindly copy the pool (or the request containing the pool) I see nothing that would prevent an attempt to free the pool twice.
> 
> Agreed, we probably need something like this:
> 
> Index: server/eor_bucket.c
> ===================================================================
> --- server/eor_bucket.c    (revision 1707064)
> +++ server/eor_bucket.c    (working copy)
> @@ -91,6 +91,17 @@ static void eor_bucket_destroy(void *data)
>     }
> }
> 
> +static apr_status_t eor_bucket_copy(apr_bucket *a, apr_bucket **b)
> +{
> +    *b = apr_bucket_alloc(sizeof(**b), a->list); /* XXX: check for failure? */
> +    **b = *a;
> +
> +    /* we don't wan't request to be destroyed twice */
> +    (*b)->data = NULL;
> +
> +    return APR_SUCCESS;
> +}
> +
> AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
>     "EOR", 5, APR_BUCKET_METADATA,
>     eor_bucket_destroy,
> @@ -97,6 +108,6 @@ AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_
>     eor_bucket_read,
>     apr_bucket_setaside_noop,
>     apr_bucket_split_notimpl,
> -    apr_bucket_simple_copy
> +    eor_bucket_copy
> };
> 
> --
> 
> Regards,
> Yann.


Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 5:34 PM, Graham Leggett <mi...@sharp.fm> wrote:
>
> apr_bucket_simple_copy() looks wrong - in theory we should have a proper copy function that does the right thing with the second copy, for example by not copying the pool. If we blindly copy the pool (or the request containing the pool) I see nothing that would prevent an attempt to free the pool twice.

Agreed, we probably need something like this:

Index: server/eor_bucket.c
===================================================================
--- server/eor_bucket.c    (revision 1707064)
+++ server/eor_bucket.c    (working copy)
@@ -91,6 +91,17 @@ static void eor_bucket_destroy(void *data)
     }
 }

+static apr_status_t eor_bucket_copy(apr_bucket *a, apr_bucket **b)
+{
+    *b = apr_bucket_alloc(sizeof(**b), a->list); /* XXX: check for failure? */
+    **b = *a;
+
+    /* we don't wan't request to be destroyed twice */
+    (*b)->data = NULL;
+
+    return APR_SUCCESS;
+}
+
 AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
     "EOR", 5, APR_BUCKET_METADATA,
     eor_bucket_destroy,
@@ -97,6 +108,6 @@ AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_
     eor_bucket_read,
     apr_bucket_setaside_noop,
     apr_bucket_split_notimpl,
-    apr_bucket_simple_copy
+    eor_bucket_copy
 };

--

Regards,
Yann.

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Graham Leggett <mi...@sharp.fm>.
On 06 Oct 2015, at 5:22 PM, Yann Ylavic <yl...@gmail.com> wrote:

>> Just did that, same thing.
>> I was using mpm_worker, but now tried mpm_event with same segfaults.
> 
> Looks like mod_bucketeer is the culprit.
> It fails to remove itself from the filter stack on EOS, and hence
> makes a copy of the EOR bucket (as any METADATA bucket) for its own
> brigade when it sees it.
> Then the request gets destroyed twice (once per brigade it is in) on cleanup.
> 
> I fail to see where your patch comes into play here though, for now.
> I also suspect EOR buckets shouldn't be copyable anyway, and bucketeer
> should do the right thing, still the new filter should probably work
> with the existing...

The core used to inject EOR buckets into the connection filters, bypassing the request filters. Now that request filters can buffer data, injecting EOR into the connection filters causes the end of requests to be chopped off, so now EOR is injected at the very end of the last request filter, as it should have been.

In theory none of this should matter, if EOR is copied it should set up the proper cleanups to clean up the second copy. A quick look at eor_bucket.c shows the following:

AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
    "EOR", 5, APR_BUCKET_METADATA,
    eor_bucket_destroy,
    eor_bucket_read,
    apr_bucket_setaside_noop,
    apr_bucket_split_notimpl,
    apr_bucket_simple_copy
};

apr_bucket_simple_copy() looks wrong - in theory we should have a proper copy function that does the right thing with the second copy, for example by not copying the pool. If we blindly copy the pool (or the request containing the pool) I see nothing that would prevent an attempt to free the pool twice.

Regards,
Graham
—


Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 5:22 PM, Yann Ylavic <yl...@gmail.com> wrote:
> still the new filter should probably work
> with the existing...

s/new filter/new filter stack mechanism/

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 3:51 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Oct 6, 2015 at 3:41 PM, Graham Leggett <mi...@sharp.fm> wrote:
>> On 6 Oct 2015, at 14:32, Yann Ylavic <yl...@gmail.com> wrote:
>>
>>> So it seems to relate to the EOR bucket, first not being passed
>>> through, and second leading to a double-free or alike.
>>>
>>> Did not investigate further...
>>
>> Did you rebuild you tree completely clean before trying it out?
>>
>> Whenever I got weird behavior a clean rebuild sorted it out. The full test suite works fine for me using the event mpm.
>
> Just did that, same thing.
> I was using mpm_worker, but now tried mpm_event with same segfaults.

Looks like mod_bucketeer is the culprit.
It fails to remove itself from the filter stack on EOS, and hence
makes a copy of the EOR bucket (as any METADATA bucket) for its own
brigade when it sees it.
Then the request gets destroyed twice (once per brigade it is in) on cleanup.

I fail to see where your patch comes into play here though, for now.
I also suspect EOR buckets shouldn't be copyable anyway, and bucketeer
should do the right thing, still the new filter should probably work
with the existing...

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 3:41 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 6 Oct 2015, at 14:32, Yann Ylavic <yl...@gmail.com> wrote:
>
>> So it seems to relate to the EOR bucket, first not being passed
>> through, and second leading to a double-free or alike.
>>
>> Did not investigate further...
>
> Did you rebuild you tree completely clean before trying it out?
>
> Whenever I got weird behavior a clean rebuild sorted it out. The full test suite works fine for me using the event mpm.

Just did that, same thing.
I was using mpm_worker, but now tried mpm_event with same segfaults.

Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Graham Leggett <mi...@sharp.fm>.
On 6 Oct 2015, at 14:32, Yann Ylavic <yl...@gmail.com> wrote:

> So it seems to relate to the EOR bucket, first not being passed
> through, and second leading to a double-free or alike.
> 
> Did not investigate further...

Did you rebuild you tree completely clean before trying it out?

Whenever I got weird behavior a clean rebuild sorted it out. The full test suite works fine for me using the event mpm.

Regards,
Graham
--


Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Oct 4, 2015 at 12:10 PM,  <mi...@apache.org> wrote:
> Author: minfrin
> Date: Sun Oct  4 10:10:51 2015
> New Revision: 1706669
>
> URL: http://svn.apache.org/viewvc?rev=1706669&view=rev
> Log:
> core: Extend support for asynchronous write completion from the
> network filter to any connection or request filter.

Just wanted to run the tests framework on trunk for a local change but
it seems that this commit (bisected to) makes many tests to segfault.

Here is the backtrace:

Program terminated with signal 6, Aborted.
(gdb) bt
#0  0x00007f1984bd1ed5 in *__GI_raise (sig=<optimized out>) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007f1984bd4ce0 in *__GI_abort () at abort.c:92
#2  0x00007f1984c0b9db in __libc_message (do_abort=<optimized out>,
fmt=<optimized out>) at ../sysdeps/unix/sysv/linux/libc_fatal.c:189
#3  0x00007f1984c15236 in malloc_printerr (action=3,
str=0x7f1984ccce7e "corrupted double-linked list", ptr=<optimized
out>) at malloc.c:6296
#4  0x00007f1984c1566d in malloc_consolidate (av=<optimized out>) at
malloc.c:5174
#5  0x00007f1984c16b78 in _int_free (av=0x7f1984f04e40, p=0xc9f140) at
malloc.c:5047
#6  0x00007f1984c1a01c in *__GI___libc_free (mem=<optimized out>) at
malloc.c:3739
#7  0x00007f19859dad6f in allocator_free (allocator=0x9cf580,
node=0xc9f150) at memory/unix/apr_pools.c:474
#8  0x00007f19859db527 in apr_pool_destroy (pool=0xc91108) at
memory/unix/apr_pools.c:1016
#9  0x0000000000459dcf in eor_bucket_destroy (data=0xc91180) at eor_bucket.c:90
#10 0x00007f19859a6517 in apr_brigade_cleanup (data=0xc99b40) at
buckets/apr_brigade.c:51
#11 0x0000000000488c20 in ap_process_request_after_handler
(r=0xc91180) at http_request.c:283
#12 0x0000000000488f2c in ap_process_async_request (r=0xc91180) at
http_request.c:371
#13 0x0000000000488f58 in ap_process_request (r=0xc91180) at http_request.c:381
#14 0x0000000000485190 in ap_process_http_sync_connection (c=0xb21168)
at http_core.c:210
#15 0x000000000048529e in ap_process_http_connection (c=0xb21168) at
http_core.c:251
#16 0x0000000000478bf5 in ap_run_process_connection (c=0xb21168) at
connection.c:41
#17 0x00000000004790d7 in ap_process_connection (c=0xb21168,
csd=0xb20f50) at connection.c:206
#18 0x00007f197bb3b8fb in process_socket (thd=0x944940, p=0xb20ec8,
sock=0xb20f50, my_child_num=0, my_thread_num=0, bucket_alloc=0xc4aed8)
at worker.c:632
#19 0x00007f197bb3c576 in worker_thread (thd=0x944940, dummy=0x9c8b90)
at worker.c:992
#20 0x00007f19859ea6af in dummy_worker (opaque=0x944940) at
threadproc/unix/thread.c:145
#21 0x00007f198533b8ca in start_thread (arg=<optimized out>) at
pthread_create.c:300
#22 0x00007f1984c7608d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#23 0x0000000000000000 in ?? ()
(gdb) frame 11
#11 0x0000000000488c20 in ap_process_request_after_handler
(r=0xc91180) at http_request.c:283
283         apr_brigade_cleanup(bb);
(gdb) l 280
275         ap_pass_brigade(r->output_filters, bb);
276
277         /* The EOR bucket has either been handled by an output filter (eg.
278          * deleted or moved to a buffered_bb => no more in bb), or an error
279          * occured before that (eg. c->aborted => still in bb) and we ought
280          * to destroy it now. So cleanup any remaining bucket along with
281          * the orphan request (if any).
282          */
283         apr_brigade_cleanup(bb);
284

So it seems to relate to the EOR bucket, first not being passed
through, and second leading to a double-free or alike.

Did not investigate further...

Regards,
Yann.