Skip to content

Commit

Permalink
* Analyzing PR63170 more, mod_proxy_http2 needs to differentiate bet…
Browse files Browse the repository at this point in the history
…ween its hosting

   stream gone and its master connection gone. The later is terminal, the former is not.
 * mod_proxy_http2: ping the other side of an idle connection only when not already
   waiting on a PING response.
  • Loading branch information
Stefan Eissing committed Feb 19, 2019
1 parent 090da49 commit 3461781
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 11 deletions.
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
v1.12.5
--------------------------------------------------------------------------------
* Analyzing PR63170 more, mod_proxy_http2 needs to differentiate between its hosting
stream gone and its master connection gone. The later is terminal, the former is not.
* mod_proxy_http2: ping the other side of an idle connection only when not already
waiting on a PING response.

v1.12.4
--------------------------------------------------------------------------------
* Fixed an issue where a proxy_http2 handler entered an infinit loop when encountering
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#

AC_PREREQ([2.69])
AC_INIT([mod_http2], [1.12.4], [[email protected]])
AC_INIT([mod_http2], [1.12.5], [[email protected]])

LT_PREREQ([2.2.6])
LT_INIT()
Expand Down
9 changes: 8 additions & 1 deletion mod_http2/h2_mplx.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ static int stream_cancel_iter(void *ctx, void *val) {
void h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
{
apr_status_t status;
int i, wait_secs = 60;
int i, wait_secs = 60, old_aborted;

ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c,
"h2_mplx(%ld): start release", m->id);
Expand All @@ -440,6 +440,12 @@ void h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)

H2_MPLX_ENTER_ALWAYS(m);

/* While really terminating any slave connections, treat the master
* connection as aborted. It's not as if we could send any more data
* at this point. */
old_aborted = m->c->aborted;
m->c->aborted = 1;

/* How to shut down a h2 connection:
* 1. cancel all streams still active */
while (!h2_ihash_iter(m->streams, stream_cancel_iter, m)) {
Expand Down Expand Up @@ -484,6 +490,7 @@ void h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
h2_ihash_iter(m->shold, unexpected_stream_iter, m);
}

m->c->aborted = old_aborted;
H2_MPLX_LEAVE(m);

ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c,
Expand Down
10 changes: 6 additions & 4 deletions mod_http2/h2_proxy_session.c
Original file line number Diff line number Diff line change
Expand Up @@ -653,10 +653,12 @@ h2_proxy_session *h2_proxy_session_setup(const char *id, proxy_conn_rec *p_conn,
}
else {
h2_proxy_session *session = p_conn->data;
apr_interval_time_t age = apr_time_now() - session->last_frame_received;
if (age > apr_time_from_sec(1)) {
session->check_ping = 1;
nghttp2_submit_ping(session->ngh2, 0, (const uint8_t *)"nevergonnagiveyouup");
if (!session->check_ping) {
apr_interval_time_t age = apr_time_now() - session->last_frame_received;
if (age > apr_time_from_sec(1)) {
session->check_ping = 1;
nghttp2_submit_ping(session->ngh2, 0, (const uint8_t *)"nevergonnagiveyouup");
}
}
}
return p_conn->data;
Expand Down
4 changes: 2 additions & 2 deletions mod_http2/h2_version.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@
* @macro
* Version number of the http2 module as c string
*/
#define MOD_HTTP2_VERSION "1.12.4-git"
#define MOD_HTTP2_VERSION "1.12.5-git"

/**
* @macro
* Numerical representation of the version number of the http2 module
* release. This is a 24 bit number with 8 bits for major number, 8 bits
* for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
*/
#define MOD_HTTP2_VERSION_NUM 0x010c04
#define MOD_HTTP2_VERSION_NUM 0x010c05


#endif /* mod_h2_h2_version_h */
25 changes: 22 additions & 3 deletions mod_http2/mod_proxy_http2.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ static void (*req_engine_done)(h2_req_engine *engine, conn_rec *r_conn,
apr_status_t status);

typedef struct h2_proxy_ctx {
conn_rec *master;
conn_rec *owner;
apr_pool_t *pool;
request_rec *rbase;
Expand Down Expand Up @@ -363,7 +364,7 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) {
ctx->session->user_data = ctx;

while (1) {
if (ctx->owner->aborted) {
if (ctx->master->aborted) {
status = APR_ECONNABORTED;
goto out;
}
Expand Down Expand Up @@ -416,7 +417,9 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) {
next_request(ctx, 1);
h2_proxy_session_cancel_all(ctx->session);
h2_proxy_session_process(ctx->session);
status = ctx->r_status = APR_SUCCESS;
if (!ctx->master->aborted) {
status = ctx->r_status = APR_SUCCESS;
}
}

ctx->session->user_data = NULL;
Expand Down Expand Up @@ -508,6 +511,7 @@ static int proxy_http2_handler(request_rec *r,
}

ctx = apr_pcalloc(r->pool, sizeof(*ctx));
ctx->master = r->connection->master? r->connection->master : r->connection;
ctx->owner = r->connection;
ctx->pool = r->pool;
ctx->rbase = r;
Expand All @@ -529,6 +533,11 @@ static int proxy_http2_handler(request_rec *r,
"H2: serving URL %s", url);

run_connect:
if (ctx->master->aborted) {
ctx->r_status = APR_ECONNABORTED;
goto cleanup;
}

/* Get a proxy_conn_rec from the worker, might be a new one, might
* be one still open from another request, or it might fail if the
* worker is stopped or in error. */
Expand Down Expand Up @@ -602,6 +611,11 @@ static int proxy_http2_handler(request_rec *r,
}

run_session:
if (ctx->owner->aborted) {
ctx->r_status = APR_ECONNABORTED;
goto cleanup;
}

status = proxy_engine_run(ctx);
if (status == APR_SUCCESS) {
/* session and connection still ok */
Expand All @@ -616,6 +630,11 @@ static int proxy_http2_handler(request_rec *r,
}

reconnect:
if (ctx->master->aborted) {
ctx->r_status = APR_ECONNABORTED;
goto cleanup;
}

if (next_request(ctx, 1) == APR_SUCCESS) {
/* Still more to do, tear down old conn and start over */
if (ctx->p_conn) {
Expand All @@ -627,7 +646,7 @@ static int proxy_http2_handler(request_rec *r,
ctx->p_conn = NULL;
}
++reconnects;
if (reconnects < 5 && !ctx->owner->aborted) {
if (reconnects < 5 && !ctx->master->aborted) {
goto run_connect;
}
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(10023)
Expand Down

0 comments on commit 3461781

Please sign in to comment.