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.

Reply via email to