Re: OLD_WRITE stuff (was: Re: 2.0.34 - erratic behavior with autoindexes)
On Tue, Apr 02, 2002 at 02:56:33PM -0800, Ryan Bloom wrote: > > Everything below is true, assuming you are not talking about > sub-requests. According to the stack that Greg posted, the OLD_WRITE > filter was not found in the sub-request filter stack. That is to be > expected, because the OLD_WRITE filter is a RESOURCE filter. Since > RESOURCE filters are not kept for sub-requests, the data must be > flushed. Right. > Hm Actually it seems like Resource filters should be kept for > sub-requests that are created with a filter list. If that is the case, > then it looks like the OLD_WRITE filter should be in the filter list. If the sub-request goes, then its resource filters should go. Those filters are associated with that particular sub-request. That implies that the resource filters should be flushed before they die. [ but we should probably have a SUBREQ_ENDING_FLUSH bucket so the SUBREQ filter can chew it up; we don't want to flush all the way to the network just cuz the subreq is disappearing. ] > Unless, the OLD_WRITE filter is being added after the sub-request is > created.. The filter is added at the first call to ap_r*(). We decided on the lazy approach so that pure-brigade systems would not suffer the overhead. Cheers, -g -- Greg Stein, http://www.lyra.org/
RE: OLD_WRITE stuff (was: Re: 2.0.34 - erratic behavior with autoindexes)
Everything below is true, assuming you are not talking about sub-requests. According to the stack that Greg posted, the OLD_WRITE filter was not found in the sub-request filter stack. That is to be expected, because the OLD_WRITE filter is a RESOURCE filter. Since RESOURCE filters are not kept for sub-requests, the data must be flushed. Hm Actually it seems like Resource filters should be kept for sub-requests that are created with a filter list. If that is the case, then it looks like the OLD_WRITE filter should be in the filter list. Unless, the OLD_WRITE filter is being added after the sub-request is created.. Lots to look at.. Ryan > On Tue, Apr 02, 2002 at 01:50:59PM -0800, Ryan Bloom wrote: > >... > > > chain. Bill S. stuck his head in here and said something about a rule > > > that if old_write is ever used, it has to always be used. > > > > If you read through mod_include, you will see that whenever we create a > > sub-request, we send the output from the current filter to the next > > filter. This is required to make sure that things are output in the > > correct order. If you are a handler and are generating data through > > ap_r* functions, you must flush the filter stack so that you are sure > > that OLD_WRITE isn't buffering for you. > > That's not true. OLD_WRITE was designed to allow a mix/match of writing to > the stack *and* using the ap_r* functions. > > The design is that ap_r* implies a write into the filter stack. That write > is optimized to use a brigade for buffering (so every ap_rputc() doesn't > traverse the filter stack). If OLD_WRITE isn't at the top, then it doesn't > do the buffering (because there are filters between the top and the > OLD_WRITE buffers). > > If somebody writes directly into the filter stack, then it prepends any > buffered content to whatever is written, and then passes it all down the > stack. > > [ just reviewed the code; it seems to still match the original design ] > > It all looks pretty good, but there might be a problem where: > > *) ap_r* is used, so OLD_WRITE is inserted, and the content is stored into >its buffer. > > *) another filter is inserted "above" OLD_WRITE, so it is not the top. > ap_r* >content should now go into this new filter. > > *) ap_r* is called again, but it thinks it cannot buffer. it passes the > new >data directly into the filter stack. > >BUG: the previously-buffered content is not prepended. > > > So... the bug might appear because of the increased dynamicity of the > filter > chain nowadays. Back when OLD_WRITE was written, it was perceived that the > chain would be quite static by the time the first write occurred. > > Note that OLD_WRITE tries quite hard to keep its filter on "top" (its > filter > type is RESOURCE-10). If something broke that, then it is possible to hit > the above bug. > > The fix to the (potential) bug is to change line 1370 in server/protocol.c > to concat the brigades, similar to ap_old_write_filter() (defined just > above, at line 1316). > > Another question is whether anybody defines their filter type as less than > RESOURCE-10 and ends up ahead of OLD_WRITE. > > And lastly: I just realized that if a filter gets in front of OLD_WRITE, > then the branch at line 1366 will get called for each ap_r* call. If that > is > a series of 1000's of ap_rputc() calls, then you'll end up creating > thousands of brigades for those transient buckets(!). > > [ a similar brigade over-creation can occur if you alternate ap_r* and > writing to the filter stack; ap_old_write_filter() will forget about the > brigade it created, so the next ap_r* needs to recreate one ] > > The answer might be to test for brigade-empty rather than ctx->bb == NULL. > Then the branch at 1366 can store and reuse a brigade in ctx->bb. > > Cheers, > -g
OLD_WRITE stuff (was: Re: 2.0.34 - erratic behavior with autoindexes)
On Tue, Apr 02, 2002 at 01:50:59PM -0800, Ryan Bloom wrote: >... > > chain. Bill S. stuck his head in here and said something about a rule > > that if old_write is ever used, it has to always be used. > > If you read through mod_include, you will see that whenever we create a > sub-request, we send the output from the current filter to the next > filter. This is required to make sure that things are output in the > correct order. If you are a handler and are generating data through > ap_r* functions, you must flush the filter stack so that you are sure > that OLD_WRITE isn't buffering for you. That's not true. OLD_WRITE was designed to allow a mix/match of writing to the stack *and* using the ap_r* functions. The design is that ap_r* implies a write into the filter stack. That write is optimized to use a brigade for buffering (so every ap_rputc() doesn't traverse the filter stack). If OLD_WRITE isn't at the top, then it doesn't do the buffering (because there are filters between the top and the OLD_WRITE buffers). If somebody writes directly into the filter stack, then it prepends any buffered content to whatever is written, and then passes it all down the stack. [ just reviewed the code; it seems to still match the original design ] It all looks pretty good, but there might be a problem where: *) ap_r* is used, so OLD_WRITE is inserted, and the content is stored into its buffer. *) another filter is inserted "above" OLD_WRITE, so it is not the top. ap_r* content should now go into this new filter. *) ap_r* is called again, but it thinks it cannot buffer. it passes the new data directly into the filter stack. BUG: the previously-buffered content is not prepended. So... the bug might appear because of the increased dynamicity of the filter chain nowadays. Back when OLD_WRITE was written, it was perceived that the chain would be quite static by the time the first write occurred. Note that OLD_WRITE tries quite hard to keep its filter on "top" (its filter type is RESOURCE-10). If something broke that, then it is possible to hit the above bug. The fix to the (potential) bug is to change line 1370 in server/protocol.c to concat the brigades, similar to ap_old_write_filter() (defined just above, at line 1316). Another question is whether anybody defines their filter type as less than RESOURCE-10 and ends up ahead of OLD_WRITE. And lastly: I just realized that if a filter gets in front of OLD_WRITE, then the branch at line 1366 will get called for each ap_r* call. If that is a series of 1000's of ap_rputc() calls, then you'll end up creating thousands of brigades for those transient buckets(!). [ a similar brigade over-creation can occur if you alternate ap_r* and writing to the filter stack; ap_old_write_filter() will forget about the brigade it created, so the next ap_r* needs to recreate one ] The answer might be to test for brigade-empty rather than ctx->bb == NULL. Then the branch at 1366 can store and reuse a brigade in ctx->bb. Cheers, -g -- Greg Stein, http://www.lyra.org/