httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: [Patch] Async write completion for the full connection filter stack
Date Thu, 11 Sep 2014 15:20:40 GMT
On 11 Sep 2014, at 9:51 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:

> If we or any filter has created the brigade on *deferred_write_pool
> (or an ancestor, I'm only talking about this case), it is dead now
> (unaccessible), and IMO should be marked as so.
> In the case the caller let the function handle the brigade (as we do),
> using NULL the first time only, (s)he expects to not crash whenever
> (s)he come back with it after a clear (which (s)he isn't supposed to
> know about).

Agreed - as rpluem suggested a pool cleanup should be registered for this, I’ve added it.

> The core and mod_ssl deferred_write_pool won't be needed anymore.

The reason the deferred write pool exists is that connections might live for a very long time.
Any memory allocated during that time will accumulate and won’t be cleaned up until the
connection is closed. The deferred write pool allows us to temporarily create data during
the connection and then clean it out as soon as we no longer need it.

> You're right, I misread ap_core_output_filter() where I thought
> ap_filter_reinstate_brigade() wasn't called when
> APR_BRIGADE_EMPTY(bb).
> 
> Since ap_filter_reinstate_brigade() can be called with an empty bb,
> ap_core_output_filter() can (still) be simplified though, ie :
> 
>    should_write = ap_filter_reinstate_brigade(f, ctx->buffered_bb, bb,
>            &flush_upto);
>    if (APR_BRIGADE_EMPTY(bb)) {
>        return APR_SUCCESS;
>    }
> 
>    if (flush_upto != NULL) {
>        ...
>    }
> 
>    if (should_write) {
>        ...
>    }
> 
> flush_upto will always be NULL with an empty bb.

This is indeed true - I’ve simplified it as above.

Regards,
Graham
—


Mime
View raw message