BUG/MEDIUM: cli: Always release back endpoint between two commands on the mcli

When several commands are chained on the master CLI, the same client
connection is used. Because, it is a TCP connection, the mux PT is used. It
means there is no stream at the mux level. It is not possible to release the
applicative stream between each commands as for the HTTP. So, to work around
this limitation, between two commands, the master CLI is resetting the
stream. It does exactly what it was performed on HTTP to manage keep-alive
connections on old HAProxy versions.

But this part was copied from a code dealing with connection only while the
back endpoint can be an applet or a mux for the master cli. The previous fix
on the mux PT ("BUG/MEDIUM: mux-pt: Never fully close the connection on
shutdown") revealed a bug. Between two commands, the back endpoint was only
released if the connection's XPRT was closed. This works if the back
endpoint is an applet because there is no connection. But for commands sent
to a worker, a connection is used. At this stage, this only works if the
connection's XPRT is closed. Otherwise, the old endpoint is never detached
leading to undefined behavior on the next command execution (most probably a
crash).

Without the commit above, the connection's XPRT is always closed on
shutdown. It is no longer true. At this stage, we must inconditionnally
release the back endpoint by resetting the corresponding sedesc to fix the
bug.

This patch must be backported with the commit above in all stable
versions. On 2.4 and lower, it will need to be adapted.

diff --git a/src/cli.c b/src/cli.c
index 3e0ec1e506aeb643da16d5ea4f1de247709b4feb..fafe6a34aa042f3885212dd73e1334e774f474be 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -3183,20 +3183,16 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
 
                s->target = NULL;
 
-               /* only release our endpoint if we don't intend to reuse the
-                * connection.
-                */
-               if (!sc_conn_ready(s->scb)) {
-                       s->srv_conn = NULL;
-                       if (sc_reset_endp(s->scb) < 0) {
-                               if (!s->conn_err_type)
-                                       s->conn_err_type = STRM_ET_CONN_OTHER;
-                               if (s->srv_error)
-                                       s->srv_error(s, s->scb);
-                               return 1;
-                       }
-                       se_fl_clr(s->scb->sedesc, ~SE_FL_DETACHED);
+               /* Always release our endpoint */
+               s->srv_conn = NULL;
+               if (sc_reset_endp(s->scb) < 0) {
+                       if (!s->conn_err_type)
+                               s->conn_err_type = STRM_ET_CONN_OTHER;
+                       if (s->srv_error)
+                               s->srv_error(s, s->scb);
+                       return 1;
                }
+               se_fl_clr(s->scb->sedesc, ~SE_FL_DETACHED);
 
                sockaddr_free(&s->scb->dst);