On Sat, Aug 30, 2014 at 3:19 PM, Yann Ylavic <[email protected]> wrote:
> On Sat, Aug 30, 2014 at 3:02 PM, Eric Covener <[email protected]> wrote:
>> On Tue, Aug 26, 2014 at 5:22 AM, Yann Ylavic <[email protected]> wrote:
>>> I don't think mod_reqtimeout should handle/count speculative bytes,
>>> they ought to be read for real later (and taken into account then).
>>> Otherwise, the same bytes may be counted multiple times.
>>>
>>> How about simply forward the ap_get_brigade() call?
>>
>> Makes sense -- I did limit it to nonblock as well. Can you take a
>> look before I propose? http://svn.apache.org/r1621453
>
> I'm not sure we should limit it to nonblock, speculative mode is
> mainly to be used in non-blocking, but ap_rgetline_core() for example
> does not (when folding), so mod_reqtimeout may still count header
> bytes twice.
Eric, Jeff, since you already voted for r1621453 in 2.4.x/STATUS, how
about this additional patch?
As said above, IMHO we really shouldn't count any speculative bytes in
mod_reqtimeout (nonblocking or not), relevant data should be asked
"for real" soon or later.
Index: modules/filters/mod_reqtimeout.c
===================================================================
--- modules/filters/mod_reqtimeout.c (revision 1640032)
+++ modules/filters/mod_reqtimeout.c (working copy)
@@ -183,12 +183,13 @@ static apr_status_t reqtimeout_filter(ap_filter_t
return ap_get_brigade(f->next, bb, mode, block, readbytes);
}
- if (block == APR_NONBLOCK_READ && mode == AP_MODE_SPECULATIVE) {
+ if (mode == AP_MODE_SPECULATIVE) {
/* The source of these above us in the core is check_pipeline(), which
* is between requests but before this filter knows to reset timeouts
- * during log_transaction(). If they appear elsewhere, just don't
- * check or extend the time since they won't block and we'll see the
- * bytes again later
+ * during log_transaction(), or ap_rgetline_core() to handle headers'
+ * folding (next char prefetch). Likewise, if they appear elsewhere,
+ * just don't check or extend the time since we should see the
+ * relevant bytes again later.
*/
return ap_get_brigade(f->next, bb, mode, block, readbytes);
}
>
> Regards,
> Yann.
