httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r921526 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
Date Mon, 15 Mar 2010 07:29:56 GMT
On 12.03.2010 21:07, Stefan Fritsch wrote:
> On Fri, 12 Mar 2010, Ruediger Pluem wrote:
>> On 10.03.2010 20:40, sf@apache.org wrote:
>>> +            if (!APR_BRIGADE_EMPTY(bb)) {
>>> +                rv = have_lf_or_eos(bb);
>>> +                if (rv != APR_INCOMPLETE) {
>>> +                    break;
>>> +                }
>>> +
>>> +                if (ccfg->min_rate > 0) {
>>> +                    extend_timeout(ccfg, bb);
>>> +                }
>>
>> Hm, shouldn't we extend the timeout in the case of APR_SUCCESS as well?
>> We likely have future reads of headers in this case.
> 
> True, thanks. Fixed in r922407.
> 
>>> +out:
>>>      if (rv == APR_TIMEUP) {
>>>          ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, f->c,
>>>                        "Request %s read timeout", ccfg->type);
>>> +        /*
>>> +         * If we allow lingering close, the client may keep this
>>> +         * process/thread busy for another 30s (MAX_SECS_TO_LINGER).
>>> +         * Therefore we have to abort the connection. The downside is
>>> +         * that the client will most likely not receive the error
>>> +         * message.
>>> +         */
>>> +        f->c->aborted = 1;
>>
>> We had previous discussions on list (don't have pointers at hand right
>> now),
>> that c->aborted is not allowed to be used to abort a connection from
>> our side
>> but that it is strictly reserved to indicate that the client aborted the
>> connection. On the other side I currently have no alternate proposal
>> to get
>> the worthwhile effect you want by doing this. Maybe we need to create
>> a new
>> API / flag on trunk for this?
> 
> I didn't know about that restriction of c->aborted.
> 
> Maybe add a note to the conn_rec that we need a quick lingering close
> and shorten the max wait time in ap_lingering_close() from 30 to 2
> seconds if that note is set? Since this will only be used in error
> conditions, the relatively expensive setting/checking of a note should
> not be a problem here. One could also make the lingering close wait time
> configurable, but I don't think that is really necessary.
> 
> 

IMHO the idea with note sounds good. Other opinions?

Regards

RĂ¼diger



Mime
View raw message