httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rici Lake <r...@ricilake.net>
Subject RFC: Who owns a passed brigade?
Date Thu, 21 Apr 2005 17:34:27 GMT
After a filter calls ap_pass_brigade(f->next, bb), who owns bb?

The name of the function might lead one to believe that ownership of 
the brigade was transferred, but inspection of distributed filters 
shows that it has not been; both mod_deflate and mod_include expect the 
brigade to be usable (and apparently empty) after return from 
ap_pass_brigade.

This would imply that the rule is that ownership of the brigade is 
retained by the caller, but that ownership of the buckets in the 
brigade is transferred to the callee.

If this is the case, I believe that it should be clearly stated in the 
API documentation. Otherwise, it is inevitable that people writing 
filters will get it wrong. (And it would also suggest that the function 
should be renamed ap_pass_buckets())

It also leads to the question of what the expected behaviour is if the 
callee does not empty the passed brigade. Is it reasonable for a called 
filter to expect to be able to leave a bucket in the passed brigade so 
as to receive it on the next call? That would seem eccentric, although 
I believe it is what would happen in the case of mod_deflate and 
mod_include, at least.

If the brigade is required to have been emptied when the receiving 
filter returns, then would  it not make more sense for ap_pass_brigade 
(or ap_pass_buckets) to call apr_brigade_cleanup just before returning? 
This would avoid the situation where a called filter religiously cleans 
up the brigade, in order to conform to the API expectations, but the 
calling filter also cleans up the brigade, in order to avoid worrying 
about misbehaving downstream filters.

If, on the other hand, buckets can be left in the brigade for retention 
between filter invocations, it must be clear that the passing filter 
must always use the same brigade for every call to ap_pass_brigade(), 
or otherwise arrange for buckets left in the brigade to be repassed.

I note that apr_brigade_split() seems to be oriented towards a model 
where ownership of the brigade is passed, since it creates a new 
brigade. For the model where brigade ownership is retained, it would be 
better to have a function which shifted buckets from one brigade to 
another one.

There are a number of possible bugs resulting from a lack of clarity on 
brigade ownership. For example:

-- if a filter believes that brigade ownership has been passed to it, 
it may call apr_brigade_destroy on the brigade. This will not cause any 
obvious misbehaviour, but the pool cleanup on the brigade will have 
been removed, and if the request terminates on an error, it is 
conceivable that heap buckets later added to the brigade will leak.

-- if a filter believes that it is passing ownership, it will not call 
apr_brigade_destroy on the brigade. This is only serious in the case 
where the filter creates a new brigade on every invocation (as, for 
example, mod_include does). In that case, the pool's cleanup list will 
grow dramatically for large requests. (There are a couple of issues in 
bugzilla which seem to be showing this symptom.)

-- as mentioned above, if a filter does not understand that it is being 
passed ownership of the buckets, it may leave them in the brigade and 
then receive them again on a subsequent invocation. However, the 
upstream filter may choose to send a different brigade on the 
subsequent invocation, so the filter cannot rely on this behaviour.

My vote would be for a clear statement that ownership of brigades is 
retained and ownership of buckets is transferred, along with a 
documented call to ap_brigade_cleanup() in ap_pass_brigade to precisely 
define the expected state of the brigade on return.


Mime
View raw message