On 26 August 2016 at 14:27, Evgeny Kotkov <evgeny.kot...@visualsvn.com> wrote: > Ivan Zhakov <i...@visualsvn.com> writes: > >>> @@ -3266,15 +3264,13 @@ static svn_error_t * __attribute__((war >>> close_filter(void *baton) >>> { >>> diff_ctx_t *dc = baton; >>> - apr_bucket_brigade *bb; >>> apr_bucket *bkt; >>> apr_status_t status; >>> >>> /* done with the file. write an EOS bucket now. */ >>> - bb = apr_brigade_create(dc->pool, dc->output->c->bucket_alloc); >>> bkt = apr_bucket_eos_create(dc->output->c->bucket_alloc); >>> - APR_BRIGADE_INSERT_TAIL(bb, bkt); >>> - if ((status = ap_pass_brigade(dc->output, bb)) != APR_SUCCESS) >>> + APR_BRIGADE_INSERT_TAIL(dc->bb, bkt); >>> + if ((status = ap_pass_brigade(dc->output, dc->bb)) != APR_SUCCESS) >>> return svn_error_create(status, NULL, "Could not write EOS to filter"); >>> >> As far I understand apr_brigade_cleanup() should be called after >> ap_pass_brigade(). So code should be something like: >> [[[ >> APR_BRIGADE_INSERT_TAIL(dc->bb, bkt); >> status = ap_pass_brigade(dc->output, dc->bb); >> apr_brigade_cleanup(dc->bb); >> if (status != APR_SUCCESS) >> return svn_error_create(status, NULL, "Could not write EOS to filter"); >> ]]] > > Thanks, I will add the missing call in trunk. > > While it's not strictly required here (this is the stream's close_fn(), and > we always call apr_brigade_destroy() upon returning from the deliver() > hook), this may not hold in the future. Or someone might improperly use > this code as a reference. > Ok, thanks! I added my +1 for backport anyway, since there is no actual problem/memory leak in this particular case.
-- Ivan Zhakov