You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Joe Orton <jo...@redhat.com> on 2004/06/19 14:53:42 UTC

Fwd: destroying passed brigades

Hiya folks, just wondering what impact (if any) these changes I've been
working on would have on mod_perl and filters implemented using
mod_perl?

The change is simply that after calling ap_pass_brigade() on a brigade,
any subsequent use of that brigade is likely to segfault since the
brigade structure really is free'd.

I notice the mod_perl pass_brigade documentation includes the necessary
"caller relinquishes ownership" caveat:

http://perl.apache.org/docs/2.0/api/Apache/Filter.html#C_pass_brigade_

but the example there processes the brigade both before and after it's
passed; is that a doc mistake, or do you really expect to be able to do
that in mod_perl?

      # ... process $bb
      
      my $rc = $f->next->pass_brigade($bb);
      return $rc unless $rc == APR::SUCCESS;
      
      # process $bb
      return Apache::OK;

Regards,

joe (please CC me on replies)

----- Forwarded message from Joe Orton <jo...@redhat.com> -----

From: Joe Orton <jo...@redhat.com>
Reply-To: dev@httpd.apache.org
To: dev@httpd.apache.org
Mail-Followup-To: dev@httpd.apache.org
Date: Thu, 17 Jun 2004 10:46:37 +0100
Subject: destroying passed brigades
User-Agent: Mutt/1.4.1i

Any further thoughts on this, Cliff or anyone else?  At least the
core_output_filter, the byterange filter, and the http_header filter
will all destroy the brigade which is passed to them.  Many other
filters presume that it's OK to reuse brigades which have been passed
on.

With a combination of filters which assume both ways, it is inevitable
that changing apr_brigade_destroy() to actually destroy the brigade
structure will break some set of existing filters, unfortunately.

There's also ambiguity about what to do with a passed brigade if the
next filter returns an error.

joe

----- End forwarded message -----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Fwd: destroying passed brigades

Posted by Stas Bekman <st...@stason.org>.
Joe Orton wrote:
> On Sat, Jun 19, 2004 at 11:17:28PM +0300, Stas Bekman wrote:
> 
>>Joe Orton wrote:
>>
>>>Hiya folks, just wondering what impact (if any) these changes I've been
>>>working on would have on mod_perl and filters implemented using
>>>mod_perl?
>>>
>>>The change is simply that after calling ap_pass_brigade() on a brigade,
>>>any subsequent use of that brigade is likely to segfault since the
>>>brigade structure really is free'd.
>>
>>I understood that only the content/buckets are lost, but the bb itself 
>>remains re-usable. We have quite a few tests that go like this (.e.g see: 
>>t/filter/TestFilter/both_str_con_add.pm):
> 
> ...
> 
> OK, thanks, useful info.  I guess you prefer the model where a filter
> may never destroy the brigade which it is passed, and it's the
> responsibility of whomever *creates* the brigade to destroy it.

I think it's more effective to be able to reuse the bb shell.

>>Why should we create bb in a tight loop when we can make things faster and 
>>re-use the shell? Sounds like a waste of CPU cycles and memory to me.
> 
> 
> With the patches I have it's not really expensive.  But currently we're
> in a mess where it's not clear who should destroy the brigade, and so
> they are often getting destroyed and then reused, or sometimes never
> destroyed at all.  And destroy doesn't really free the brigade anyway so
> the memory is leaked everywhere.
> 
> I'm going to go through and see if all the memory leaks can be fixed
> using the "creator must destroy" model and see if that will work instead
> since it will probably be less disruptive to third-party filters.

Cool. Either way, since mp2 is not released yet, we can adjust to whatever the 
right model is, as long as it's consistent with Apache 2.0. So please let us 
know what approach should we use.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Fwd: destroying passed brigades

Posted by Joe Orton <jo...@redhat.com>.
On Sat, Jun 19, 2004 at 11:17:28PM +0300, Stas Bekman wrote:
> Joe Orton wrote:
> >Hiya folks, just wondering what impact (if any) these changes I've been
> >working on would have on mod_perl and filters implemented using
> >mod_perl?
> >
> >The change is simply that after calling ap_pass_brigade() on a brigade,
> >any subsequent use of that brigade is likely to segfault since the
> >brigade structure really is free'd.
> 
> I understood that only the content/buckets are lost, but the bb itself 
> remains re-usable. We have quite a few tests that go like this (.e.g see: 
> t/filter/TestFilter/both_str_con_add.pm):
...

OK, thanks, useful info.  I guess you prefer the model where a filter
may never destroy the brigade which it is passed, and it's the
responsibility of whomever *creates* the brigade to destroy it.

> Why should we create bb in a tight loop when we can make things faster and 
> re-use the shell? Sounds like a waste of CPU cycles and memory to me.

With the patches I have it's not really expensive.  But currently we're
in a mess where it's not clear who should destroy the brigade, and so
they are often getting destroyed and then reused, or sometimes never
destroyed at all.  And destroy doesn't really free the brigade anyway so
the memory is leaked everywhere.

I'm going to go through and see if all the memory leaks can be fixed
using the "creator must destroy" model and see if that will work instead
since it will probably be less disruptive to third-party filters.

Regards,

joe

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Fwd: destroying passed brigades

Posted by Stas Bekman <st...@stason.org>.
Joe Orton wrote:
> Hiya folks, just wondering what impact (if any) these changes I've been
> working on would have on mod_perl and filters implemented using
> mod_perl?
> 
> The change is simply that after calling ap_pass_brigade() on a brigade,
> any subsequent use of that brigade is likely to segfault since the
> brigade structure really is free'd.

I understood that only the content/buckets are lost, but the bb itself remains 
re-usable. We have quite a few tests that go like this (.e.g see: 
t/filter/TestFilter/both_str_con_add.pm):

     my $bb = APR::Brigade->new($c->pool, $c->bucket_alloc);

     for (;;) {
         $c->input_filters->get_brigade($bb, Apache::MODE_GETLINE);
         last if $bb->is_empty;

         my $b = APR::Bucket::flush_create($c->bucket_alloc);
         $bb->insert_tail($b);
         $c->output_filters->pass_brigade($bb);
         # fflush is the equivalent of the previous 3 lines of code:
         # but it's tested elsewhere, here testing flush_create
         # $c->output_filters->fflush($bb);
     }

     $bb->destroy;

Why should we create bb in a tight loop when we can make things faster and 
re-use the shell? Sounds like a waste of CPU cycles and memory to me.


> I notice the mod_perl pass_brigade documentation includes the necessary
> "caller relinquishes ownership" caveat:
> 
> http://perl.apache.org/docs/2.0/api/Apache/Filter.html#C_pass_brigade_
> 
> but the example there processes the brigade both before and after it's
> passed; is that a doc mistake, or do you really expect to be able to do
> that in mod_perl?
> 
>       # ... process $bb
>       
>       my $rc = $f->next->pass_brigade($bb);
>       return $rc unless $rc == APR::SUCCESS;
>       
>       # process $bb
>       return Apache::OK;

It's a typo, Joe. I've removed that bogus comment. Thanks a lot for the heads up.

> Regards,
> 
> joe (please CC me on replies)
> 
> ----- Forwarded message from Joe Orton <jo...@redhat.com> -----
> 
> From: Joe Orton <jo...@redhat.com>
> Reply-To: dev@httpd.apache.org
> To: dev@httpd.apache.org
> Mail-Followup-To: dev@httpd.apache.org
> Date: Thu, 17 Jun 2004 10:46:37 +0100
> Subject: destroying passed brigades
> User-Agent: Mutt/1.4.1i
> 
> Any further thoughts on this, Cliff or anyone else?  At least the
> core_output_filter, the byterange filter, and the http_header filter
> will all destroy the brigade which is passed to them.  Many other
> filters presume that it's OK to reuse brigades which have been passed
> on.
> 
> With a combination of filters which assume both ways, it is inevitable
> that changing apr_brigade_destroy() to actually destroy the brigade
> structure will break some set of existing filters, unfortunately.
> 
> There's also ambiguity about what to do with a passed brigade if the
> next filter returns an error.
> 
> joe
> 
> ----- End forwarded message -----
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org


-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org