From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 21 Apr 2022 12:22:53 +0000 (+0200)
Subject: MINOR: conn-stream: Make cs_detach_* private and use cs_destroy() from outside
X-Git-Tag: v2.6-dev7~12
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eb50c01fef2701eb7810f9f79df3a9cf5526bfe1;p=thirdparty%2Fhaproxy.git

MINOR: conn-stream: Make cs_detach_* private and use cs_destroy() from outside

A conn-stream is never detached from an endpoint or an application alone,
except on a reset. Thus, to avoid any error, these functions are now
private. And cs_destroy() function is added to destroy a conn-stream. This
function is called when a stream is released, on the front and back
conn-streams, and when a health-check is finished.
---

diff --git a/include/haproxy/conn_stream.h b/include/haproxy/conn_stream.h
index 8511b96df3..2d5cc4ffa4 100644
--- a/include/haproxy/conn_stream.h
+++ b/include/haproxy/conn_stream.h
@@ -47,9 +47,8 @@ void cs_free(struct conn_stream *cs);
 int cs_attach_mux(struct conn_stream *cs, void *target, void *ctx);
 int cs_attach_strm(struct conn_stream *cs, struct stream *strm);
 
+void cs_destroy(struct conn_stream *cs);
 int cs_reset_endp(struct conn_stream *cs);
-void cs_detach_endp(struct conn_stream *cs);
-void cs_detach_app(struct conn_stream *cs);
 
 struct appctx *cs_applet_create(struct conn_stream *cs, struct applet *app);
 void cs_applet_shut(struct conn_stream *cs);
diff --git a/src/check.c b/src/check.c
index 71d3f3cdd5..c39561a508 100644
--- a/src/check.c
+++ b/src/check.c
@@ -1199,8 +1199,7 @@ struct task *process_chk_conn(struct task *t, void *context, unsigned int state)
 		 * the tasklet
 		 */
 		tasklet_remove_from_tasklet_list(check->wait_list.tasklet);
-		cs_detach_endp(cs);
-		cs_detach_app(cs);
+		cs_destroy(cs);
 		cs = check->cs = NULL;
 		conn = NULL;
 	}
@@ -1364,8 +1363,7 @@ void free_check(struct check *check)
 	check_release_buf(check, &check->bi);
 	check_release_buf(check, &check->bo);
 	if (check->cs) {
-		cs_detach_endp(check->cs);
-		cs_detach_app(check->cs);
+		cs_destroy(check->cs);
 		check->cs = NULL;
 	}
 }
diff --git a/src/conn_stream.c b/src/conn_stream.c
index e753749ca9..fda8ab44e8 100644
--- a/src/conn_stream.c
+++ b/src/conn_stream.c
@@ -244,13 +244,18 @@ void cs_free(struct conn_stream *cs)
 	pool_free(pool_head_connstream, cs);
 }
 
-/* Conditionally removes a conn-stream if it is detached and it there is no app
- * layer defined. Except on error path, this one must be used.
+/* Conditionally removes a conn-stream if it is detached and if there is no app
+ * layer defined. Except on error path, this one must be used. if release, the
+ * pointer on the CS is set to NULL.
  */
-static void cs_free_cond(struct conn_stream *cs)
+static void cs_free_cond(struct conn_stream **csp)
 {
-	if (!cs->app && (!cs->endp || (cs->endp->flags & CS_EP_DETACHED)))
+	struct conn_stream *cs = *csp;
+
+	if (!cs->app && (!cs->endp || (cs->endp->flags & CS_EP_DETACHED))) {
 		cs_free(cs);
+		*csp = NULL;
+	}
 }
 
 
@@ -344,8 +349,13 @@ int cs_attach_strm(struct conn_stream *cs, struct stream *strm)
  * endpoint is reset and flag as detached. If the app layer is also detached,
  * the conn-stream is released.
  */
-void cs_detach_endp(struct conn_stream *cs)
+static void cs_detach_endp(struct conn_stream **csp)
 {
+	struct conn_stream *cs = *csp;
+
+	if (!cs)
+		return;
+
 	if (!cs->endp)
 		goto reset_cs;
 
@@ -394,14 +404,19 @@ void cs_detach_endp(struct conn_stream *cs)
 	if (cs_strm(cs))
 		cs->ops = &cs_app_embedded_ops;
 	cs->data_cb = NULL;
-	cs_free_cond(cs);
+	cs_free_cond(csp);
 }
 
 /* Detaches the conn_stream from the app layer. If there is no endpoint attached
  * to the conn_stream
  */
-void cs_detach_app(struct conn_stream *cs)
+static void cs_detach_app(struct conn_stream **csp)
 {
+	struct conn_stream *cs = *csp;
+
+	if (!cs)
+		return;
+
 	cs->app = NULL;
 	cs->data_cb = NULL;
 	sockaddr_free(&cs->src);
@@ -411,7 +426,17 @@ void cs_detach_app(struct conn_stream *cs)
 		tasklet_free(cs->wait_event.tasklet);
 	cs->wait_event.tasklet = NULL;
 	cs->wait_event.events = 0;
-	cs_free_cond(cs);
+	cs_free_cond(csp);
+}
+
+/* Destroy the conn_stream. It is detached from its endpoint and its
+ * application. After this call, the conn_stream must be considered as released.
+ */
+void cs_destroy(struct conn_stream *cs)
+{
+	cs_detach_endp(&cs);
+	cs_detach_app(&cs);
+	BUG_ON_HOT(cs);
 }
 
 /* Resets the conn-stream endpoint. It happens when the app layer want to renew
@@ -426,9 +451,10 @@ int cs_reset_endp(struct conn_stream *cs)
 	if (!__cs_endp_target(cs)) {
 		/* endpoint not attached or attached to a mux with no
 		 * target. Thus the endpoint will not be release but just
-		 * reset
+		 * reset. The app is still attached, the cs will not be
+		 * released.
 		 */
-		cs_detach_endp(cs);
+		cs_detach_endp(&cs);
 		return 0;
 	}
 
@@ -440,7 +466,8 @@ int cs_reset_endp(struct conn_stream *cs)
 		return -1;
 	}
 
-	cs_detach_endp(cs);
+	/* The app is still attached, the cs will not be released */
+	cs_detach_endp(&cs);
 	BUG_ON(cs->endp);
 	cs->endp = new_endp;
 	cs->endp->flags |= CS_EP_DETACHED;
diff --git a/src/http_client.c b/src/http_client.c
index 6bf28d03b9..57ca92839f 100644
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -519,7 +519,6 @@ struct appctx *httpclient_start(struct httpclient *hc)
 			s->target = &httpclient_srv_ssl->obj_type;
 #else
 			ha_alert("httpclient: OpenSSL is not available %s:%d.\n", __FUNCTION__, __LINE__);
-			cs_detach_app(cs);
 			LIST_DELETE(&s->list);
 			pool_free(pool_head_stream, s);
 			cs_free(cs);
diff --git a/src/stream.c b/src/stream.c
index ace922d31d..3d5c740691 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -714,10 +714,8 @@ static void stream_free(struct stream *s)
 	/* FIXME: Handle it in appctx_free ??? */
 	must_free_sess = objt_appctx(sess->origin) && sess->origin == __cs_endp_target(s->csf);
 
-	cs_detach_endp(s->csb);
-	cs_detach_endp(s->csf);
-	cs_detach_app(s->csb);
-	cs_detach_app(s->csf);
+	cs_destroy(s->csb);
+	cs_destroy(s->csf);
 
 	if (must_free_sess) {
 		sess->origin = NULL;