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.
diff --git a/src/mux_h2.c b/src/mux_h2.c
index b524f49..bc10768 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -327,9 +327,18 @@
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 @@
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 @@
__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 @@
/* 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