From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 1 Jun 2021 12:06:05 +0000 (+0200)
Subject: BUG/MAJOR: stream-int: Release SI endpoint on server side ASAP on retry
X-Git-Tag: v2.5-dev1~213
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f822decfda1f662147c02fd8b4f752da7500f374;p=thirdparty%2Fhaproxy.git

BUG/MAJOR: stream-int: Release SI endpoint on server side ASAP on retry

When a connection attempt failed, if a retry is possible, the SI endpoint on
the server side is immediately released, instead of waiting to establish a
new connection to a server. Thus, when the backend SI is switched from
SI_ST_CER state to SI_ST_REQ, SI_ST_ASS or SI_ST_TAR, its endpoint is
released. It is expected because the SI is moved to a state prior to the
connection stage ( < SI_ST_CONN). So it seems logical to not have any server
connection.

It is especially important if the retry is delayed (SI_ST_TAR or
SI_ST_QUE). Because, if the server connection is preserved, any error at the
connection level is unexpectedly relayed to the stream, via the
stream-interface, leading to an infinite loop in process_stream(). if
SI_FL_ERR flag is set on the backend SI in another state than SI_ST_CLO, an
internal goto is performed to resync the stream-interfaces. In addtition,
some ressources are not released ASAP.

This bug is quite old and was reported 1 or 2 times per years since the 2.2
(at least) with not enough information to catch it. It must be backported as
far as 2.2 with a special care because this part has moved several times and
after some observation period and feedback from users to be sure. For info,
in 2.0 and prior, the connection is released when an error is encountered in
SI_ST_CON or SI_ST_RDY states.
---

diff --git a/src/backend.c b/src/backend.c
index 8b06ddb779..0f6b9db3a6 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -2254,6 +2254,18 @@ void back_handle_st_cer(struct stream *s)
 		goto end;
 	}
 
+	/* At this stage, we will trigger a connection retry (with or without
+	 * redispatch). Thus we must release the SI endpoint on the server side
+	 * an close the attached connection. It is especially important to do it
+	 * now if the retry is not immediately performed, to be sure to release
+	 * ressources as soon as possible and to not catch errors from the lower
+	 * layers in an unexpected state (i.e < ST_CONN).
+	 *
+	 * Note: the stream-interface will be switched to ST_REQ, ST_ASS or
+	 * ST_TAR and SI_FL_ERR and SI_FL_EXP flags will be unset.
+	 */
+	si_release_endpoint(&s->si[1]);
+
 	stream_choose_redispatch(s);
 
 	if (si->flags & SI_FL_ERR) {