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 2008/11/16 20:22:42 UTC
svn commit: r718082 - /httpd/httpd/trunk/modules/filters/mod_buffer.c
Author: minfrin
Date: Sun Nov 16 11:22:41 2008
New Revision: 718082
URL: http://svn.apache.org/viewvc?rev=718082&view=rev
Log:
Refactor the input buffer filter to use a context as the output buffer
filter does. Ensure that the filter properly handles the non blocking
case, both when a downstream filter returns EAGAIN, or if a filter
returns APR_SUCCESS and an empty brigade.
Modified:
httpd/httpd/trunk/modules/filters/mod_buffer.c
Modified: httpd/httpd/trunk/modules/filters/mod_buffer.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_buffer.c?rev=718082&r1=718081&r2=718082&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_buffer.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_buffer.c Sun Nov 16 11:22:41 2008
@@ -43,7 +43,10 @@
typedef struct buffer_ctx {
apr_bucket_brigade *bb;
+ apr_bucket_brigade *tmp;
buffer_conf *conf;
+ apr_off_t remaining;
+ int seen_eos;
} buffer_ctx;
/**
@@ -158,11 +161,9 @@
*/
static apr_status_t buffer_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes) {
- apr_bucket *e;
- apr_bucket_brigade *tmp;
+ apr_bucket *e, *after;
apr_status_t rv;
- buffer_conf *c;
- apr_off_t remaining;
+ buffer_ctx *ctx = f->ctx;
/* buffer on main requests only */
if (!ap_is_initial_req(f->r)) {
@@ -170,65 +171,84 @@
return ap_get_brigade(f->next, bb, mode, block, readbytes);
}
+ /* first time in? create a context */
+ if (!ctx) {
+ ctx = f->ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
+ ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
+ ctx->tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
+ ctx->conf = ap_get_module_config(f->r->per_dir_config, &buffer_module);
+ }
+
/* just get out of the way of things we don't want. */
if (mode != AP_MODE_READBYTES) {
return ap_get_brigade(f->next, bb, mode, block, readbytes);
}
- c = ap_get_module_config(f->r->per_dir_config, &buffer_module);
-
- tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
-
- remaining = readbytes;
- while (remaining > 0) {
- const char *data;
- apr_off_t len;
- apr_size_t size;
-
- rv = ap_get_brigade(f->next, tmp, mode, block, remaining);
-
- /* if an error was received, bail out now */
- if (rv != APR_SUCCESS) {
- APR_BRIGADE_CONCAT(bb, tmp);
- return rv;
- }
-
- apr_brigade_length(tmp, 1, &len);
- remaining -= len;
-
- for (e = APR_BRIGADE_FIRST(tmp); e != APR_BRIGADE_SENTINEL(tmp); e
- = APR_BUCKET_NEXT(e)) {
-
- /* if we see an EOS, we are done */
- if (APR_BUCKET_IS_EOS(e)) {
- APR_BUCKET_REMOVE(e);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- remaining = 0;
- break;
+ /* if our buffer is empty, read off the network until the buffer is full */
+ if (APR_BRIGADE_EMPTY(ctx->bb)) {
+ ctx->remaining = ctx->conf->size;
+
+ while (!ctx->seen_eos && ctx->remaining > 0) {
+ const char *data;
+ apr_size_t size = 0;
+
+ rv = ap_get_brigade(f->next, ctx->tmp, mode, block, ctx->remaining);
+
+ /* if an error was received, bail out now. If the error is
+ * EAGAIN and we have not yet seen an EOS, we will definitely
+ * be called again, at which point we will send our buffered
+ * data. Instead of sending EAGAIN, some filters return an
+ * empty brigade instead when data is not yet available. In
+ * this case, pass through the APR_SUCCESS and emulate the
+ * underlying filter.
+ */
+ if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(ctx->tmp)) {
+ return rv;
}
- /* pass flush buckets through */
- if (APR_BUCKET_IS_FLUSH(e)) {
- APR_BUCKET_REMOVE(e);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- continue;
- }
+ for (e = APR_BRIGADE_FIRST(ctx->tmp); e != APR_BRIGADE_SENTINEL(
+ ctx->tmp); e = APR_BUCKET_NEXT(e)) {
- /* pass metadata buckets through */
- if (APR_BUCKET_IS_METADATA(e)) {
- APR_BUCKET_REMOVE(e);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- continue;
- }
+ /* if we see an EOS, we are done */
+ if (APR_BUCKET_IS_EOS(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ ctx->seen_eos = 1;
+ break;
+ }
+
+ /* pass flush and metadata buckets through */
+ if (APR_BUCKET_IS_FLUSH(e) || APR_BUCKET_IS_METADATA(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ continue;
+ }
+
+ /* read the bucket in, pack it into the buffer */
+ if (APR_SUCCESS == (rv = apr_bucket_read(e, &data, &size,
+ APR_BLOCK_READ))) {
+ apr_brigade_write(ctx->bb, NULL, NULL, data, size);
+ ctx->remaining -= size;
+ apr_bucket_delete(e);
+ } else {
+ return rv;
+ }
- /* read */
- if (APR_SUCCESS == (rv = apr_bucket_read(e, &data, &size,
- APR_BLOCK_READ))) {
- apr_brigade_write(bb, NULL, NULL, data, size);
}
+ }
+ }
+ /* give the caller the data they asked for from the buffer */
+ apr_brigade_partition(ctx->bb, readbytes, &after);
+ e = APR_BRIGADE_FIRST(ctx->bb);
+ while (e != after) {
+ if (APR_BUCKET_IS_EOS(e)) {
+ /* last bucket read, step out of the way */
+ ap_remove_input_filter(f);
}
- apr_brigade_cleanup(tmp);
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ e = APR_BRIGADE_FIRST(ctx->bb);
}
return APR_SUCCESS;
Re: svn commit: r718082 - /httpd/httpd/trunk/modules/filters/mod_buffer.c
Posted by Graham Leggett <mi...@sharp.fm>.
Ruediger Pluem wrote:
>> + /* pass flush and metadata buckets through */
>> + if (APR_BUCKET_IS_FLUSH(e) || APR_BUCKET_IS_METADATA(e)) {
>
> This is redundant as flush buckets are metadata buckets :-).
>
>> + APR_BUCKET_REMOVE(e);
>> + APR_BRIGADE_INSERT_TAIL(bb, e);
>> + continue;
>> + }
Hmmm... true. A metadata bucket should be passed over into the buffer,
but flush buckets should cause the buffer to be read straight away. Will
fix.
Regards,
Graham
--
Re: svn commit: r718082 - /httpd/httpd/trunk/modules/filters/mod_buffer.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 11/16/2008 08:22 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Nov 16 11:22:41 2008
> New Revision: 718082
>
> URL: http://svn.apache.org/viewvc?rev=718082&view=rev
> Log:
> Refactor the input buffer filter to use a context as the output buffer
> filter does. Ensure that the filter properly handles the non blocking
> case, both when a downstream filter returns EAGAIN, or if a filter
> returns APR_SUCCESS and an empty brigade.
>
> Modified:
> httpd/httpd/trunk/modules/filters/mod_buffer.c
>
> Modified: httpd/httpd/trunk/modules/filters/mod_buffer.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_buffer.c?rev=718082&r1=718081&r2=718082&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_buffer.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_buffer.c Sun Nov 16 11:22:41 2008
> @@ -170,65 +171,84 @@
> return ap_get_brigade(f->next, bb, mode, block, readbytes);
> }
>
> + /* first time in? create a context */
> + if (!ctx) {
> + ctx = f->ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
> + ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> + ctx->tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> + ctx->conf = ap_get_module_config(f->r->per_dir_config, &buffer_module);
> + }
> +
> /* just get out of the way of things we don't want. */
> if (mode != AP_MODE_READBYTES) {
> return ap_get_brigade(f->next, bb, mode, block, readbytes);
> }
>
> - c = ap_get_module_config(f->r->per_dir_config, &buffer_module);
> -
> - tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> -
> - remaining = readbytes;
> - while (remaining > 0) {
> - const char *data;
> - apr_off_t len;
> - apr_size_t size;
> -
> - rv = ap_get_brigade(f->next, tmp, mode, block, remaining);
> -
> - /* if an error was received, bail out now */
> - if (rv != APR_SUCCESS) {
> - APR_BRIGADE_CONCAT(bb, tmp);
> - return rv;
> - }
> -
> - apr_brigade_length(tmp, 1, &len);
> - remaining -= len;
> -
> - for (e = APR_BRIGADE_FIRST(tmp); e != APR_BRIGADE_SENTINEL(tmp); e
> - = APR_BUCKET_NEXT(e)) {
> -
> - /* if we see an EOS, we are done */
> - if (APR_BUCKET_IS_EOS(e)) {
> - APR_BUCKET_REMOVE(e);
> - APR_BRIGADE_INSERT_TAIL(bb, e);
> - remaining = 0;
> - break;
> + /* if our buffer is empty, read off the network until the buffer is full */
> + if (APR_BRIGADE_EMPTY(ctx->bb)) {
> + ctx->remaining = ctx->conf->size;
> +
> + while (!ctx->seen_eos && ctx->remaining > 0) {
> + const char *data;
> + apr_size_t size = 0;
> +
> + rv = ap_get_brigade(f->next, ctx->tmp, mode, block, ctx->remaining);
> +
> + /* if an error was received, bail out now. If the error is
> + * EAGAIN and we have not yet seen an EOS, we will definitely
> + * be called again, at which point we will send our buffered
> + * data. Instead of sending EAGAIN, some filters return an
> + * empty brigade instead when data is not yet available. In
> + * this case, pass through the APR_SUCCESS and emulate the
> + * underlying filter.
> + */
> + if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(ctx->tmp)) {
> + return rv;
> }
>
> - /* pass flush buckets through */
> - if (APR_BUCKET_IS_FLUSH(e)) {
> - APR_BUCKET_REMOVE(e);
> - APR_BRIGADE_INSERT_TAIL(bb, e);
> - continue;
> - }
> + for (e = APR_BRIGADE_FIRST(ctx->tmp); e != APR_BRIGADE_SENTINEL(
> + ctx->tmp); e = APR_BUCKET_NEXT(e)) {
>
> - /* pass metadata buckets through */
> - if (APR_BUCKET_IS_METADATA(e)) {
> - APR_BUCKET_REMOVE(e);
> - APR_BRIGADE_INSERT_TAIL(bb, e);
> - continue;
> - }
> + /* if we see an EOS, we are done */
> + if (APR_BUCKET_IS_EOS(e)) {
> + APR_BUCKET_REMOVE(e);
> + APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
> + ctx->seen_eos = 1;
> + break;
> + }
> +
> + /* pass flush and metadata buckets through */
> + if (APR_BUCKET_IS_FLUSH(e) || APR_BUCKET_IS_METADATA(e)) {
This is redundant as flush buckets are metadata buckets :-).
> + APR_BUCKET_REMOVE(e);
> + APR_BRIGADE_INSERT_TAIL(bb, e);
> + continue;
> + }
> +
> + /* read the bucket in, pack it into the buffer */
> + if (APR_SUCCESS == (rv = apr_bucket_read(e, &data, &size,
> + APR_BLOCK_READ))) {
> + apr_brigade_write(ctx->bb, NULL, NULL, data, size);
> + ctx->remaining -= size;
> + apr_bucket_delete(e);
> + } else {
> + return rv;
> + }
>
> - /* read */
> - if (APR_SUCCESS == (rv = apr_bucket_read(e, &data, &size,
> - APR_BLOCK_READ))) {
> - apr_brigade_write(bb, NULL, NULL, data, size);
> }
> + }
> + }
>
> + /* give the caller the data they asked for from the buffer */
> + apr_brigade_partition(ctx->bb, readbytes, &after);
> + e = APR_BRIGADE_FIRST(ctx->bb);
> + while (e != after) {
> + if (APR_BUCKET_IS_EOS(e)) {
> + /* last bucket read, step out of the way */
> + ap_remove_input_filter(f);
> }
> - apr_brigade_cleanup(tmp);
> + APR_BUCKET_REMOVE(e);
> + APR_BRIGADE_INSERT_TAIL(bb, e);
> + e = APR_BRIGADE_FIRST(ctx->bb);
> }
This should be possible without a loop over the brigade. See
brigade_move in core_filters for a similar function.
Regards
Rüdiger