You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2018/07/11 13:03:35 UTC
svn commit: r1835640 - in /httpd/httpd/trunk: include/ap_mmn.h
include/httpd.h include/util_filter.h server/core.c server/util_filter.c
Author: ylavic
Date: Wed Jul 11 13:03:35 2018
New Revision: 1835640
URL: http://svn.apache.org/viewvc?rev=1835640&view=rev
Log:
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).
Modified:
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/include/httpd.h
httpd/httpd/trunk/include/util_filter.h
httpd/httpd/trunk/server/core.c
httpd/httpd/trunk/server/util_filter.c
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1835640&r1=1835639&r2=1835640&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Wed Jul 11 13:03:35 2018
@@ -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 */
Modified: httpd/httpd/trunk/include/httpd.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1835640&r1=1835639&r2=1835640&view=diff
==============================================================================
--- httpd/httpd/trunk/include/httpd.h (original)
+++ httpd/httpd/trunk/include/httpd.h Wed Jul 11 13:03:35 2018
@@ -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;
Modified: httpd/httpd/trunk/include/util_filter.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_filter.h?rev=1835640&r1=1835639&r2=1835640&view=diff
==============================================================================
--- httpd/httpd/trunk/include/util_filter.h (original)
+++ httpd/httpd/trunk/include/util_filter.h Wed Jul 11 13:03:35 2018
@@ -299,9 +299,16 @@ 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
* filter doesn't read from the network, then ::AP_NOBODY_READ is returned.
Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1835640&r1=1835639&r2=1835640&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Wed Jul 11 13:03:35 2018
@@ -5326,7 +5326,6 @@ 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->async_filter = sconf->async_filter;
c->clogging_input_filters = 0;
Modified: httpd/httpd/trunk/server/util_filter.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?rev=1835640&r1=1835639&r2=1835640&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_filter.c (original)
+++ httpd/httpd/trunk/server/util_filter.c Wed Jul 11 13:03:35 2018
@@ -293,7 +293,7 @@ static ap_filter_t *add_any_filter_handl
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_handl
/* 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_
&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
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(a
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_
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_
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_p
}
}
-
- rindex = apr_hash_next(rindex);
}
return DECLINED;