On 10/04/2009 10:54 AM, Stefan Fritsch wrote: > On Sunday 04 October 2009, Paul Querna wrote: >>> URL: http://svn.apache.org/viewvc?rev=821477&view=rev >>> Log: >>> Make sure to not destroy bucket brigades that have been created >>> by earlier filters. Otherwise the pool cleanups would be removed >>> causing potential memory leaks later on. >> I am not sure these changes make sense. The 'traditional' API view >> says that brigades passed down the output fitler chain should be >> destroyed, not cleared -- please see the thread started with this >> message: >> <http://mail-archives.apache.org/mod_mbox/httpd-dev/200504.mbox/%3C >> [email protected]%3e> >> > > This is at odds with the documentation at > http://httpd.apache.org/docs/trunk/developer/output-filters.html , in > particular with rules 6 and 9 at the end. If output filters reuse > brigades, the core must not destroy them. > > I just noticed that include/util_filter.h still says "The caller > relinquishes ownership of the brigade" for ap_pass_brigade(). Either > output-filters.html or util_filter.h must be changed. > > In my opinion, it is much better to not destroy the brigades and reuse > them instead of creating a new brigade on every filter invocation. > Otherwise there is huge memory usage for streaming content. > > The thread you have pointed out also has a suggested patch for the > docs in util_filter.h: > > http://mail-archives.apache.org/mod_mbox/httpd- > dev/200504.mbox/<[email protected]> > > It is not clear from the thread why it wasn't applied.
>From rereading the old discussion back in 2005 I guess it is the correct thing to fix the comment in util_filter.h and not to destroy brigades that weren't created by us but just to clean them up. So the ownership of the brigade remains with the creator / caller, whereas the ownership of the buckets is transferred (either consume them directly or put them safely aside for later consumption). Regarding adding an apr_brigade_cleanup call to ap_pass_brigade I am undecided. Regards Rüdiger
