On 09/11/2014 05:20 PM, Graham Leggett wrote: > On 11 Sep 2014, at 9:51 AM, Yann Ylavic <ylavic....@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.
I know that I proposed this, but rethinking I see one risk with the cleanup. Take the following code with long_living_apr_bucket_brigade_ptr being NULL in the beginning: apr_bucket_brigade *stack_variable; stack_variable = long_living_apr_bucket_brigade_ptr; ap_save_brigade(f, &stack_variable, &bb, *deferred_write_pool); long_living_apr_bucket_brigade_ptr = stack_variable; I guess in this case this could cause unpredictable behaviour as the stack of the calling function is likely to be repurposed when the pool dies. Do we see that as something that we need to consider or do we see this as a "bug" on caller side? If we see it as a "bug" on caller side we should probably document our expectations. Regards Rüdiger
