On Thu, Dec 21, 2017 at 7:04 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
> On Thu, Dec 21, 2017 at 6:59 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>> On Thu, Dec 21, 2017 at 6:49 PM, Stefan Eissing <ste...@eissing.org> wrote:
>>> Oops, thought it was OK. That causes the hangers?
>>
>> Not anymore :)
>>
>> But h2_conn_run() seems to possibly return CONN_STATE_HANDLER, it
>> looks stange to me, and for once causes hanger ;)
>> What's the meaning of it?
>
> That's for when it's called from h2_protocol_switch() possibly, the
> issue is that it's also called from h2_h2_process_conn(), a
> process_connection hook which returns straight to the MPM...
Actually I don't see any other possible state than CONN_STATE_LINGER
after h2_conn_run(), the master connection has to be terminated in
both h2_h2_process_conn() hook and
core_upgrade_handler::ap_switch_protocol() cases.
So how about the attached patch?
It changes h2_conn_run() to finally/unconditionally set
CONN_STATE_LINGER after having flushed and aborted the connection
(aborting prevents further unnecessary actions like
ap_check_pipeline() or lingering before closing the socket, once the
connection is flushed...).
This patch does also s/return DONE/return OK/ where appropriate (the
DONE in core_upgrade_handler() is fine because we want to stop request
processing from there).
Index: modules/http2/h2_conn.c
===================================================================
--- modules/http2/h2_conn.c (revision 1818982)
+++ modules/http2/h2_conn.c (working copy)
@@ -253,25 +253,14 @@ apr_status_t h2_conn_run(struct h2_ctx *ctx, conn_
} while (!async_mpm
&& c->keepalive == AP_CONN_KEEPALIVE
&& mpm_state != AP_MPMQ_STOPPING);
-
+
+ ap_flush_conn(c);
+ c->aborted = 1;
if (c->cs) {
- switch (session->state) {
- case H2_SESSION_ST_INIT:
- case H2_SESSION_ST_CLEANUP:
- case H2_SESSION_ST_DONE:
- case H2_SESSION_ST_IDLE:
- c->cs->state = CONN_STATE_WRITE_COMPLETION;
- break;
- case H2_SESSION_ST_BUSY:
- case H2_SESSION_ST_WAIT:
- default:
- c->cs->state = CONN_STATE_HANDLER;
- break;
-
- }
+ c->cs->state = CONN_STATE_LINGER;
}
-
- return DONE;
+
+ return APR_SUCCESS;
}
apr_status_t h2_conn_pre_close(struct h2_ctx *ctx, conn_rec *c)
Index: modules/http2/h2_h2.c
===================================================================
--- modules/http2/h2_h2.c (revision 1818982)
+++ modules/http2/h2_h2.c (working copy)
@@ -669,10 +669,11 @@ int h2_h2_process_conn(conn_rec* c)
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c, "conn_setup");
if (status != APR_SUCCESS) {
h2_ctx_clear(c);
- return status;
+ return !OK;
}
}
- return h2_conn_run(ctx, c);
+ h2_conn_run(ctx, c);
+ return OK;
}
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, declined");
Index: modules/http2/h2_switch.c
===================================================================
--- modules/http2/h2_switch.c (revision 1818982)
+++ modules/http2/h2_switch.c (working copy)
@@ -185,13 +185,12 @@ static int h2_protocol_switch(conn_rec *c, request
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(03088)
"session setup");
h2_ctx_clear(c);
- return status;
+ return !OK;
}
h2_conn_run(ctx, c);
- return DONE;
}
- return DONE;
+ return OK;
}
return DECLINED;
