httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jus...@erenkrantz.com>
Subject Re: bug in ap_get_client_block (wrong handling of EOS)
Date Wed, 14 May 2003 04:40:52 GMT
--On Tuesday, May 13, 2003 7:00 PM +1000 Stas Bekman <stas@stason.org> wrote:

> but this is bogus. If the EOS bucket arrives in the same bucket brigade with
> real data, ap_get_client_block() will return the size of the read data,
> completely missing the fact that EOS went through. Next it'll be called
> again (because the caller wasn't signalled with EOF) and bad things happen
> because there are no rules for how filters should behave after they have
> processed EOS the last time they were called.

The ap_get_client_block() API can't support that semantic of a partial-read 
given its interface.  ap_get_client_block is a kluge that only exists to make 
people who want partial source compatibility with 1.3 modules.  ap_get_brigade 
should be used instead...

> 1) replace apr_brigade_flatten with a loop that reads the data and sets the
> eos flag, so the next time this function is invoked it'll immediately return
> EOF. (not sure where to store this flag though)

No, that's lame.

> 1a) same as (1) but adjust apr_brigade_flatten to recognize situations when
> it read the EOS bucket, and somehow pass that knowledge back to the caller.
> Probably a bad idea since it adds a performance hit for situations where
> this check is not needed.

No.  People have been wanting to move away from the EOS returns partial data 
semantic (hi, Jeff!).  EOS should be on its own call in a flat, non-brigade 
situation.  When brigades are used, they are interleaved.

> The current behavior is broken and ap_get_client_block shouldn't be used at
> all, so it doesn't really matter if we change the API. So unless we can
> maintain the status elsewhere, need to modify the API for get_client_block
> to always return status, but have the size returned by reference.

I've maintained that 
ap_get_client_block/ap_setup_client_block/ap_should_client_block should be 
removed.  It has no place in a filter-centric API.  I'd suggest that we just 
remove it in 2.1.  We could place a hack in 2.0 that will search through the 
brigade - ideally just check APR_BRIGADE_LAST for EOS and set r->remaining to 
0.  If ap_get_client_block() sees r->remaining is 0, then it can assume we saw 
EOS.  Shouldn't need a MMN bump or anything like that.

I'm sick of dealing with the conflicts of keeping the old input API with the 
new input API.  They aren't equivalent and the mapping is just painful and 
kills performance when people could easily use the brigade model instead.

> Should all filters remember in their context that they have seen/sent eos?

No.  There is a global r->eos_sent though.

> If so, what good filters should do if they are being called again, even
> though they have sent the EOS bucket already?

Blow up and point fingers at the culprit who called EOS twice.  =)  -- justin

Mime
View raw message