On 11 Sep 2014, at 2:12 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
> Again I missed something...
>
> This code is indeed what I'd like ap_core_output_filter() (or any
> filter) to be able to do, but it won't work without a slight change in
> ap_filter_reinstate_brigade(), which is :
>
> AP_DECLARE(int) ap_filter_reinstate_brigade(ap_filter_t *f,
> apr_bucket_brigade *buffered_bb,
> apr_bucket_brigade *bb,
> apr_bucket **flush_upto)
> {
> apr_bucket *bucket, *next;
> apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
> int eor_buckets_in_brigade, morphing_bucket_in_brigade;
> int loglevel = ap_get_conn_module_loglevel(f->c, APLOG_MODULE_INDEX);
>
> *flush_upto = NULL;
>
> if ((buffered_bb != NULL) && !APR_BRIGADE_EMPTY(buffered_bb)) {
> int flush = APR_BRIGADE_EMPTY(bb);
> APR_BRIGADE_PREPEND(bb, buffered_bb);
> f->c->data_in_output_filters--;
> if (flush) {
> return 1;
> }
> }
>
> ...
> }
>
> The new thing is the "flush" check to return 1 immediatly when an
> empty bb is given.
> In this case the intent of the caller is to use what we have ("push it
> thru" would say Jim), we don't need to walk the buffered_bb again.
>
> That's, I think, what the original code did in ap_core_output_filter()
> to send_brigade_nonblocking() immediatly in this case, and what the
> current patch misses.
>
> Thoughts?
We need this yes - if we’ve been called with an empty brigade and we still have
a buffered brigade that is “too small to write” we want to just write that
remaining brigade regardless of it’s size, otherwise we spin.
I would still do the flush check anyway - the ap_filter_reinstate_brigade() has
no idea what happened outside this function, we may have flushed buckets, we
may not have and setaside those flush buckets for whatever reason, and we want
the behaviour to remain predictable.
I’ve updated the patch to force the should_write if the original brigade was
empty.
Regards,
Graham
—
httpd-async-fullstack-ssl3.patch
Description: Binary data
