From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 8 Apr 2019 09:23:22 +0000 (+0200)
Subject: MINOR: muxes: Pass the context of the mux to destroy() instead of the connection
X-Git-Tag: v2.0-dev3~291
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=73c1207c7196f3c1fc814e424e6b486cc5f3afb2;p=thirdparty%2Fhaproxy.git

MINOR: muxes: Pass the context of the mux to destroy() instead of the connection

It is mandatory to handle mux upgrades, because during a mux upgrade, the
connection will be reassigned to another multiplexer. So when the old one is
destroyed, it does not own the connection anymore. Or in other words, conn->ctx
does not point to the old mux's context when its destroy() callback is
called. So we now rely on the multiplexer context do destroy it instead of the
connection.

In addition, h1_release() and h2_release() have also been updated in the same
way.
---

diff --git a/include/proto/session.h b/include/proto/session.h
index f1e33fa0cd..2ff8e382d2 100644
--- a/include/proto/session.h
+++ b/include/proto/session.h
@@ -126,7 +126,7 @@ static inline int session_check_idle_conn(struct session *sess, struct connectio
 		conn->owner = NULL;
 		if (!srv_add_to_idle_list(objt_server(conn->target), conn)) {
 			/* The server doesn't want it, let's kill the connection right away */
-			conn->mux->destroy(conn);
+			conn->mux->destroy(conn->ctx);
 			return -1;
 		} else
 			conn->flags &= ~CO_FL_SESS_IDLE;
diff --git a/include/types/connection.h b/include/types/connection.h
index af27d0cd10..ab1f8bea81 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -346,7 +346,7 @@ struct mux_ops {
 	int (*unsubscribe)(struct conn_stream *cs, int event_type, void *param); /* Unsubscribe to events */
 	int (*avail_streams)(struct connection *conn); /* Returns the number of streams still available for a connection */
 	int (*used_streams)(struct connection *conn);  /* Returns the number of streams in use on a connection. */
-	void (*destroy)(struct connection *conn); /* Let the mux know one of its users left, so it may have to disappear */
+	void (*destroy)(void *ctx); /* Let the mux know one of its users left, so it may have to disappear */
 	void (*reset)(struct connection *conn); /* Reset the mux, because we're re-trying to connect */
 	const struct cs_info *(*get_cs_info)(struct conn_stream *cs); /* Return info on the specified conn_stream or NULL if not defined */
 	unsigned int flags;                           /* some flags characterizing the mux's capabilities (MX_FL_*) */
diff --git a/src/backend.c b/src/backend.c
index 5e82bf4503..abe7308e6f 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1371,7 +1371,7 @@ int connect_server(struct stream *s)
 				old_conn->owner = sess;
 				if (!session_add_conn(sess, old_conn, old_conn->target)) {
 					old_conn->owner = NULL;
-					old_conn->mux->destroy(old_conn);
+					old_conn->mux->destroy(old_conn->ctx);
 				} else
 					session_check_idle_conn(sess, old_conn);
 			}
@@ -1427,7 +1427,7 @@ int connect_server(struct stream *s)
 			srv_conn->owner = NULL;
 			if (srv_conn->mux && !srv_add_to_idle_list(objt_server(srv_conn->target), srv_conn))
 			/* The server doesn't want it, let's kill the connection right away */
-				srv_conn->mux->destroy(srv_conn);
+				srv_conn->mux->destroy(srv_conn->ctx);
 			srv_conn = NULL;
 
 		}
diff --git a/src/mux_h1.c b/src/mux_h1.c
index 50afbc20f4..43b062bbb5 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -438,13 +438,12 @@ static int h1_init(struct connection *conn, struct proxy *proxy, struct session
 	return -1;
 }
 
-
-/* release function for a connection. This one should be called to free all
- * resources allocated to the mux.
+/* release function. This one should be called to free all resources allocated
+ * to the mux.
  */
-static void h1_release(struct connection *conn)
+static void h1_release(struct h1c *h1c)
 {
-	struct h1c *h1c = conn->ctx;
+	struct connection *conn = h1c->conn;
 
 	if (h1c) {
 		if (!LIST_ISEMPTY(&h1c->buf_wait.list)) {
@@ -1867,7 +1866,7 @@ static int h1_process(struct h1c * h1c)
 	return 0;
 
   release:
-	h1_release(conn);
+	h1_release(h1c);
 	return -1;
 }
 
@@ -1936,13 +1935,14 @@ static struct task *h1_timeout_task(struct task *t, void *context, unsigned shor
 	if (h1c->h1s && h1c->h1s->cs)
 		h1c->flags |= H1C_F_CS_ERROR;
 	else
-		h1_release(h1c->conn);
+		h1_release(h1c);
 	return NULL;
 }
 
 /*******************************************/
 /* functions below are used by the streams */
 /*******************************************/
+
 /*
  * Attach a new stream to a connection
  * (Used for outgoing connections)
@@ -1984,12 +1984,12 @@ static const struct conn_stream *h1_get_first_cs(const struct connection *conn)
 	return NULL;
 }
 
-static void h1_destroy(struct connection *conn)
+static void h1_destroy(void *ctx)
 {
-	struct h1c *h1c = conn->ctx;
+	struct h1c *h1c = ctx;
 
 	if (!h1c->h1s)
-		h1_release(conn);
+		h1_release(h1c);
 }
 
 /*
@@ -2065,7 +2065,7 @@ static void h1_detach(struct conn_stream *cs)
 	/* We don't want to close right now unless the connection is in error */
 	if ((h1c->flags & (H1C_F_CS_ERROR|H1C_F_CS_SHUTDOWN)) ||
 	    (h1c->conn->flags & CO_FL_ERROR) || !h1c->conn->owner)
-		h1_release(h1c->conn);
+		h1_release(h1c);
 	else {
 		tasklet_wakeup(h1c->wait_event.task);
 		if (h1c->task) {
diff --git a/src/mux_h2.c b/src/mux_h2.c
index e440424813..32e1fc1ab1 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -614,12 +614,12 @@ static inline struct h2s *h2c_st_by_id(struct h2c *h2c, int id)
 	return container_of(node, struct h2s, by_id);
 }
 
-/* release function for a connection. This one should be called to free all
- * resources allocated to the mux.
+/* release function. This one should be called to free all resources allocated
+ * to the mux.
  */
-static void h2_release(struct connection *conn)
+static void h2_release(struct h2c *h2c)
 {
-	struct h2c *h2c = conn->ctx;
+	struct connection *conn = h2c->conn;
 
 	if (h2c) {
 		hpack_dht_free(h2c->ddht);
@@ -2845,7 +2845,7 @@ static int h2_process(struct h2c *h2c)
 
 		if (eb_is_empty(&h2c->streams_by_id)) {
 			/* no more stream, kill the connection now */
-			h2_release(conn);
+			h2_release(h2c);
 			return -1;
 		}
 	}
@@ -2930,7 +2930,7 @@ static struct task *h2_timeout_task(struct task *t, void *context, unsigned shor
 	 * the last stream closes.
 	 */
 	if (eb_is_empty(&h2c->streams_by_id))
-		h2_release(h2c->conn);
+		h2_release(h2c);
 
 	return NULL;
 }
@@ -2985,12 +2985,12 @@ static const struct conn_stream *h2_get_first_cs(const struct connection *conn)
 /*
  * Destroy the mux and the associated connection, if it is no longer used
  */
-static void h2_destroy(struct connection *conn)
+static void h2_destroy(void *ctx)
 {
-	struct h2c *h2c = conn->ctx;
+	struct h2c *h2c = ctx;
 
 	if (eb_is_empty(&h2c->streams_by_id))
-		h2_release(h2c->conn);
+		h2_release(h2c);
 }
 
 /*
@@ -3082,7 +3082,7 @@ static void h2_detach(struct conn_stream *cs)
 	 */
 	if (h2c_is_dead(h2c)) {
 		/* no more stream will come, kill it now */
-		h2_release(h2c->conn);
+		h2_release(h2c);
 	}
 	else if (h2c->task) {
 		if (eb_is_empty(&h2c->streams_by_id) || b_data(&h2c->mbuf)) {
@@ -3248,7 +3248,7 @@ static struct task *h2_deferred_shut(struct task *t, void *ctx, unsigned short s
 		h2s_destroy(h2s);
 
 		if (h2c_is_dead(h2c))
-			h2_release(h2c->conn);
+			h2_release(h2c);
 	}
 
 	return NULL;
diff --git a/src/mux_pt.c b/src/mux_pt.c
index 2086d5fd0a..6fb9f2f56e 100644
--- a/src/mux_pt.c
+++ b/src/mux_pt.c
@@ -166,13 +166,14 @@ static const struct conn_stream *mux_pt_get_first_cs(const struct connection *co
 	return cs;
 }
 
-/* Destroy the mux and the associated connection, if no longer used */
-static void mux_pt_destroy_meth(struct connection *conn)
+/* Destroy the mux and the associated connection if still attached to this mux
+ * and no longer used */
+static void mux_pt_destroy_meth(void *ctx)
 {
-	struct mux_pt_ctx *ctx = conn->ctx;
+	struct mux_pt_ctx *pt = ctx;
 
-	if (!(ctx->cs))
-		mux_pt_destroy(ctx);
+	if (!(pt->cs))
+		mux_pt_destroy(pt);
 }
 
 /*
diff --git a/src/server.c b/src/server.c
index bffc2ee50c..25a9189310 100644
--- a/src/server.c
+++ b/src/server.c
@@ -5317,7 +5317,7 @@ struct task *srv_cleanup_toremove_connections(struct task *task, void *context,
 
 	while ((conn = LIST_POP_LOCKED(&toremove_connections[tid],
 	                               struct connection *, list)) != NULL) {
-		conn->mux->destroy(conn);
+		conn->mux->destroy(conn->ctx);
 	}
 
 	return task;
diff --git a/src/session.c b/src/session.c
index ab49fb6fbd..9b0db46b23 100644
--- a/src/session.c
+++ b/src/session.c
@@ -74,7 +74,7 @@ void session_free(struct session *sess)
 	vars_prune_per_sess(&sess->vars);
 	conn = objt_conn(sess->origin);
 	if (conn != NULL && conn->mux)
-		conn->mux->destroy(conn);
+		conn->mux->destroy(conn->ctx);
 	list_for_each_entry_safe(srv_list, srv_list_back, &sess->srv_list, srv_list) {
 		list_for_each_entry_safe(conn, conn_back, &srv_list->conn_list, session_list) {
 			if (conn->mux) {
@@ -84,7 +84,7 @@ void session_free(struct session *sess)
 				conn->owner = NULL;
 				conn->flags &= ~CO_FL_SESS_IDLE;
 				if (!srv_add_to_idle_list(objt_server(conn->target), conn))
-					conn->mux->destroy(conn);
+					conn->mux->destroy(conn->ctx);
 			} else {
 				/* We have a connection, but not yet an associated mux.
 				 * So destroy it now.