httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject Re: Fw: [PATCH] mod_proxy infinite cpu eating loop
Date Wed, 02 Jan 2002 20:21:04 GMT
> On Wednesday 02 January 2002 10:34 am, Bill Stoddard wrote:
> > I see this pattern several places in the code:
> >
> > rc = apr_get_brigade();
> > if (rc != APR_SUCCESS) {
> >    fail;
> > }
> > /* Is the brigade empty? */
> > if (APR_BRIGADE_EMPTY(b)) {
> >    fail;
> > }
> > /* Get first bucket */
> > e = APR_BRIGADE_FIRST(b);
> >
> > /* Is the bucket an EOS? */
> > if (APR_BUCKET_IS_EOS(e)) {
> > }
> >
> > /* Does the bucket have zero length? */
> > if (e->length == 0) {
> > }
> >
> > You have to make these checks each and everytime we fetch a brigade which seems
> > unintuitive. Should a apr_bucket_read() call on a bucket with e->length == 0
cause a
seg
> > fault? Seems this check is masking a deeper problem.
>
> A bucket_read on a bucket with e->length == 0 should not seg fault, but that isn't
> what is happening here.  What is actually happening is that we are calling bucket
> read on the sentinel which is seg faulting.  The problem is that the sentinel is kind
> of a fake bucket, so it doesn't actually have a bucket->type field, but we are
> trying to use the bucket->type to find the read function,  which causes us
> to segfault.
>
> There are a few possible solutions:
>
> 1)  Fix the apr_bucket_read macro to check if this bucket is the sentinel,
> and fail appropriately.  If we do this, we will add one if to every bucket_read
> operation.
>
> 2)  Make the sentinel a more complete bucket along with a bucket->type field.
> This bucket->type will include a no-op for the read function.
>
> I personally prefer option 2, but it may be a bit harder to implement.  I still haven't
> wrapped my head around the ring macros yet.
>
> Ryan

Yea, 2) looks good in concept, but I'm unsure how APR_BUCKET_REMOVE would be implemented
to special case removing a sentinel bucket.  The latest fix checked into proxy_util should
avoid the segfault in ap_proxy_string_read() (Thanks Adam!)

I am beginning to collect some notes to put into a 'how-to' document for developers. There
are certain patterns that repeat all over the server and it would be good to codify that
knowledge in one reference document.

Bill


Mime
View raw message