From 3f133570b862009c6100b6cc740519b8cfb2c420 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 31 Oct 2017 19:21:06 +0100 Subject: [PATCH] BUG/MEDIUM: h2: fix incorrect timeout handling on the connection Previous commit ea3928 (MEDIUM: h2: apply a timeout to h2 connections) was wrong for two reasons. The first one is that if the client timeout is not set, it's used as zero, preventing connections from establishing. The second reason is that if the timeout triggers with active streams (normally it should not since the task is supposed to be disabled), the task is removed (h2c->task=NULL), and the last quitting stream might try to dereference it. Instead of doing this, we simply not register the task if there's no timeout (it's useless) and we always control its presence in the streams. --- src/mux_h2.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/mux_h2.c b/src/mux_h2.c index b524f4952..bc1076884 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -327,9 +327,18 @@ static int h2c_frt_init(struct connection *conn) if (!h2c) goto fail; - t = task_new(tid_bit); - if (!t) - goto fail; + + h2c->timeout = sess->fe->timeout.client; + if (tick_isset(h2c->timeout)) { + t = task_new(tid_bit); + if (!t) + goto fail; + + h2c->task = t; + t->process = h2_timeout_task; + t->context = h2c; + t->expire = tick_add(now_ms, h2c->timeout); + } h2c->ddht = hpack_dht_alloc(h2_settings_header_table_size); if (!h2c->ddht) @@ -360,13 +369,8 @@ static int h2c_frt_init(struct connection *conn) LIST_INIT(&h2c->mbuf_wait.list); conn->mux_ctx = h2c; - h2c->timeout = sess->fe->timeout.client; - h2c->task = t; - t->process = h2_timeout_task; - t->context = h2c; - t->expire = tick_add(now_ms, h2c->timeout); - task_queue(t); - + if (t) + task_queue(t); conn_xprt_want_recv(conn); /* mux->wake will be called soon to complete the operation */ @@ -2048,13 +2052,14 @@ static int h2_wake(struct connection *conn) __conn_xprt_stop_send(conn); } - if (eb_is_empty(&h2c->streams_by_id)) { - h2c->task->expire = tick_add(now_ms, h2c->timeout); - task_queue(h2c->task); + if (h2c->task) { + if (eb_is_empty(&h2c->streams_by_id)) { + h2c->task->expire = tick_add(now_ms, h2c->timeout); + task_queue(h2c->task); + } + else + h2c->task->expire = TICK_ETERNITY; } - else - h2c->task->expire = TICK_ETERNITY; - return 0; } @@ -2191,12 +2196,14 @@ static void h2_detach(struct conn_stream *cs) /* h2s still attached to the h2c */ eb32_delete(&h2s->by_id); - if (eb_is_empty(&h2c->streams_by_id)) { - h2c->task->expire = tick_add(now_ms, h2c->timeout); - task_queue(h2c->task); + if (h2c->task) { + if (eb_is_empty(&h2c->streams_by_id)) { + h2c->task->expire = tick_add(now_ms, h2c->timeout); + task_queue(h2c->task); + } + else + h2c->task->expire = TICK_ETERNITY; } - else - h2c->task->expire = TICK_ETERNITY; /* We don't want to close right now unless we're removing the * last stream, and either the connection is in error, or it