From 6e940c259da80fa8aea6dc1070b8b4cdd5847914 Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Thu, 12 Dec 2024 15:00:26 +0100
Subject: [PATCH] CLI: show threads all crash fixed

When socket dropped before finished, it failed to cleanup.
---
 nest/cli.c            | 10 ++++++++--
 nest/cli.h            |  4 +++-
 nest/rt-show.c        |  5 ++++-
 proto/mrt/mrt.c       |  3 ++-
 sysdep/unix/io-loop.c | 13 +++++++++++--
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/nest/cli.c b/nest/cli.c
index 934451b1..3b8e6f46 100644
--- a/nest/cli.c
+++ b/nest/cli.c
@@ -337,12 +337,18 @@ extern cli *cmd_reconfig_stored_cli;
 void
 cli_free(cli *c)
 {
-  CALL(c->cleanup, c);
+  bool done = c->cleanup ? c->cleanup(c) : true;
 
   if (c == cmd_reconfig_stored_cli)
     cmd_reconfig_stored_cli = NULL;
 
-  rp_free(c->pool);
+  if (done)
+    rp_free(c->pool);
+  else
+  {
+    sk_close(c->sock);
+    c->sock = NULL;
+  }
 }
 
 /**
diff --git a/nest/cli.h b/nest/cli.h
index 54fc2964..d86ec380 100644
--- a/nest/cli.h
+++ b/nest/cli.h
@@ -37,7 +37,9 @@ typedef struct cli {
   struct cli_out *tx_buf, *tx_pos, *tx_write;
   event *event;
   void (*cont)(struct cli *c);
-  void (*cleanup)(struct cli *c);	/* The CLI has closed prematurely */
+  bool (*cleanup)(struct cli *c);	/* The CLI has closed prematurely; if done, return true,
+					   otherwise return false and call rp_free(c->pool) later yourself.
+					   The socket is closed anyway, tho. */
   void *rover;				/* Private to continuation routine */
   struct config *main_config;		/* Main config currently in use */
   int last_reply;
diff --git a/nest/rt-show.c b/nest/rt-show.c
index e4149e66..3986da83 100644
--- a/nest/rt-show.c
+++ b/nest/rt-show.c
@@ -225,7 +225,7 @@ rt_show_net(struct rt_show_data *d, const struct rt_export_feed *feed)
   }
 }
 
-static void
+static bool
 rt_show_cleanup(struct cli *c)
 {
   struct rt_show_data *d = c->rover;
@@ -241,6 +241,9 @@ rt_show_cleanup(struct cli *c)
 
   /* Unreference the config */
   OBSREF_CLEAR(d->running_on_config);
+
+  /* Everything cleaned up */
+  return true;
 }
 
 static void
diff --git a/proto/mrt/mrt.c b/proto/mrt/mrt.c
index cc5ad06a..015a1e15 100644
--- a/proto/mrt/mrt.c
+++ b/proto/mrt/mrt.c
@@ -760,11 +760,12 @@ mrt_dump_cont(struct cli *c)
   return mrt_table_dump_step(c->rover);
 }
 
-void
+static bool
 mrt_dump_cleanup(struct cli *c)
 {
   mrt_table_dump_free(c->rover);
   c->rover = NULL;
+  return true;
 }
 
 void
diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c
index 6dca5f3c..f69189e0 100644
--- a/sysdep/unix/io-loop.c
+++ b/sysdep/unix/io-loop.c
@@ -1259,10 +1259,11 @@ bird_thread_show_cli_cont(struct cli *c UNUSED)
   /* Explicitly do nothing to prevent CLI from trying to parse another command. */
 }
 
-static int
+static bool
 bird_thread_show_cli_cleanup(struct cli *c UNUSED)
 {
-  return 1; /* Defer the cleanup until the writeout is finished. */
+  /* Defer the cleanup until the writeout is finished. */
+  return false;
 }
 
 static void
@@ -1328,6 +1329,14 @@ cmd_show_threads_done(struct bird_thread_syncer *sync)
   SKIP_BACK_DECLARE(struct bird_thread_show_data, tsd, sync, sync);
   ASSERT_DIE(birdloop_inside(&main_birdloop));
 
+  /* The client lost their patience and dropped the session early. */
+  if (!tsd->cli->sock)
+  {
+    mb_free(tsd);
+    rp_free(tsd->cli->pool);
+    return;
+  }
+
   tsd->cli->cont = NULL;
   tsd->cli->cleanup = NULL;