From 39a96ee16eeec51774f9f52a783cf624a0de4ccb Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 8 Apr 2019 10:52:21 +0200
Subject: [PATCH] MEDIUM: muxes: Be prepared to don't own connection during the
 release

This happens during mux upgrades. In such case, when the destroy() callback is
called, the connection points to a different mux's context than the one passed
to the callback. It means the connection is owned by another mux. The old mux is
then released but the connection is not closed.
---
 src/mux_h1.c | 26 +++++++++++++++-----------
 src/mux_h2.c | 27 ++++++++++++++++-----------
 src/mux_pt.c | 30 ++++++++++++++++++------------
 3 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 43b062bbb..ef9dd01dc 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -445,6 +445,10 @@ static void h1_release(struct h1c *h1c)
 {
 	struct connection *conn = h1c->conn;
 
+	/* The connection was attached to another mux */
+	if (conn && conn->ctx != h1c)
+		conn = NULL;
+
 	if (h1c) {
 		if (!LIST_ISEMPTY(&h1c->buf_wait.list)) {
 			HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock);
@@ -466,20 +470,20 @@ static void h1_release(struct h1c *h1c)
 			tasklet_free(h1c->wait_event.task);
 
 		h1s_destroy(h1c->h1s);
-		if (h1c->wait_event.events != 0)
-			conn->xprt->unsubscribe(conn, h1c->wait_event.events,
-			    &h1c->wait_event);
 		pool_free(pool_head_h1c, h1c);
 	}
 
-	conn->mux = NULL;
-	conn->ctx = NULL;
+	if (conn) {
+		conn->mux = NULL;
+		conn->ctx = NULL;
 
-	conn_stop_tracking(conn);
-	conn_full_close(conn);
-	if (conn->destroy_cb)
-		conn->destroy_cb(conn);
-	conn_free(conn);
+		conn_force_unsubscribe(conn);
+		conn_stop_tracking(conn);
+		conn_full_close(conn);
+		if (conn->destroy_cb)
+			conn->destroy_cb(conn);
+		conn_free(conn);
+	}
 }
 
 /******************************************************/
@@ -1988,7 +1992,7 @@ static void h1_destroy(void *ctx)
 {
 	struct h1c *h1c = ctx;
 
-	if (!h1c->h1s)
+	if (!h1c->h1s || !h1c->conn || h1c->conn->ctx != h1c)
 		h1_release(h1c);
 }
 
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 32e1fc1ab..48876ee20 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -621,6 +621,11 @@ static void h2_release(struct h2c *h2c)
 {
 	struct connection *conn = h2c->conn;
 
+	/* The connection was attached to another mux (unexpected but safer to
+	 * check) */
+	if (conn && conn->ctx != h2c)
+		conn = NULL;
+
 	if (h2c) {
 		hpack_dht_free(h2c->ddht);
 
@@ -638,21 +643,21 @@ static void h2_release(struct h2c *h2c)
 		}
 		if (h2c->wait_event.task)
 			tasklet_free(h2c->wait_event.task);
-		if (h2c->wait_event.events != 0)
-			conn->xprt->unsubscribe(conn, h2c->wait_event.events,
-			    &h2c->wait_event);
 
 		pool_free(pool_head_h2c, h2c);
 	}
 
-	conn->mux = NULL;
-	conn->ctx = NULL;
+	if (conn) {
+		conn->mux = NULL;
+		conn->ctx = NULL;
 
-	conn_stop_tracking(conn);
-	conn_full_close(conn);
-	if (conn->destroy_cb)
-		conn->destroy_cb(conn);
-	conn_free(conn);
+		conn_force_unsubscribe(conn);
+		conn_stop_tracking(conn);
+		conn_full_close(conn);
+		if (conn->destroy_cb)
+			conn->destroy_cb(conn);
+		conn_free(conn);
+	}
 }
 
 
@@ -2989,7 +2994,7 @@ static void h2_destroy(void *ctx)
 {
 	struct h2c *h2c = ctx;
 
-	if (eb_is_empty(&h2c->streams_by_id))
+	if (eb_is_empty(&h2c->streams_by_id) || !h2c->conn || h2c->conn->ctx != h2c)
 		h2_release(h2c);
 }
 
diff --git a/src/mux_pt.c b/src/mux_pt.c
index 6fb9f2f56..82ba124b1 100644
--- a/src/mux_pt.c
+++ b/src/mux_pt.c
@@ -28,17 +28,23 @@ static void mux_pt_destroy(struct mux_pt_ctx *ctx)
 {
 	struct connection *conn = ctx->conn;
 
-	conn_stop_tracking(conn);
-	conn_full_close(conn);
-	tasklet_free(ctx->wait_event.task);
-	conn->mux = NULL;
-	conn->ctx = NULL;
-	if (conn->destroy_cb)
-		conn->destroy_cb(conn);
-	/* We don't bother unsubscribing here, as we're about to destroy
-	 * both the connection and the mux_pt_ctx
-	 */
-	conn_free(conn);
+	/* The connection was attached to another mux */
+	if (conn && conn->ctx != ctx)
+		conn = NULL;
+
+	if (conn) {
+		conn_stop_tracking(conn);
+		conn_full_close(conn);
+		tasklet_free(ctx->wait_event.task);
+		conn->mux = NULL;
+		conn->ctx = NULL;
+		if (conn->destroy_cb)
+			conn->destroy_cb(conn);
+		/* We don't bother unsubscribing here, as we're about to destroy
+		 * both the connection and the mux_pt_ctx
+		 */
+		conn_free(conn);
+	}
 	pool_free(pool_head_pt_ctx, ctx);
 }
 
@@ -172,7 +178,7 @@ static void mux_pt_destroy_meth(void *ctx)
 {
 	struct mux_pt_ctx *pt = ctx;
 
-	if (!(pt->cs))
+	if (!(pt->cs) || !(pt->conn) || pt->conn->ctx != pt)
 		mux_pt_destroy(pt);
 }