From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 6 Feb 2024 13:54:54 +0000 (+0100)
Subject: MEDIUM: cli/applet: Stop to test opposite SC in I/O handler of CLI commands
X-Git-Tag: v3.0-dev7~59
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=94b8ed446f70a381dde0ea2dc891b62fcc3cc8e1;p=thirdparty%2Fhaproxy.git

MEDIUM: cli/applet: Stop to test opposite SC in I/O handler of CLI commands

The main CLI I/O handle is responsible to interrupt the processing on
shutdown/abort. It is not the responsibility of the I/O handler of CLI
commands to take care of it.
---

diff --git a/src/activity.c b/src/activity.c
index 07a30e60ca..d9f03e5650 100644
--- a/src/activity.c
+++ b/src/activity.c
@@ -647,17 +647,12 @@ static int cli_io_handler_show_profiling(struct appctx *appctx)
 	unsigned long long tot_alloc_calls, tot_free_calls;
 	unsigned long long tot_alloc_bytes, tot_free_bytes;
 #endif
-	struct stconn *sc = appctx_sc(appctx);
 	struct buffer *name_buffer = get_trash_chunk();
 	const struct ha_caller *caller;
 	const char *str;
 	int max_lines;
 	int i, j, max;
 
-	/* FIXME: Don't watch the other side ! */
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	chunk_reset(&trash);
 
 	switch (profiling & HA_PROF_TASKS_MASK) {
@@ -911,7 +906,6 @@ static int cli_parse_show_profiling(char **args, char *payload, struct appctx *a
 static int cli_io_handler_show_tasks(struct appctx *appctx)
 {
 	struct sched_activity tmp_activity[SCHED_ACT_HASH_BUCKETS] __attribute__((aligned(64)));
-	struct stconn *sc = appctx_sc(appctx);
 	struct buffer *name_buffer = get_trash_chunk();
 	struct sched_activity *entry;
 	const struct tasklet *tl;
@@ -922,10 +916,6 @@ static int cli_io_handler_show_tasks(struct appctx *appctx)
 	int thr, queue;
 	int i, max;
 
-	/* FIXME: Don't watch the other side ! */
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	/* It's not possible to scan queues in small chunks and yield in the
 	 * middle of the dump and come back again. So what we're doing instead
 	 * is to freeze all threads and inspect their queues at once as fast as
@@ -1057,17 +1047,12 @@ static int cli_io_handler_show_tasks(struct appctx *appctx)
  */
 static int cli_io_handler_show_activity(struct appctx *appctx)
 {
-	struct stconn *sc = appctx_sc(appctx);
 	struct show_activity_ctx *actctx = appctx->svcctx;
 	int tgt = actctx->thr; // target thread, -1 for all, 0 for total only
 	uint up_sec, up_usec;
 	int base_line;
 	ullong up;
 
-	/* FIXME: Don't watch the other side ! */
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	/* this macro is used below to dump values. The thread number is "thr",
 	 * and runs from 0 to nbt-1 when values are printed using the formula.
 	 * We normally try to dmup integral lines in order to keep counters
diff --git a/src/cli.c b/src/cli.c
index 7c856bb318..389f19efab 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1267,13 +1267,8 @@ static void cli_release_handler(struct appctx *appctx)
 static int cli_io_handler_show_env(struct appctx *appctx)
 {
 	struct show_env_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	char **var = ctx->var;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	chunk_reset(&trash);
 
 	/* we have two inner loops here, one for the proxy, the other one for
@@ -1303,16 +1298,11 @@ static int cli_io_handler_show_env(struct appctx *appctx)
  */
 static int cli_io_handler_show_fd(struct appctx *appctx)
 {
-	struct stconn *sc = appctx_sc(appctx);
 	struct show_fd_ctx *fdctx = appctx->svcctx;
 	uint match = fdctx->show_mask;
 	int fd = fdctx->fd;
 	int ret = 1;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		goto end;
-
 	chunk_reset(&trash);
 
 	/* isolate the threads once per round. We're limited to a buffer worth
diff --git a/src/debug.c b/src/debug.c
index 077b0377f4..7420c999b9 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -457,13 +457,8 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx)
  */
 static int cli_io_handler_show_threads(struct appctx *appctx)
 {
-	struct stconn *sc = appctx_sc(appctx);
 	int thr;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	if (appctx->st0)
 		thr = appctx->st1;
 	else
@@ -1638,7 +1633,6 @@ static int debug_parse_cli_fd(char **args, char *payload, struct appctx *appctx,
 static int debug_iohandler_fd(struct appctx *appctx)
 {
 	struct dev_fd_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	struct sockaddr_storage sa;
 	struct stat statbuf;
 	socklen_t salen, vlen;
@@ -1647,10 +1641,6 @@ static int debug_iohandler_fd(struct appctx *appctx)
 	int ret = 1;
 	int i, fd;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		goto end;
-
 	chunk_reset(&trash);
 
 	thread_isolate();
@@ -1800,7 +1790,6 @@ static int debug_iohandler_fd(struct appctx *appctx)
 	}
 
 	thread_release();
- end:
 	return ret;
 }
 
@@ -1875,10 +1864,6 @@ static int debug_iohandler_memstats(struct appctx *appctx)
 	const char *pfx = ctx->match;
 	int ret = 1;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		goto end;
-
 	if (!ctx->width) {
 		/* we don't know the first column's width, let's compute it
 		 * now based on a first pass on printable entries and their
diff --git a/src/map.c b/src/map.c
index 2c7157af53..b285a1f51a 100644
--- a/src/map.c
+++ b/src/map.c
@@ -373,22 +373,8 @@ struct show_map_ctx {
 static int cli_io_handler_pat_list(struct appctx *appctx)
 {
 	struct show_map_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	struct pat_ref_elt *elt;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE)) {
-		/* If we're forced to shut down, we might have to remove our
-		 * reference to the last ref_elt being dumped.
-		 */
-		if (!LIST_ISEMPTY(&ctx->bref.users)) {
-			HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock);
-			LIST_DEL_INIT(&ctx->bref.users);
-			HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock);
-		}
-		return 1;
-	}
-
 	switch (ctx->state) {
 	case STATE_INIT:
 		ctx->state = STATE_LIST;
diff --git a/src/mworker.c b/src/mworker.c
index 07d5a99b53..be98246bf4 100644
--- a/src/mworker.c
+++ b/src/mworker.c
@@ -565,17 +565,12 @@ void mworker_cleanup_proc()
 /*  Displays workers and processes  */
 static int cli_io_handler_show_proc(struct appctx *appctx)
 {
-	struct stconn *sc = appctx_sc(appctx);
 	struct mworker_proc *child;
 	int old = 0;
 	int up = date.tv_sec - proc_self->timestamp;
 	char *uptime = NULL;
 	char *reloadtxt = NULL;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	if (up < 0) /* must never be negative because of clock drift */
 		up = 0;
 
@@ -719,15 +714,10 @@ static int cli_parse_reload(char **args, char *payload, struct appctx *appctx, v
 static int cli_io_handler_show_loadstatus(struct appctx *appctx)
 {
 	char *env;
-	struct stconn *sc = appctx_sc(appctx);
 
 	if (!cli_has_level(appctx, ACCESS_LVL_OPER))
 		return 1;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	env = getenv("HAPROXY_LOAD_SUCCESS");
 	if (!env)
 		return 1;
diff --git a/src/proxy.c b/src/proxy.c
index ba15b3b9c0..9adc6a2568 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2735,9 +2735,8 @@ static void dump_server_addr(const struct sockaddr_storage *addr, char *addr_str
  * ->px, the proxy's id ->only_pxid, the server's pointer from ->sv, and the
  * choice of what to dump from ->show_conn.
  */
-static int dump_servers_state(struct stconn *sc)
+static int dump_servers_state(struct appctx *appctx)
 {
-	struct appctx *appctx = __sc_appctx(sc);
 	struct show_srv_ctx *ctx = appctx->svcctx;
 	struct proxy *px = ctx->px;
 	struct server *srv;
@@ -2820,7 +2819,6 @@ static int dump_servers_state(struct stconn *sc)
 static int cli_io_handler_servers_state(struct appctx *appctx)
 {
 	struct show_srv_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	struct proxy *curproxy;
 
 	if (ctx->state == SHOW_SRV_HEAD) {
@@ -2844,7 +2842,7 @@ static int cli_io_handler_servers_state(struct appctx *appctx)
 		curproxy = ctx->px;
 		/* servers are only in backends */
 		if ((curproxy->cap & PR_CAP_BE) && !(curproxy->cap & PR_CAP_INT)) {
-			if (!dump_servers_state(sc))
+			if (!dump_servers_state(appctx))
 				return 0;
 		}
 		/* only the selected proxy is dumped */
@@ -3182,10 +3180,6 @@ static int cli_io_handler_show_errors(struct appctx *appctx)
 	struct stconn *sc = appctx_sc(appctx);
 	extern const char *monthname[12];
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	chunk_reset(&trash);
 
 	if (!ctx->px) {
diff --git a/src/quic_cli.c b/src/quic_cli.c
index b4b9329da2..f0e147c7e7 100644
--- a/src/quic_cli.c
+++ b/src/quic_cli.c
@@ -363,7 +363,6 @@ static void dump_quic_full(struct show_quic_ctx *ctx, struct quic_conn *qc)
 static int cli_io_handler_dump_quic(struct appctx *appctx)
 {
 	struct show_quic_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	struct quic_conn *qc;
 
 	thread_isolate();
@@ -371,16 +370,6 @@ static int cli_io_handler_dump_quic(struct appctx *appctx)
 	if (ctx->thr >= global.nbthread)
 		goto done;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE)) {
-		/* If we're forced to shut down, we might have to remove our
-		 * reference to the last stream being dumped.
-		 */
-		if (!LIST_ISEMPTY(&ctx->bref.users))
-			LIST_DEL_INIT(&ctx->bref.users);
-		goto done;
-	}
-
 	chunk_reset(&trash);
 
 	if (!LIST_ISEMPTY(&ctx->bref.users)) {
diff --git a/src/ring.c b/src/ring.c
index 1b9cd8d8e6..cc0e3a79d8 100644
--- a/src/ring.c
+++ b/src/ring.c
@@ -692,10 +692,6 @@ int cli_io_handler_show_ring(struct appctx *appctx)
 	size_t ofs;
 	int ret;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		return 1;
-
 	MT_LIST_DELETE(&appctx->wait_entry);
 
 	ret = ring_dispatch_messages(ring, appctx, &ctx->ofs, &last_ofs, ctx->flags, applet_append_line);
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index add42b69eb..b69caacb35 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2157,16 +2157,11 @@ void ckch_store_replace(struct ckch_store *old_ckchs, struct ckch_store *new_ckc
 static int cli_io_handler_commit_cert(struct appctx *appctx)
 {
 	struct commit_cert_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	int y = 0;
 	struct ckch_store *old_ckchs, *new_ckchs = NULL;
 	struct ckch_inst *ckchi;
 
 	usermsgs_clr("CLI");
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		goto end;
-
 	while (1) {
 		switch (ctx->state) {
 			case CERT_ST_INIT:
@@ -2843,17 +2838,12 @@ error:
 static int cli_io_handler_commit_cafile_crlfile(struct appctx *appctx)
 {
 	struct commit_cacrlfile_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	int y = 0;
 	struct cafile_entry *old_cafile_entry = ctx->old_entry;
 	struct cafile_entry *new_cafile_entry = ctx->new_entry;
 	struct ckch_inst_link *ckchi_link;
 	char *path;
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		goto end;
-
 	/* The ctx was already validated by the ca-file/crl-file parsing
 	 * function. Entries can only be NULL in CACRL_ST_SUCCESS or
 	 * CACRL_ST_FIN states
diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c
index 4cc1fd88c8..beb3398029 100644
--- a/src/ssl_crtlist.c
+++ b/src/ssl_crtlist.c
@@ -1134,7 +1134,6 @@ static int cli_io_handler_add_crtlist(struct appctx *appctx)
 {
 	struct add_crtlist_ctx *ctx = appctx->svcctx;
 	struct bind_conf_list *bind_conf_node;
-	struct stconn *sc = appctx_sc(appctx);
 	struct crtlist *crtlist = ctx->crtlist;
 	struct crtlist_entry *entry = ctx->entry;
 	struct ckch_store *store = entry->node.key;
@@ -1145,10 +1144,6 @@ static int cli_io_handler_add_crtlist(struct appctx *appctx)
 	/* for each bind_conf which use the crt-list, a new ckch_inst must be
 	 * created.
 	 */
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE))
-		goto end;
-
 	switch (ctx->state) {
 	case ADDCRT_ST_INIT:
 		/* This state just print the update message */
@@ -1575,4 +1570,3 @@ static struct cli_kw_list cli_kws = {{ },{
 };
 
 INITCALL1(STG_REGISTER, cli_register_kw, &cli_kws);
-
diff --git a/src/stick_table.c b/src/stick_table.c
index d1e0de29b2..964542cdea 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -5416,14 +5416,6 @@ static int cli_io_handler_table(struct appctx *appctx)
 	 *   - STATE_DONE : nothing left to dump, the buffer may contain some
 	 *     data though.
 	 */
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE)) {
-		/* in case of abort, remove any refcount we might have set on an entry */
-		if (ctx->state == STATE_DUMP) {
-			stksess_kill_if_expired(ctx->t, ctx->entry, 1);
-		}
-		return 1;
-	}
 
 	chunk_reset(&trash);
 
diff --git a/src/stream.c b/src/stream.c
index 0c8d87cfb5..9d99d2f6b3 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -3494,9 +3494,8 @@ void strm_dump_to_buffer(struct buffer *buf, const struct stream *strm, const ch
  * buffer is full and it needs to be called again, otherwise non-zero. It is
  * designed to be called from stats_dump_strm_to_buffer() below.
  */
-static int stats_dump_full_strm_to_buffer(struct stconn *sc, struct stream *strm)
+static int stats_dump_full_strm_to_buffer(struct appctx *appctx, struct stream *strm)
 {
-	struct appctx *appctx = __sc_appctx(sc);
 	struct show_sess_ctx *ctx = appctx->svcctx;
 
 	chunk_reset(&trash);
@@ -3588,7 +3587,6 @@ static int cli_parse_show_sess(char **args, char *payload, struct appctx *appctx
 static int cli_io_handler_dump_sess(struct appctx *appctx)
 {
 	struct show_sess_ctx *ctx = appctx->svcctx;
-	struct stconn *sc = appctx_sc(appctx);
 	struct connection *conn;
 
 	thread_isolate();
@@ -3598,18 +3596,6 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
 		goto done;
 	}
 
-	/* FIXME: Don't watch the other side !*/
-	if (unlikely(sc_opposite(sc)->flags & SC_FL_SHUT_DONE)) {
-		/* If we're forced to shut down, we might have to remove our
-		 * reference to the last stream being dumped.
-		 */
-		if (!LIST_ISEMPTY(&ctx->bref.users)) {
-			LIST_DELETE(&ctx->bref.users);
-			LIST_INIT(&ctx->bref.users);
-		}
-		goto done;
-	}
-
 	chunk_reset(&trash);
 
 	/* first, let's detach the back-ref from a possible previous stream */
@@ -3666,7 +3652,7 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
 
 			LIST_APPEND(&curr_strm->back_refs, &ctx->bref.users);
 			/* call the proper dump() function and return if we're missing space */
-			if (!stats_dump_full_strm_to_buffer(sc, curr_strm))
+			if (!stats_dump_full_strm_to_buffer(appctx, curr_strm))
 				goto full;
 
 			/* stream dump complete */