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.


--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to