Stas Bekman <[EMAIL PROTECTED]> writes: [...]
> > - push @{ $ctx->{buckets} }, APR::Bucket->new($c->bucket_alloc, > > $header); > > + my $bucket = APR::Bucket->new($c->bucket_alloc, $header); > > + $bucket->setaside($c->pool); > > + push @{ $ctx->{buckets} }, $bucket; > > debug "queued header [$header]"; > > } > > elsif ($data =~ /^[\r\n]+$/) { > > @@ -197,7 +199,9 @@ > > # time to add extra headers: > > for my $key (keys %headers) { > > my $header = "$key: $headers{$key}\n"; > > - push @{ $ctx->{buckets} }, APR::Bucket->new($c->bucket_alloc, > > $header); > > + my $bucket = APR::Bucket->new($c->bucket_alloc, $header); > > + $bucket->setaside($c->pool); > > + push @{ $ctx->{buckets} }, $bucket; > > debug "queued header [$header]"; > > } > > @@ -205,8 +209,8 @@ > > # the separator header will be sent as a last header > > # so we send one newly added header and push the separator > > # to the end of the queue > > - # XXX: this is broken: the bucket must be set-aside before > > - # it can be stashed away (missing $b->setaside wrapper) > > + > > + $b->setaside($c->pool); > > push @{ $ctx->{buckets} }, $b; > > debug "queued header [$data]"; > > inject_header_bucket($bb, $ctx); > > I think so. It was obviously wrong before, isn't it? Yes, the $b->setaside($c->pool) call is certainly necessary. The earlier setaside calls I added aren't really required, because those modperl buckets already belong to you, so you know what their lifetime is. > > > One criticism I have about this test- it is technically > > incorrect to store the setaside buckets in a Perl array > > instead of keeping them in a bucket brigade. That's because any buckets > > left-over in the array will not get destroyed, > > which can cause a memory leak. It is always good practice to always keep > > buckets in brigades, because the brigade's cleanup should take care of this > > (which is the reason why calling destroy() on a brigade is inferior to > > calling cleanup()). > > An excellent point, Joe. So should we just change the AV for bb and > store that in the context? Perhaps, or just make the array-ref an object with an appropriate DESTROY sub, that will traverse its elements with $_->destroy for @bucket_array; It might be a bit pedantic to worry this much about memory leaks in the test suite though. -- Joe Schaefer --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]