From 597b18789ed93b39234ce3ce7b9e03508a58c54b Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 10:34:11 +0200
Subject: [PATCH 1/9] debugger: add cancellation support to Tracer

The tracer is responsible with the communication between the debugger
thread and any worker threads. Up to this point this was not cancellable,
e.g. once a breakpoint fired, the worker stopped with no way of
cancelling this.

With the addition to tracer_cancel() pending breakpoints are let go and
at the same time tracer_wait_for_breakpoint() returns as well with
a gboolean to indicate whether it was cancelled.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/tracer.c | 39 +++++++++++++++++++++++++++++++++++----
 lib/debugger/tracer.h |  4 +++-
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/lib/debugger/tracer.c b/lib/debugger/tracer.c
index 39b6d8b11e..66adf8db1a 100644
--- a/lib/debugger/tracer.c
+++ b/lib/debugger/tracer.c
@@ -23,32 +23,52 @@
  */
 #include "debugger/tracer.h"
 
+/* NOTE: called by workers to stop on a breakpoint, wait for the debugger to
+ * do its stuff and return to continue */
 void
 tracer_stop_on_breakpoint(Tracer *self)
 {
   g_mutex_lock(&self->breakpoint_mutex);
 
+  if (self->cancel_requested)
+    goto exit;
+
   /* send break point */
   self->breakpoint_hit = TRUE;
   g_cond_signal(&self->breakpoint_cond);
 
-  /* wait for resume */
-  while (!self->resume_requested)
+  /* wait for resume or cancel */
+  while (!(self->resume_requested || self->cancel_requested))
     g_cond_wait(&self->resume_cond, &self->breakpoint_mutex);
   self->resume_requested = FALSE;
+
+exit:
   g_mutex_unlock(&self->breakpoint_mutex);
 }
 
-void
+/* NOTE: called by the interactive debugger to wait for a breakpoint to
+ * trigger, a return of FALSE indicates that the tracing was cancelled */
+gboolean
 tracer_wait_for_breakpoint(Tracer *self)
 {
+  gboolean cancelled = FALSE;
   g_mutex_lock(&self->breakpoint_mutex);
-  while (!self->breakpoint_hit)
+  while (!(self->breakpoint_hit || self->cancel_requested))
     g_cond_wait(&self->breakpoint_cond, &self->breakpoint_mutex);
   self->breakpoint_hit = FALSE;
+  if (self->cancel_requested)
+    {
+      cancelled = TRUE;
+
+      /* cancel out threads waiting on breakpoint, e.g.  in the cancelled
+       * case no need to call tracer_resume_after_breakpoint() */
+      g_cond_signal(&self->resume_cond);
+    }
   g_mutex_unlock(&self->breakpoint_mutex);
+  return !cancelled;
 }
 
+/* NOTE: called by the interactive debugger to resume the worker after a breakpoint */
 void
 tracer_resume_after_breakpoint(Tracer *self)
 {
@@ -58,6 +78,17 @@ tracer_resume_after_breakpoint(Tracer *self)
   g_mutex_unlock(&self->breakpoint_mutex);
 }
 
+/* NOTE: called by any thread, not necessarily the debugger thread or worker
+ * threads.  It cancels out the tracer_wait_for_breakpoint() calls */
+void
+tracer_cancel(Tracer *self)
+{
+  g_mutex_lock(&self->breakpoint_mutex);
+  self->cancel_requested = TRUE;
+  g_cond_signal(&self->breakpoint_cond);
+  g_mutex_unlock(&self->breakpoint_mutex);
+}
+
 Tracer *
 tracer_new(GlobalConfig *cfg)
 {
diff --git a/lib/debugger/tracer.h b/lib/debugger/tracer.h
index 2893e277ce..b2b2cc91da 100644
--- a/lib/debugger/tracer.h
+++ b/lib/debugger/tracer.h
@@ -33,11 +33,13 @@ typedef struct _Tracer
   GCond resume_cond;
   gboolean breakpoint_hit;
   gboolean resume_requested;
+  gboolean cancel_requested;
 } Tracer;
 
 void tracer_stop_on_breakpoint(Tracer *self);
-void tracer_wait_for_breakpoint(Tracer *self);
+gboolean tracer_wait_for_breakpoint(Tracer *self);
 void tracer_resume_after_breakpoint(Tracer *self);
+void tracer_cancel(Tracer *self);
 
 Tracer *tracer_new(GlobalConfig *cfg);
 void tracer_free(Tracer *self);

From b50b52538633779f7b0cc8d4dd302f812d2da46e Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 10:36:48 +0200
Subject: [PATCH 2/9] debugger: add debugger_exit()

The new debugger_exit() can be called from any thread, triggers the
cancellation of the tracer and then waits for the debugger thread
to finish.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger.c | 16 ++++++++++++++--
 lib/debugger/debugger.h |  7 +++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c
index a677bc7839..54e89e5697 100644
--- a/lib/debugger/debugger.c
+++ b/lib/debugger/debugger.c
@@ -29,6 +29,7 @@
 #include "mainloop.h"
 #include "timeutils/misc.h"
 #include "compat/time.h"
+#include "scratch-buffers.h"
 
 #include <stdio.h>
 #include <unistd.h>
@@ -44,6 +45,7 @@ struct _Debugger
   LogPipe *current_pipe;
   gboolean drop_current_message;
   struct timespec last_trace_event;
+  GThread *interactive_thread;
 };
 
 static gboolean
@@ -272,6 +274,7 @@ debugger_builtin_fetch_command(void)
 
   printf("(syslog-ng) ");
   fflush(stdout);
+  clearerr(stdin);
 
   if (!fgets(buf, sizeof(buf), stdin))
     return NULL;
@@ -372,11 +375,13 @@ _interactive_console_thread_func(Debugger *self)
   printf("Waiting for breakpoint...\n");
   while (1)
     {
-      tracer_wait_for_breakpoint(self->tracer);
+      if (!tracer_wait_for_breakpoint(self->tracer))
+        break;
 
       _handle_interactive_prompt(self);
       tracer_resume_after_breakpoint(self->tracer);
     }
+  scratch_buffers_explicit_gc();
   app_thread_stop();
   return NULL;
 }
@@ -384,7 +389,7 @@ _interactive_console_thread_func(Debugger *self)
 void
 debugger_start_console(Debugger *self)
 {
-  g_thread_new(NULL, (GThreadFunc) _interactive_console_thread_func, self);
+  self->interactive_thread = g_thread_new(NULL, (GThreadFunc) _interactive_console_thread_func, self);
 }
 
 gboolean
@@ -417,6 +422,13 @@ debugger_perform_tracing(Debugger *self, LogPipe *pipe_, LogMessage *msg)
   return TRUE;
 }
 
+void
+debugger_exit(Debugger *self)
+{
+  tracer_cancel(self->tracer);
+  g_thread_join(self->interactive_thread);
+}
+
 Debugger *
 debugger_new(MainLoop *main_loop, GlobalConfig *cfg)
 {
diff --git a/lib/debugger/debugger.h b/lib/debugger/debugger.h
index 8742651d7f..a1c2b38ac3 100644
--- a/lib/debugger/debugger.h
+++ b/lib/debugger/debugger.h
@@ -32,13 +32,16 @@ typedef struct _Debugger Debugger;
 
 typedef gchar *(*FetchCommandFunc)(void);
 
-Debugger *debugger_new(MainLoop *main_loop, GlobalConfig *cfg);
-void debugger_free(Debugger *self);
 
 gchar *debugger_builtin_fetch_command(void);
 void debugger_register_command_fetcher(FetchCommandFunc fetcher);
+void debugger_exit(Debugger *self);
 void debugger_start_console(Debugger *self);
 gboolean debugger_perform_tracing(Debugger *self, LogPipe *pipe, LogMessage *msg);
 gboolean debugger_stop_at_breakpoint(Debugger *self, LogPipe *pipe, LogMessage *msg);
 
+Debugger *debugger_new(MainLoop *main_loop, GlobalConfig *cfg);
+void debugger_free(Debugger *self);
+
+
 #endif

From 3b64bcdd5b2b109ae2af396fef3db949a756d51f Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 18:15:44 +0200
Subject: [PATCH 3/9] debugger: rename interactive_thread to debugger_thread

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c
index 54e89e5697..4ef5e4d9eb 100644
--- a/lib/debugger/debugger.c
+++ b/lib/debugger/debugger.c
@@ -45,7 +45,7 @@ struct _Debugger
   LogPipe *current_pipe;
   gboolean drop_current_message;
   struct timespec last_trace_event;
-  GThread *interactive_thread;
+  GThread *debugger_thread;
 };
 
 static gboolean
@@ -369,7 +369,7 @@ _handle_interactive_prompt(Debugger *self)
 }
 
 static gpointer
-_interactive_console_thread_func(Debugger *self)
+_debugger_thread_func(Debugger *self)
 {
   app_thread_start();
   printf("Waiting for breakpoint...\n");
@@ -389,7 +389,7 @@ _interactive_console_thread_func(Debugger *self)
 void
 debugger_start_console(Debugger *self)
 {
-  self->interactive_thread = g_thread_new(NULL, (GThreadFunc) _interactive_console_thread_func, self);
+  self->debugger_thread = g_thread_new(NULL, (GThreadFunc) _debugger_thread_func, self);
 }
 
 gboolean
@@ -426,7 +426,7 @@ void
 debugger_exit(Debugger *self)
 {
   tracer_cancel(self->tracer);
-  g_thread_join(self->interactive_thread);
+  g_thread_join(self->debugger_thread);
 }
 
 Debugger *

From a21b3d9dd77e268464f683a738f242938b8deaa5 Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 10:39:43 +0200
Subject: [PATCH 4/9] debugger: attach and deattach the single step hook in a
 synchronized manner

The debugger installed the pipe_single_step_hook without locking, but that
only works if we do that at startup. Since I want to be able to
start the debugger on demand, this needs to sync with any workers that
might be executing.

On x86 it might be safe to just store the pointer, but other less
forgiving architectures (e.g. ARM) may not like that. Let's do this
properly.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger-main.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c
index 088078a0cb..cb0214f5a9 100644
--- a/lib/debugger/debugger-main.c
+++ b/lib/debugger/debugger-main.c
@@ -24,6 +24,8 @@
 
 #include "debugger/debugger.h"
 #include "logpipe.h"
+#include "mainloop-worker.h"
+#include "mainloop-call.h"
 
 static Debugger *current_debugger;
 
@@ -36,12 +38,28 @@ _pipe_hook(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options)
     return debugger_stop_at_breakpoint(current_debugger, s, msg);
 }
 
+
+static void
+_install_hook(gpointer user_data)
+{
+  /* NOTE: this is invoked via main_loop_worker_sync_call(), e.g. all workers are stopped */
+
+  pipe_single_step_hook = _pipe_hook;
+}
+
+static gpointer
+_attach_debugger(gpointer user_data)
+{
+  /* NOTE: this function is always run in the main thread via main_loop_call. */
+  main_loop_worker_sync_call(_install_hook, NULL);
+  return NULL;
+}
 void
 debugger_start(MainLoop *main_loop, GlobalConfig *cfg)
 {
   /* we don't support threaded mode (yet), force it to non-threaded */
   cfg->threaded = FALSE;
   current_debugger = debugger_new(main_loop, cfg);
-  pipe_single_step_hook = _pipe_hook;
   debugger_start_console(current_debugger);
+  main_loop_call(_attach_debugger, NULL, FALSE);
 }

From dd6ba5a288c43c0b947194c165a999f0c31edfb6 Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 10:40:46 +0200
Subject: [PATCH 5/9] debugger: add debugger_stop() call alongside
 debugger_start()

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger-main.c | 28 ++++++++++++++++++++++++++++
 lib/debugger/debugger-main.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c
index cb0214f5a9..c9af10701f 100644
--- a/lib/debugger/debugger-main.c
+++ b/lib/debugger/debugger-main.c
@@ -54,6 +54,28 @@ _attach_debugger(gpointer user_data)
   main_loop_worker_sync_call(_install_hook, NULL);
   return NULL;
 }
+
+static void
+_remove_hook_and_clean_up_the_debugger(gpointer user_data)
+{
+  /* NOTE: this is invoked via main_loop_worker_sync_call(), e.g. all workers are stopped */
+
+  pipe_single_step_hook = NULL;
+
+  Debugger *d = current_debugger;
+  current_debugger = NULL;
+
+  debugger_exit(d);
+  debugger_free(d);
+}
+
+static gpointer
+_detach_debugger(gpointer user_data)
+{
+  main_loop_worker_sync_call(_remove_hook_and_clean_up_the_debugger, NULL);
+  return NULL;
+}
+
 void
 debugger_start(MainLoop *main_loop, GlobalConfig *cfg)
 {
@@ -63,3 +85,9 @@ debugger_start(MainLoop *main_loop, GlobalConfig *cfg)
   debugger_start_console(current_debugger);
   main_loop_call(_attach_debugger, NULL, FALSE);
 }
+
+void
+debugger_stop(void)
+{
+  main_loop_call(_detach_debugger, NULL, FALSE);
+}
diff --git a/lib/debugger/debugger-main.h b/lib/debugger/debugger-main.h
index cd03143213..0a38738227 100644
--- a/lib/debugger/debugger-main.h
+++ b/lib/debugger/debugger-main.h
@@ -29,5 +29,6 @@
 #include "cfg.h"
 
 void debugger_start(MainLoop *main_loop, GlobalConfig *cfg);
+void debugger_stop(void);
 
 #endif

From 2c138cfe4f3a49db30e6776bb2a05eaf68ab09b5 Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 18:15:08 +0200
Subject: [PATCH 6/9] debugger: make sure that debugger_start_console and
 debugger_exit() run from the main thread

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger-main.c | 5 +++--
 lib/debugger/debugger.c      | 4 ++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c
index c9af10701f..27ebf89120 100644
--- a/lib/debugger/debugger-main.c
+++ b/lib/debugger/debugger-main.c
@@ -52,6 +52,8 @@ _attach_debugger(gpointer user_data)
 {
   /* NOTE: this function is always run in the main thread via main_loop_call. */
   main_loop_worker_sync_call(_install_hook, NULL);
+
+  debugger_start_console(current_debugger);
   return NULL;
 }
 
@@ -82,8 +84,7 @@ debugger_start(MainLoop *main_loop, GlobalConfig *cfg)
   /* we don't support threaded mode (yet), force it to non-threaded */
   cfg->threaded = FALSE;
   current_debugger = debugger_new(main_loop, cfg);
-  debugger_start_console(current_debugger);
-  main_loop_call(_attach_debugger, NULL, FALSE);
+  main_loop_call(_attach_debugger, current_debugger, FALSE);
 }
 
 void
diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c
index 4ef5e4d9eb..73b410ac15 100644
--- a/lib/debugger/debugger.c
+++ b/lib/debugger/debugger.c
@@ -389,6 +389,8 @@ _debugger_thread_func(Debugger *self)
 void
 debugger_start_console(Debugger *self)
 {
+  main_loop_assert_main_thread();
+
   self->debugger_thread = g_thread_new(NULL, (GThreadFunc) _debugger_thread_func, self);
 }
 
@@ -425,6 +427,8 @@ debugger_perform_tracing(Debugger *self, LogPipe *pipe_, LogMessage *msg)
 void
 debugger_exit(Debugger *self)
 {
+  main_loop_assert_main_thread();
+
   tracer_cancel(self->tracer);
   g_thread_join(self->debugger_thread);
 }

From 574d7a989e5e942e82a676b0ad3846da403dcca4 Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 13:58:47 +0200
Subject: [PATCH 7/9] tracer: move the Tracer struct to the implementation file

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/tracer.c | 10 ++++++++++
 lib/debugger/tracer.h | 10 +---------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/lib/debugger/tracer.c b/lib/debugger/tracer.c
index 66adf8db1a..5072d39d4b 100644
--- a/lib/debugger/tracer.c
+++ b/lib/debugger/tracer.c
@@ -23,6 +23,16 @@
  */
 #include "debugger/tracer.h"
 
+struct _Tracer
+{
+  GMutex breakpoint_mutex;
+  GCond breakpoint_cond;
+  GCond resume_cond;
+  gboolean breakpoint_hit;
+  gboolean resume_requested;
+  gboolean cancel_requested;
+};
+
 /* NOTE: called by workers to stop on a breakpoint, wait for the debugger to
  * do its stuff and return to continue */
 void
diff --git a/lib/debugger/tracer.h b/lib/debugger/tracer.h
index b2b2cc91da..618d4fc169 100644
--- a/lib/debugger/tracer.h
+++ b/lib/debugger/tracer.h
@@ -26,15 +26,7 @@
 
 #include "syslog-ng.h"
 
-typedef struct _Tracer
-{
-  GMutex breakpoint_mutex;
-  GCond breakpoint_cond;
-  GCond resume_cond;
-  gboolean breakpoint_hit;
-  gboolean resume_requested;
-  gboolean cancel_requested;
-} Tracer;
+typedef struct _Tracer Tracer;
 
 void tracer_stop_on_breakpoint(Tracer *self);
 gboolean tracer_wait_for_breakpoint(Tracer *self);

From 7fa251eeb0ec723668d1fd4f183352a55c5f0abb Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 10:41:45 +0200
Subject: [PATCH 8/9] debugger: support multi-threaded mode

The key ingredient to support multi-threaded debugger session is to allow
the breakpoint sites to submit debugger_stop_on_breakpoint() calls
in parallel so that these requests are queued and no variables are
used for the different invocations.

The solution is to introduce the BreakpointSite struct, this is where
we store state that relates to each distinct breakpoint events.

This struct contains both the state the debugger may want to inspect (e.g.
msg, pipe) and also  the state related to inter-thread communication
(e.g. resume_requested).

Each such site instances are queued in an internal queue which is then
processed by the interactive debugger.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger-main.c |  2 --
 lib/debugger/debugger.c      | 44 ++++++++++++++++-----------------
 lib/debugger/tracer.c        | 47 ++++++++++++++++++++++++------------
 lib/debugger/tracer.h        | 16 +++++++++---
 4 files changed, 66 insertions(+), 43 deletions(-)

diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c
index 27ebf89120..52660609fb 100644
--- a/lib/debugger/debugger-main.c
+++ b/lib/debugger/debugger-main.c
@@ -81,8 +81,6 @@ _detach_debugger(gpointer user_data)
 void
 debugger_start(MainLoop *main_loop, GlobalConfig *cfg)
 {
-  /* we don't support threaded mode (yet), force it to non-threaded */
-  cfg->threaded = FALSE;
   current_debugger = debugger_new(main_loop, cfg);
   main_loop_call(_attach_debugger, current_debugger, FALSE);
 }
diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c
index 73b410ac15..89e6781224 100644
--- a/lib/debugger/debugger.c
+++ b/lib/debugger/debugger.c
@@ -41,9 +41,7 @@ struct _Debugger
   GlobalConfig *cfg;
   gchar *command_buffer;
   LogTemplate *display_template;
-  LogMessage *current_msg;
-  LogPipe *current_pipe;
-  gboolean drop_current_message;
+  BreakpointSite *breakpoint_site;
   struct timespec last_trace_event;
   GThread *debugger_thread;
 };
@@ -162,11 +160,11 @@ static gboolean
 _cmd_print(Debugger *self, gint argc, gchar *argv[])
 {
   if (argc == 1)
-    _display_msg_details(self, self->current_msg);
+    _display_msg_details(self, self->breakpoint_site->msg);
   else if (argc == 2)
     {
       GError *error = NULL;
-      if (!_display_msg_with_template_string(self, self->current_msg, argv[1], &error))
+      if (!_display_msg_with_template_string(self, self->breakpoint_site->msg, argv[1], &error))
         {
           printf("print: %s\n", error->message);
           g_clear_error(&error);
@@ -197,14 +195,14 @@ _cmd_display(Debugger *self, gint argc, gchar *argv[])
 static gboolean
 _cmd_drop(Debugger *self, gint argc, gchar *argv[])
 {
-  self->drop_current_message = TRUE;
+  self->breakpoint_site->drop = TRUE;
   return FALSE;
 }
 
 static gboolean
 _cmd_trace(Debugger *self, gint argc, gchar *argv[])
 {
-  self->current_msg->flags |= LF_STATE_TRACING;
+  self->breakpoint_site->msg->flags |= LF_STATE_TRACING;
   return FALSE;
 }
 
@@ -212,7 +210,7 @@ static gboolean
 _cmd_quit(Debugger *self, gint argc, gchar *argv[])
 {
   main_loop_exit(self->main_loop);
-  self->drop_current_message = TRUE;
+  self->breakpoint_site->drop = TRUE;
   return FALSE;
 }
 
@@ -233,7 +231,7 @@ _cmd_info(Debugger *self, gint argc, gchar *argv[])
   if (argc >= 2)
     {
       if (strcmp(argv[1], "pipe") == 0)
-        return _cmd_info_pipe(self, self->current_pipe);
+        return _cmd_info_pipe(self, self->breakpoint_site->pipe);
     }
 
   printf("info: List of info subcommands\n"
@@ -352,11 +350,11 @@ static void
 _handle_interactive_prompt(Debugger *self)
 {
   gchar buf[1024];
-  LogPipe *current_pipe = self->current_pipe;
+  LogPipe *current_pipe = self->breakpoint_site->pipe;
 
   printf("Breakpoint hit %s\n", log_expr_node_format_location(current_pipe->expr_node, buf, sizeof(buf)));
   _display_source_line(current_pipe->expr_node);
-  _display_msg_with_template(self, self->current_msg, self->display_template);
+  _display_msg_with_template(self, self->breakpoint_site->msg, self->display_template);
   while (1)
     {
       _fetch_command(self);
@@ -375,11 +373,12 @@ _debugger_thread_func(Debugger *self)
   printf("Waiting for breakpoint...\n");
   while (1)
     {
-      if (!tracer_wait_for_breakpoint(self->tracer))
+      self->breakpoint_site = NULL;
+      if (!tracer_wait_for_breakpoint(self->tracer, &self->breakpoint_site))
         break;
 
       _handle_interactive_prompt(self);
-      tracer_resume_after_breakpoint(self->tracer);
+      tracer_resume_after_breakpoint(self->tracer, self->breakpoint_site);
     }
   scratch_buffers_explicit_gc();
   app_thread_stop();
@@ -397,15 +396,16 @@ debugger_start_console(Debugger *self)
 gboolean
 debugger_stop_at_breakpoint(Debugger *self, LogPipe *pipe_, LogMessage *msg)
 {
-  self->drop_current_message = FALSE;
-  self->current_msg = log_msg_ref(msg);
-  self->current_pipe = log_pipe_ref(pipe_);
-  tracer_stop_on_breakpoint(self->tracer);
-  log_msg_unref(self->current_msg);
-  log_pipe_unref(self->current_pipe);
-  self->current_msg = NULL;
-  self->current_pipe = NULL;
-  return !self->drop_current_message;
+  BreakpointSite breakpoint_site = {0};
+  msg_trace("Debugger: stopping at breakpoint",
+            log_pipe_location_tag(pipe_));
+
+  breakpoint_site.msg = log_msg_ref(msg);
+  breakpoint_site.pipe = log_pipe_ref(pipe_);
+  tracer_stop_on_breakpoint(self->tracer, &breakpoint_site);
+  log_msg_unref(breakpoint_site.msg);
+  log_pipe_unref(breakpoint_site.pipe);
+  return !breakpoint_site.drop;
 }
 
 gboolean
diff --git a/lib/debugger/tracer.c b/lib/debugger/tracer.c
index 5072d39d4b..5ad8e1c748 100644
--- a/lib/debugger/tracer.c
+++ b/lib/debugger/tracer.c
@@ -25,32 +25,38 @@
 
 struct _Tracer
 {
+  GQueue *waiting_breakpoints;
   GMutex breakpoint_mutex;
   GCond breakpoint_cond;
+
+  /* this condition variable is shared between all breakpoints, but each of
+   * them has its own "resume_requested" variable, this means that all
+   * resumes will wake up all waiting breakpoints, but only one of them will
+   * actually resume, it's not the most efficient implementation, but avoids
+   * us having to free the GCond instance as a thread is terminated. */
+
   GCond resume_cond;
-  gboolean breakpoint_hit;
-  gboolean resume_requested;
+  BreakpointSite *pending_breakpoint;
   gboolean cancel_requested;
 };
 
 /* NOTE: called by workers to stop on a breakpoint, wait for the debugger to
  * do its stuff and return to continue */
 void
-tracer_stop_on_breakpoint(Tracer *self)
+tracer_stop_on_breakpoint(Tracer *self, BreakpointSite *breakpoint_site)
 {
   g_mutex_lock(&self->breakpoint_mutex);
 
   if (self->cancel_requested)
     goto exit;
-
+  breakpoint_site->resume_requested = FALSE;
   /* send break point */
-  self->breakpoint_hit = TRUE;
+  g_queue_push_tail(self->waiting_breakpoints, breakpoint_site);
   g_cond_signal(&self->breakpoint_cond);
 
   /* wait for resume or cancel */
-  while (!(self->resume_requested || self->cancel_requested))
+  while (!(breakpoint_site->resume_requested || self->cancel_requested))
     g_cond_wait(&self->resume_cond, &self->breakpoint_mutex);
-  self->resume_requested = FALSE;
 
 exit:
   g_mutex_unlock(&self->breakpoint_mutex);
@@ -59,20 +65,26 @@ tracer_stop_on_breakpoint(Tracer *self)
 /* NOTE: called by the interactive debugger to wait for a breakpoint to
  * trigger, a return of FALSE indicates that the tracing was cancelled */
 gboolean
-tracer_wait_for_breakpoint(Tracer *self)
+tracer_wait_for_breakpoint(Tracer *self, BreakpointSite **breakpoint_site)
 {
   gboolean cancelled = FALSE;
   g_mutex_lock(&self->breakpoint_mutex);
-  while (!(self->breakpoint_hit || self->cancel_requested))
+  while (g_queue_is_empty(self->waiting_breakpoints) && !self->cancel_requested)
     g_cond_wait(&self->breakpoint_cond, &self->breakpoint_mutex);
-  self->breakpoint_hit = FALSE;
-  if (self->cancel_requested)
+
+  g_assert(self->pending_breakpoint == NULL);
+  *breakpoint_site = NULL;
+  if (!self->cancel_requested)
+    {
+      *breakpoint_site = self->pending_breakpoint = g_queue_pop_head(self->waiting_breakpoints);
+    }
+  else
     {
       cancelled = TRUE;
 
       /* cancel out threads waiting on breakpoint, e.g.  in the cancelled
        * case no need to call tracer_resume_after_breakpoint() */
-      g_cond_signal(&self->resume_cond);
+      g_cond_broadcast(&self->resume_cond);
     }
   g_mutex_unlock(&self->breakpoint_mutex);
   return !cancelled;
@@ -80,11 +92,13 @@ tracer_wait_for_breakpoint(Tracer *self)
 
 /* NOTE: called by the interactive debugger to resume the worker after a breakpoint */
 void
-tracer_resume_after_breakpoint(Tracer *self)
+tracer_resume_after_breakpoint(Tracer *self, BreakpointSite *breakpoint_site)
 {
   g_mutex_lock(&self->breakpoint_mutex);
-  self->resume_requested = TRUE;
-  g_cond_signal(&self->resume_cond);
+  g_assert(self->pending_breakpoint == breakpoint_site);
+  self->pending_breakpoint->resume_requested = TRUE;
+  self->pending_breakpoint = NULL;
+  g_cond_broadcast(&self->resume_cond);
   g_mutex_unlock(&self->breakpoint_mutex);
 }
 
@@ -107,7 +121,7 @@ tracer_new(GlobalConfig *cfg)
   g_mutex_init(&self->breakpoint_mutex);
   g_cond_init(&self->breakpoint_cond);
   g_cond_init(&self->resume_cond);
-
+  self->waiting_breakpoints = g_queue_new();
   return self;
 }
 
@@ -117,5 +131,6 @@ tracer_free(Tracer *self)
   g_mutex_clear(&self->breakpoint_mutex);
   g_cond_clear(&self->breakpoint_cond);
   g_cond_clear(&self->resume_cond);
+  g_queue_free(self->waiting_breakpoints);
   g_free(self);
 }
diff --git a/lib/debugger/tracer.h b/lib/debugger/tracer.h
index 618d4fc169..de2a8a9da5 100644
--- a/lib/debugger/tracer.h
+++ b/lib/debugger/tracer.h
@@ -28,9 +28,19 @@
 
 typedef struct _Tracer Tracer;
 
-void tracer_stop_on_breakpoint(Tracer *self);
-gboolean tracer_wait_for_breakpoint(Tracer *self);
-void tracer_resume_after_breakpoint(Tracer *self);
+/* struct to track the invocation of a breakpoint, we have an instance for each thread */
+typedef struct _BreakpointSite
+{
+  gboolean resume_requested;
+  LogMessage *msg;
+  LogPipe *pipe;
+  gboolean drop;
+} BreakpointSite;
+
+
+void tracer_stop_on_breakpoint(Tracer *self, BreakpointSite *breakpoint_site);
+gboolean tracer_wait_for_breakpoint(Tracer *self, BreakpointSite **breakpoint_site);
+void tracer_resume_after_breakpoint(Tracer *self, BreakpointSite *breakpoint_site);
 void tracer_cancel(Tracer *self);
 
 Tracer *tracer_new(GlobalConfig *cfg);

From 6e935685fcda18d5b41e48d8863ac5e445b98a3e Mon Sep 17 00:00:00 2001
From: Balazs Scheidler <balazs.scheidler@axoflow.com>
Date: Sun, 6 Oct 2024 10:31:09 +0200
Subject: [PATCH 9/9] syslog-ng-ctl: add support for "attach debugger"

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
---
 lib/debugger/debugger-main.c    |  5 +++++
 lib/debugger/debugger-main.h    |  1 +
 lib/mainloop-control.c          | 19 +++++++++++++++++++
 syslog-ng-ctl/commands/attach.c |  4 +++-
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c
index 52660609fb..7f2dc932d4 100644
--- a/lib/debugger/debugger-main.c
+++ b/lib/debugger/debugger-main.c
@@ -38,6 +38,11 @@ _pipe_hook(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options)
     return debugger_stop_at_breakpoint(current_debugger, s, msg);
 }
 
+gboolean
+debugger_is_running(void)
+{
+  return current_debugger != NULL;
+}
 
 static void
 _install_hook(gpointer user_data)
diff --git a/lib/debugger/debugger-main.h b/lib/debugger/debugger-main.h
index 0a38738227..09862bd970 100644
--- a/lib/debugger/debugger-main.h
+++ b/lib/debugger/debugger-main.h
@@ -28,6 +28,7 @@
 #include "debugger/debugger.h"
 #include "cfg.h"
 
+gboolean debugger_is_running(void);
 void debugger_start(MainLoop *main_loop, GlobalConfig *cfg);
 void debugger_stop(void);
 
diff --git a/lib/mainloop-control.c b/lib/mainloop-control.c
index 2781cc38c6..6f33f81bb7 100644
--- a/lib/mainloop-control.c
+++ b/lib/mainloop-control.c
@@ -32,6 +32,7 @@
 #include "cfg-walker.h"
 #include "logpipe.h"
 #include "console.h"
+#include "debugger/debugger-main.h"
 
 #include <string.h>
 
@@ -121,9 +122,12 @@ _wait_until_peer_disappears(ControlConnection *cc, gint max_seconds, gboolean *c
 static void
 control_connection_attach(ControlConnection *cc, GString *command, gpointer user_data, gboolean *cancelled)
 {
+  MainLoop *main_loop = (MainLoop *) user_data;
   gchar **cmds = g_strsplit(command->str, " ", 4);
+
   GString *result = g_string_sized_new(128);
   gint n_seconds = -1;
+  gboolean start_debugger = FALSE;
   struct
   {
     gboolean log_stderr;
@@ -155,6 +159,10 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user
           goto exit;
         }
     }
+  else if (g_str_equal(cmds[1], "DEBUGGER"))
+    {
+      start_debugger = TRUE;
+    }
   else
     {
       g_string_assign(result, "FAIL This version of syslog-ng only supports attaching to STDIO or LOGS");
@@ -183,8 +191,19 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user
   log_stderr = new_values.log_stderr;
   msg_set_log_level(new_values.log_level);
 
+  if (start_debugger && !debugger_is_running())
+    {
+      //cfg_load_module(self->current_configuration, "mod-python");
+      debugger_start(main_loop, main_loop_get_current_config(main_loop));
+    }
+
   _wait_until_peer_disappears(cc, n_seconds, cancelled);
 
+  if (start_debugger && debugger_is_running())
+    {
+      debugger_stop();
+    }
+
   log_stderr = old_values.log_stderr;
   msg_set_log_level(old_values.log_level);
 
diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c
index f1e1f4b655..2bd18ac246 100644
--- a/syslog-ng-ctl/commands/attach.c
+++ b/syslog-ng-ctl/commands/attach.c
@@ -47,7 +47,7 @@ GOptionEntry attach_options[] =
 {
   { "seconds", 0, 0, G_OPTION_ARG_INT, &attach_options_seconds, "amount of time to attach for", NULL },
   { "log-level", 0, 0, G_OPTION_ARG_CALLBACK, _store_log_level, "change syslog-ng log level", "<default|verbose|debug|trace>" },
-  { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &attach_commands, "attach mode: logs, stdio", NULL },
+  { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &attach_commands, "attach mode: logs, debugger, stdio", NULL },
   { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL }
 };
 
@@ -73,6 +73,8 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx)
     g_string_append(command, " STDIO");
   else if (g_str_equal(attach_mode, "logs"))
     g_string_append(command, " LOGS");
+  else if (g_str_equal(attach_mode, "debugger"))
+    g_string_append(command, " DEBUGGER");
   else
     {
       fprintf(stderr, "Unknown attach mode\n");