Skip to content

Commit 3c09b34

Browse files
committed
BUG/MEDIUM: mux-h1: Fix how timeouts are applied on H1 connections
There were several flaws in the way the different timeouts were applied on H1 connections. First, the H1C task handling timeouts was not created if no client/server timeout was specified. But there are other timeouts to consider. First, the client-fin/server-fin timeouts. But for frontend connections, http-keey-alive and http-request timeouts may also be used. And finally, on soft-stop, the close-spread-time value must be considered too. So at the end, it is probably easier to always create a task to manage H1C timeouts. Especially since the client/server timeouts are most often set. Then, when the expiration date of the H1C's task must only be updated if the considered timeout is set. So tick_add_ifset() must be used instead of tick_add(). Otherwise, if a timeout is undefined, the taks may expire immediately while it should in fact never expire. Finally, the idle expiration date must only be considered for idle connections. This patch should be backported in all stable versions, at least as far as 2.6. On the 2.4, it will have to be slightly adapted for the idle_exp part. On 2.2 and 2.0, the patch will have to be rewrite because h1_refresh_timeout() is quite different.
1 parent 9fa5b37 commit 3c09b34

File tree

1 file changed

+12
-18
lines changed

1 file changed

+12
-18
lines changed

src/mux_h1.c

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -694,24 +694,24 @@ static void h1_refresh_timeout(struct h1c *h1c)
694694
* timeouts so that we don't hang too long on clients that have
695695
* gone away (especially in tunnel mode).
696696
*/
697-
h1c->task->expire = tick_add(now_ms, h1c->shut_timeout);
697+
h1c->task->expire = tick_add_ifset(now_ms, h1c->shut_timeout);
698698
TRACE_DEVEL("refreshing connection's timeout (dead or half-closed)", H1_EV_H1C_SEND|H1_EV_H1C_RECV, h1c->conn);
699699
is_idle_conn = 1;
700700
}
701701
else if (b_data(&h1c->obuf)) {
702702
/* alive connection with pending outgoing data, need a timeout (server or client). */
703-
h1c->task->expire = tick_add(now_ms, h1c->timeout);
703+
h1c->task->expire = tick_add_ifset(now_ms, h1c->timeout);
704704
TRACE_DEVEL("refreshing connection's timeout (pending outgoing data)", H1_EV_H1C_SEND|H1_EV_H1C_RECV, h1c->conn);
705705
}
706706
else if (!(h1c->flags & H1C_F_IS_BACK) && (h1c->state == H1_CS_IDLE)) {
707707
/* idle front connections. */
708-
h1c->task->expire = (tick_isset(h1c->idle_exp) ? h1c->idle_exp : tick_add(now_ms, h1c->timeout));
708+
h1c->task->expire = (tick_isset(h1c->idle_exp) ? h1c->idle_exp : tick_add_ifset(now_ms, h1c->timeout));
709709
TRACE_DEVEL("refreshing connection's timeout (idle front h1c)", H1_EV_H1C_SEND|H1_EV_H1C_RECV, h1c->conn);
710710
is_idle_conn = 1;
711711
}
712712
else if (!(h1c->flags & H1C_F_IS_BACK) && (h1c->state != H1_CS_RUNNING)) {
713713
/* alive front connections waiting for a fully usable stream need a timeout. */
714-
h1c->task->expire = tick_add(now_ms, h1c->timeout);
714+
h1c->task->expire = tick_first(h1c->idle_exp, tick_add_ifset(now_ms, h1c->timeout));
715715
TRACE_DEVEL("refreshing connection's timeout (alive front h1c but not ready)", H1_EV_H1C_SEND|H1_EV_H1C_RECV, h1c->conn);
716716
/* A frontend connection not yet ready could be treated the same way as an idle
717717
* one in case of soft-close.
@@ -724,9 +724,6 @@ static void h1_refresh_timeout(struct h1c *h1c)
724724
TRACE_DEVEL("no connection timeout (alive back h1c or front h1c with an SC)", H1_EV_H1C_SEND|H1_EV_H1C_RECV, h1c->conn);
725725
}
726726

727-
/* Finally set the idle expiration date if shorter */
728-
h1c->task->expire = tick_first(h1c->task->expire, h1c->idle_exp);
729-
730727
if ((h1c->px->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) &&
731728
is_idle_conn && tick_isset(global.close_spread_end)) {
732729
/* If a soft-stop is in progress and a close-spread-time
@@ -1276,19 +1273,16 @@ static int h1_init(struct connection *conn, struct proxy *proxy, struct session
12761273
LIST_APPEND(&mux_stopping_data[tid].list,
12771274
&h1c->conn->stopping_list);
12781275
}
1279-
if (tick_isset(h1c->timeout)) {
1280-
t = task_new_here();
1281-
if (!t) {
1282-
TRACE_ERROR("H1C task allocation failure", H1_EV_H1C_NEW|H1_EV_H1C_END|H1_EV_H1C_ERR);
1283-
goto fail;
1284-
}
12851276

1286-
h1c->task = t;
1287-
t->process = h1_timeout_task;
1288-
t->context = h1c;
1289-
1290-
t->expire = tick_add(now_ms, h1c->timeout);
1277+
t = task_new_here();
1278+
if (!t) {
1279+
TRACE_ERROR("H1C task allocation failure", H1_EV_H1C_NEW|H1_EV_H1C_END|H1_EV_H1C_ERR);
1280+
goto fail;
12911281
}
1282+
h1c->task = t;
1283+
t->process = h1_timeout_task;
1284+
t->context = h1c;
1285+
t->expire = tick_add_ifset(now_ms, h1c->timeout);
12921286

12931287
conn->ctx = h1c;
12941288

0 commit comments

Comments
 (0)