Skip to content

Commit

Permalink
Follow up to r1840265: really privatize ap_filter_{recycle,adopt_brig…
Browse files Browse the repository at this point in the history
…ade}().

Move ap_filter_adopt_brigade()'s declaration to "server/core.h" (private).

For ap_filter_recycle(), make it static/internal to util_filter (renamed to
recycle_dead_filters() which better fits what it does). It's now also called
unconditionally from ap_filter_input_pending() which itself is always called
after the request processing and from MPM event (as input_pending hook).


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1840611 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
ylavic committed Sep 11, 2018
1 parent a7379ee commit 69a49ab
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 32 deletions.
22 changes: 0 additions & 22 deletions include/util_filter.h
Expand Up @@ -596,16 +596,6 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f);
AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
apr_bucket_brigade *bb);

/*
* Adopt a bucket brigade as is (no setaside nor copy).
* @param f The current filter
* @param bb The bucket brigade adopted. This brigade is always empty
* on return
* @remark httpd internal, not exported, needed by
* ap_core_input_filter
*/
void ap_filter_adopt_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
Expand Down Expand Up @@ -666,18 +656,6 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c);
*/
AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c);

/*
* Recycle removed request filters so that they can be reused for filters
* added later on the same connection. This typically should happen after
* each request handling.
*
* @param c The connection.
* @remark httpd internal, not exported, needed by
* ap_process_request_after_handler
*
*/
void ap_filter_recycle(conn_rec *c);

/**
* Flush function for apr_brigade_* calls. This calls ap_pass_brigade
* to flush the brigade if the brigade buffer overflows.
Expand Down
1 change: 0 additions & 1 deletion modules/http/http_request.c
Expand Up @@ -402,7 +402,6 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r)
(void)ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES);

ap_release_brigade(c, bb);
ap_filter_recycle(c);

if (c->cs) {
if (c->aborted) {
Expand Down
9 changes: 9 additions & 0 deletions server/core.h
Expand Up @@ -33,6 +33,15 @@ typedef struct conn_config_t {
apr_socket_t *socket;
} conn_config_t;

/**
* Adopt a bucket brigade as is (no setaside nor copy).
* @param f The current filter
* @param bb The bucket brigade adopted. This brigade is always empty
* on return
* @remark All buckets in bb should be allocated on f->c->pool and
* f->c->bucket_alloc.
*/
void ap_filter_adopt_brigade(ap_filter_t *f, apr_bucket_brigade *bb);

#endif /* CORE_H */
/** @} */
Expand Down
1 change: 1 addition & 0 deletions server/core_filters.c
Expand Up @@ -49,6 +49,7 @@
#include "scoreboard.h"
#include "mod_core.h"
#include "ap_listen.h"
#include "core.h"

#include "mod_so.h" /* for ap_find_loaded_module_symbol */

Expand Down
28 changes: 19 additions & 9 deletions server/util_filter.c
Expand Up @@ -26,6 +26,7 @@
#include "http_log.h"
#include "http_request.h"
#include "util_filter.h"
#include "core.h"

/* NOTE: Apache's current design doesn't allow a pool to be passed thru,
so we depend on a global to hold the correct pool
Expand Down Expand Up @@ -395,16 +396,17 @@ static apr_status_t request_filter_cleanup(void *arg)
* while it is handling/passing the EOR, and we want each filter or
* ap_filter_output_pending() to be able to dereference f until they
* return. So request filters are recycled in dead_filters and will only
* be moved to spare_filters when ap_filter_recycle() is called explicitly.
* Set f->r to NULL still for any use after free to crash quite reliably.
* be moved to spare_filters when recycle_dead_filters() is called, i.e.
* in ap_filter_{in,out}put_pending(). Set f->r to NULL still for any use
* after free to crash quite reliably.
*/
f->r = NULL;
put_spare(c, f, &x->dead_filters);

return APR_SUCCESS;
}

void ap_filter_recycle(conn_rec *c)
static void recycle_dead_filters(conn_rec *c)
{
struct ap_filter_conn_ctx *x = c->filter_conn_ctx;

Expand Down Expand Up @@ -1236,10 +1238,10 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
ap_release_brigade(c, bb);

cleanup:
/* No more flushing, all filters have returned, recycle/unleak dead request
* filters before leaving (i.e. make them reusable).
/* All filters have returned, time to recycle/unleak ap_filter_t-s
* before leaving (i.e. make them reusable).
*/
ap_filter_recycle(c);
recycle_dead_filters(c);

return rc;
}
Expand All @@ -1248,9 +1250,10 @@ AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c)
{
struct ap_filter_conn_ctx *x = c->filter_conn_ctx;
struct ap_filter_private *fp;
int rc = DECLINED;

if (!x || !x->pending_input_filters) {
return DECLINED;
goto cleanup;
}

for (fp = APR_RING_LAST(x->pending_input_filters);
Expand All @@ -1266,11 +1269,18 @@ AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c)
e = APR_BRIGADE_FIRST(fp->bb);
if (e != APR_BRIGADE_SENTINEL(fp->bb)
&& e->length != (apr_size_t)(-1)) {
return OK;
rc = OK;
break;
}
}

return DECLINED;
cleanup:
/* All filters have returned, time to recycle/unleak ap_filter_t-s
* before leaving (i.e. make them reusable).
*/
recycle_dead_filters(c);

return rc;
}

AP_DECLARE_NONSTD(apr_status_t) ap_filter_flush(apr_bucket_brigade *bb,
Expand Down

0 comments on commit 69a49ab

Please sign in to comment.