util_filter: keep filters with aside buckets in order.

Read or write of filter's pending data must happen in the same order as the
filter chain, thus we can't use an apr_hash_t to maintain the pending filters
since it provides no garantee on this matter.

Instead use an APR_RING maintained in c->pending_filters, and since both the
name (was c->filters) and the type changed, MAJOR is bumped (trunk only code
anyway so far).

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

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 1db1a6de6e114382e5de4cbe6f6c000b63dd7864..718b5223f87d8b1ddd8db3b3803a95463e195d03 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -576,12 +576,16 @@
  * 20180417.3 (2.5.1-dev)  Add ap_fgetline() and AP_GETLINE_NONBLOCK flag
  * 20180422.1 (2.5.1-dev)  Axe ap_rgetline_core()
  * 20180606.1 (2.5.1-dev)  Move ap_{make,set}_etag() from module http to core
+ * 20180711.1 (2.5.1-dev)  Add type ap_filter_ring_t, replace field 'filters'
+ *                         by the ap_filter_ring_t 'pending_filters' in struct
+ *                         conn_rec, and add ring entry 'pending' in struct
+ *                         ap_filter_t
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20180606
+#define MODULE_MAGIC_NUMBER_MAJOR 20180711
 #endif
 #define MODULE_MAGIC_NUMBER_MINOR 1                 /* 0...n */
 

diff --git a/include/httpd.h b/include/httpd.h
index b7341818649ead641c6f75c1238eeae12f4c6d82..2a7586f8ac45935bea58c937e0379c29f277ef00 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1224,8 +1224,8 @@ struct conn_rec {
     /** Empty bucket brigade */
     apr_bucket_brigade *empty;
 
-    /** Hashtable of filters with setaside buckets for write completion */
-    apr_hash_t *filters;
+    /** Ring of pending filters (with setaside buckets) */
+    struct ap_filter_ring *pending_filters;
 
     /** The minimum level of filter type to allow setaside buckets */
     int async_filter;

diff --git a/include/util_filter.h b/include/util_filter.h
index 4ed5fe8b591854cf5b1a6c8f996a5c5dc9148f6b..af9eba9c1872b9d748d132d4f20bcde795ffa586 100644 (file)
--- a/include/util_filter.h
+++ b/include/util_filter.h
@@ -299,8 +299,15 @@ struct ap_filter_t {
     /** Dedicated pool to use for deferred writes. */
     apr_pool_t *deferred_pool;
 
+    /** Entry in ring of pending filters (with setaside buckets). */
+    APR_RING_ENTRY(ap_filter_t) pending;
 };
 
+/**
+ * @brief The representation of a filters' ring.
+ */
+typedef APR_RING_HEAD(ap_filter_ring, ap_filter_t) ap_filter_ring_t;
+
 /**
  * Get the current bucket brigade from the next filter on the filter
  * stack.  The filter returns an apr_status_t value.  If the bottom-most

diff --git a/server/core.c b/server/core.c
index b962c34f4644a5f26700fc42946486cd67a61c22..5d2961d8d0512551633a9c28ca583e050e199c44 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -5326,7 +5326,6 @@ static conn_rec *core_create_conn(apr_pool_t *ptrans, server_rec *s,
     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->async_filter = sconf->async_filter;
 
     c->clogging_input_filters = 0;

diff --git a/server/util_filter.c b/server/util_filter.c
index 7b32a2d40e43553cb571e539782aa0023ec0b0e8..ecbb025ea0df7198b1d309bc004bdf3a61afc53b 100644 (file)
--- a/server/util_filter.c
+++ b/server/util_filter.c
@@ -293,7 +293,7 @@ static ap_filter_t *add_any_filter_handle(ap_filter_rec_t *frec, void *ctx,
                                           ap_filter_t **c_filters)
 {
     apr_pool_t *p = frec->ftype < AP_FTYPE_CONNECTION && r ? r->pool : c->pool;
-    ap_filter_t *f = apr_palloc(p, sizeof(*f));
+    ap_filter_t *f = apr_pcalloc(p, sizeof(*f));
     ap_filter_t **outf;
 
     if (frec->ftype < AP_FTYPE_PROTOCOL) {
@@ -325,9 +325,7 @@ static ap_filter_t *add_any_filter_handle(ap_filter_rec_t *frec, void *ctx,
     /* f->r must always be NULL for connection filters */
     f->r = frec->ftype < AP_FTYPE_CONNECTION ? r : NULL;
     f->c = c;
-    f->next = NULL;
-    f->bb = NULL;
-    f->deferred_pool = NULL;
+    APR_RING_ELEM_INIT(f, pending);
 
     if (INSERT_BEFORE(f, *outf)) {
         f->next = *outf;
@@ -451,12 +449,31 @@ AP_DECLARE(ap_filter_t *) ap_add_output_filter_handle(ap_filter_rec_t *f,
                                  &c->output_filters);
 }
 
+static APR_INLINE int is_pending_filter(ap_filter_t *f)
+{
+    return APR_RING_NEXT(f, pending) != f;
+}
+
+static apr_status_t pending_filter_cleanup(void *arg)
+{
+    ap_filter_t *f = arg;
+
+    if (is_pending_filter(f)) {
+        APR_RING_REMOVE(f, pending);
+        APR_RING_ELEM_INIT(f, pending);
+    }
+
+    return APR_SUCCESS;
+}
+
 static void remove_any_filter(ap_filter_t *f, ap_filter_t **r_filt, ap_filter_t **p_filt,
                               ap_filter_t **c_filt)
 {
     ap_filter_t **curr = r_filt ? r_filt : c_filt;
     ap_filter_t *fscan = *curr;
 
+    pending_filter_cleanup(f);
+
     if (p_filt && *p_filt == f)
         *p_filt = (*p_filt)->next;
 
@@ -695,39 +712,54 @@ AP_DECLARE(apr_status_t) ap_save_brigade(ap_filter_t *f,
     return srv;
 }
 
-static apr_status_t filters_cleanup(void *data)
-{
-    ap_filter_t **key = data;
-
-    apr_hash_set((*key)->c->filters, key, sizeof *key, NULL);
-
-    return APR_SUCCESS;
-}
-
 AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f, apr_pool_t **p)
 {
     apr_pool_t *pool;
-    ap_filter_t **key;
+    ap_filter_t *next, *e;
+    ap_filter_t *found = NULL;
 
+    pool = f->r ? f->r->pool : f->c->pool;
+    if (p) {
+        *p = pool;
+    }
     if (!f->bb) {
-
-        pool = f->r ? f->r->pool : f->c->pool;
-
-        key = apr_pmemdup(pool, &f, sizeof f);
-        apr_hash_set(f->c->filters, key, sizeof *key, f);
-
         f->bb = apr_brigade_create(pool, f->c->bucket_alloc);
+        apr_pool_pre_cleanup_register(pool, f, pending_filter_cleanup);
+    }
+    if (is_pending_filter(f)) {
+        return DECLINED;
+    }
 
-        apr_pool_pre_cleanup_register(pool, key, filters_cleanup);
-
-        if (p) {
-            *p = pool;
+    /* Pending reads/writes must happen in the same order as input/output
+     * filters, so find the first "next" filter already in place and insert
+     * before it, if any, otherwise insert last.
+     */
+    if (f->c->pending_filters) {
+        for (next = f->next; next && !found; next = next->next) {
+            for (e = APR_RING_FIRST(f->c->pending_filters);
+                 e != APR_RING_SENTINEL(f->c->pending_filters,
+                                        ap_filter_t, pending);
+                 e = APR_RING_NEXT(e, pending)) {
+                if (e == next) {
+                    found = e;
+                    break;
+                }
+            }
         }
-
-        return OK;
     }
-
-    return DECLINED;
+    else {
+        f->c->pending_filters = apr_palloc(f->c->pool,
+                                           sizeof(*f->c->pending_filters));
+        APR_RING_INIT(f->c->pending_filters, ap_filter_t, pending);
+    }
+    if (found) {
+        APR_RING_INSERT_BEFORE(found, f, pending);
+    }
+    else {
+        APR_RING_INSERT_TAIL(f->c->pending_filters, f, ap_filter_t, pending);
+    }
+ 
+    return OK;
 }
 
 AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
@@ -967,23 +999,24 @@ AP_DECLARE(int) ap_filter_should_yield(ap_filter_t *f)
 
 AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
 {
-    apr_hash_index_t *rindex;
     int data_in_output_filters = DECLINED;
+    ap_filter_t *f;
 
-    rindex = apr_hash_first(NULL, c->filters);
-    while (rindex) {
-        ap_filter_t *f = apr_hash_this_val(rindex);
+    if (!c->pending_filters) {
+        return DECLINED;
+    }
 
+    for (f = APR_RING_FIRST(c->pending_filters);
+         f != APR_RING_SENTINEL(c->pending_filters, ap_filter_t, pending);
+         f = APR_RING_NEXT(f, pending)) {
         if (f->frec->direction == AP_FILTER_OUTPUT && f->bb
                 && !APR_BRIGADE_EMPTY(f->bb)) {
-
             apr_status_t rv;
 
             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)
+            if (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv)) {
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(00470)
                         "write failure in '%s' output filter", f->frec->name);
                 return rv;
             }
@@ -992,8 +1025,6 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
                 data_in_output_filters = OK;
             }
         }
-
-        rindex = apr_hash_next(rindex);
     }
 
     return data_in_output_filters;
@@ -1001,12 +1032,15 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
 
 AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c)
 {
-    apr_hash_index_t *rindex;
+    ap_filter_t *f;
 
-    rindex = apr_hash_first(NULL, c->filters);
-    while (rindex) {
-        ap_filter_t *f = apr_hash_this_val(rindex);
+    if (!c->pending_filters) {
+        return DECLINED;
+    }
 
+    for (f = APR_RING_FIRST(c->pending_filters);
+         f != APR_RING_SENTINEL(c->pending_filters, ap_filter_t, pending);
+         f = APR_RING_NEXT(f, pending)) {
         if (f->frec->direction == AP_FILTER_INPUT && f->bb) {
             apr_bucket *e = APR_BRIGADE_FIRST(f->bb);
 
@@ -1019,8 +1053,6 @@ AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c)
             }
 
         }
-
-        rindex = apr_hash_next(rindex);
     }
 
     return DECLINED;