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.