Commit 643a39dc authored by Yann Ylavic's avatar Yann Ylavic
Browse files

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
parent 48f724ef

include/ap_mmn.h

+5 −1
Changes for include/ap_mmn.h: 5 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -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 */

include/httpd.h

+2 −2
Changes for include/httpd.h: 2 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -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;

include/util_filter.h

+7 −0
Changes for include/util_filter.h: 7 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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

server/core.c

+0 −1
Changes for server/core.c: 0 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -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;

server/util_filter.c

+74 −42
Changes for server/util_filter.c: 74 added lines, 42 removed lines.
Original line number Diff line number Diff line
@@ -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;



    if (!f->bb) {

    ap_filter_t *next, *e;

    ap_filter_t *found = NULL;



    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, key, filters_cleanup);



    if (p) {

        *p = pool;

    }

    if (!f->bb) {

        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;

    }



        return OK;

    /* 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;

                }

            }

        }

    }

    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 DECLINED;

    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;