httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <>
Subject Re: is it possible to mark buckets to be copied only when to be set-aside?
Date Wed, 19 May 2004 14:20:57 GMT
On Wed, 19 May 2004, Cliff Woolley wrote:

> Note: ap_save_brigade() handles the setaside and brigade concatenation for
> you.  I suspect there are a number of places in the code that we are
> incorrectly calling APR_BRIGADE_CONCAT() instead of ap_save_brigade().
> That's bug city right there.

Worse, there are probably places in the code where we don't even use
APR_BRIGADE_CONCAT() and simply append each bucket one at a time from the
source brigade to the "saved" brigade using APR_BRIGADE_INSERT_TAIL() (or
even APR_BUCKET_INSERT_AFTER(), though it's hard for me to imagine a case
where doing it that way would have made sense to the programmer).

Furthermore, I just found a bug in ap_save_brigade().  On line 534 of

        rv = apr_bucket_setaside(e, p);
        if (rv != APR_SUCCESS
            /* ### this ENOTIMPL will go away once we implement setaside
               ### for all bucket types. */
            && rv != APR_ENOTIMPL) {
            return rv;

Note that ### comment?  Yeah it denotes what at this point is a bug.  You
can't just ignore APR_ENOTIMPL for bucket types, because pipe and socket
buckets never had setaside implemented on them.  I thought I remembered
that that was because Greg and Ryan and I had some huge debate about it
and it was decided for some reason that setting aside a pipe or socket
didn't make sense.  But now I can't find where we talked about that in the
archives.  Anyway, when Greg originally wrote the lines in question (see, he
obviously did so assuming that setaside would be implemented on all bucket
types.  It was not.

I recommend that the interested reader go back and take a look at the
following thread (this kind of starts somewhere in the middle but this one
message and its followups are particularly relevant):

As it is, given the current State Of The Buckets: If rv == APR_ENOTIMPL
when you call apr_bucket_setaside(), then you're supposed to call
apr_bucket_read(), get the data out of the bucket, and stick it into a
heap bucket.  ap_save_brigade() could possibly do a little optimizing here
in the special cases of socket and pipe buckets, and grab the actual APR
socket or pipe out of the bucket's internal data and compare the
socket/pipe's pool to pool p.  If they already match, you don't have to do
the read and make-into-heap-bucket process; the reason being is that the
point of setaside is to tell the buckets code that you want it to
guarantee that the data storage behind bucket e will live at least as long
as pool p.

I thought I could just refer for some of this back to either of my sets of
ApacheCon slides, but I can see I discussed setaside() insufficiently well
at both ApacheCons.  :(


View raw message