On 12.03.2010 21:07, Stefan Fritsch wrote: > On Fri, 12 Mar 2010, Ruediger Pluem wrote: >> On 10.03.2010 20:40, [email protected] 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
