On 13 Sep 2014, at 9:23 PM, Ruediger Pluem <rpl...@apache.org> wrote:
> How about to require that the caller of ap_filter_setaside_brigade just hands > over a non NULL bucket_brigade as > buffered_bb (so changing apr_bucket_brigade ** to apr_bucket_brigade *) and > that it should handle *deferred_write_pool > as opaque structure and that it should not allocate from it? Or we request > the caller to provide a non NULL > deferred_write_pool as well (so changing apr_pool_t ** to apr_pool_t *) and > warn the caller that the pool might be > cleared during the call of ap_filter_setaside_brigade. Hence solving all > lifetime issues would be in the responsibility > of the caller. There is an issue I hadn’t considered - the effect of calling ap_remove_output_filter(). If we remove the filter with buckets having been set aside, the c->data_in_output_filters will remain unchanged and off-by-one, leading to a spin (it will never count back down to zero). This is a nasty bug to have to chase down. In the attached patch I have investigated adding both buffered_bb and the deferred_write_pool to ap_filter_t, and having ap_remove_output_filter() “do the right thing” when called. Both of these can be optionally set by the caller, but are otherwise not handled by the caller, the filter API worries about cleaning up, and worries about ensuring that c->data_in_output_filters is always accurate. I have also introduced an optimisation where if this is a request filter, we use r->pool to setaside buckets, while if we’re a connection filter we use the deferred write pool. Looks like a similar thing needs to be done with input filters. Regards, Graham —
httpd-async-fullstack-ssl4.patch
Description: Binary data
