httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: byterange filter + apr_brigade_partition
Date Fri, 08 Jun 2001 07:00:16 GMT
On Thu, Jun 07, 2001 at 07:38:49PM -0400, Cliff Woolley wrote:
> 
> Hey...
> 
> I'm finally getting around to applying the six-month-old patch to change
> the prototype for apr_brigade_partition() to this:
> 
> APU_DECLARE(apr_status_t) apr_brigade_partition(apr_bucket_brigade *b,
>                                                 apr_off_t point,
>                                                 apr_bucket **after_point)
> 
> This is so that the caller can have more information when a failure
> occurs, and was at the request of Greg Stein when the function first came
> into existence.

Right. The reason is because the partition function can call read() on a
bucket. That can definitely fail, so we need to return that to the caller.

> The patch just slipped through the cracks and was
> forgotten since then.

Hah! Not even. I've been thinking about our filters quite a bit recently,
and I was going to go and dig up this patch. *snicker*

Part of what led me to thinking about the patch is that we have a number of
new "write" functions related to filters/brigades that return a byte count.
That means they completely toss status information. And the byte count is
never useful. The caller can always determine the byte count (they passed
the dumb arguments!), and the "bytes written" only means what was written to
the next filter, not what goes onto the nextwork or anything else
meaningful. And our definition of filters is "write everything passed". You
can't do a partial write to a filter stack. The byte count return value is
legacy from the ap_r* functions.

>From there... I remembered the partition status return code patch.

> Anyway, I'll have to patch the byterange filter in Apache to account for
> the change, and I'm looking for input on what the correct action is when a
> failure actually occurs.  Right now, Apache doesn't test for a failure to
> partition (which would have been indicated by a NULL return value) at all.
> So in the code below, what would be the right thing to stick in the "XXX:
> handle error" spot?

Dunno. Some kind of magic error bucket? Recovering from errors during filter
processing is totally undefined. If you've sent data to the client, then
there isn't much that you *can* do except to:

*) log an error
*) close the connection ("unexpectedly" for the client)

I am not sure of the best way to effect this change. I don't think that we
have a mechanism to shut down a connection/request. What would be really
neat is to toss the filter stack and insert an ERROR filter that simply
returns APR_EDISCONNECTED (or somesuch) whenever it is called. Hopefully,
somebody will see that error and stop generating content :-)

Of course, tossing the filter stack while *in it* is problematic. The
upstream filter still has "f" in their stack frame, and f->next is still
useful (the pool is still around, after all). To some extent,
ap_pass_brigade() would need to check the connection status to assist with
rapid shutdown.
(guys doing f->next->frec->filter_func.out_func(...) are screwed, but they
 deserve it.)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message