httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: cvs commit: apache-2.0/src/main http_protocol.c
Date Tue, 17 Oct 2000 20:34:24 GMT

> Here is the problem I encountered:
> 
>   we enter http_filter() in body mode with the following data saved in ctx->b
> 
>    +-------------------+---------------------+
>    | body of request 1 | header of request 2 |
>    +-------------------+---------------------+
> 
>   the input brigade is allocated from the request pool
> 
>   we concatenate this data into the request pool brigade, but then
>   figure out where the body ends and do ap_brigade_split() to put the
>   request 2 stuff in a different brigade
> 
>   ap_brigade_split() allocates the new brigade from the same pool as
>   the first brigade...

Yeah, that's the behavior I noticed when I went to the code.  Damn that's
an annoying bug.  Glad you found it, or it would have bitten us one
day.  :-)

> > Does that make any sense at all?
> 
> yes, I think your solution will be fine...  we'll let http_filter()
> use the same brigade always...  it will move buckets into the caller's
> brigade as necessary
> 
> I'll commit this shortly...

Cool.  That lets us keep brigades allocated out of the correct
pool.

This problem may actually be biting us in more places that we have caught
so far.  Imagine the case of the core filter.  This filter also uses
AP_BRIGADE_CONCAT, and it should be concatenating one brigade allocated
out of the request_rec with one allocated out of the conn_rec.  On
keepalive requests we might cleanup the brigade before we sent the
data.  Now, this problem may not bite us yet, because if we always
allocated the brigade out of the conn_rec, and then CONCAT into that
brigade, we might be alright.

This seems like a hairy problem to track down though, and a hairy one to
fix.  Would it be better for us to create a function
ap_brigade_concat?  I'm imagining something like:

void ap_brigade_concat(ap_brigade *b1, ap_brigade *b2, apr_pool_t *p)
{
    AP_ASSERT_DEBUG(p == b1->p || p == b2->p);

    if (b1->p == p) {
        AP_BRIGADE_CONCAT(b1, b2);
    }
    else {
        AP_BRIGADE_CONCAT(b1, b2);
        b1->p == p;
    }
}

The pool assignment is probably pretty bogus though, and we would
definately need to kill and re-register a cleanup.

Oh well.

Ryan 
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message