From 1eb3781e5496a2149d64ba1edbca6f5c4f10d1e3 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:30:55 +0200 Subject: [PATCH 01/42] scratch-buffers: do not use ivykis in non-ivykis threads scratch_buffers_lazy_update_stats() uses the ivykis time state to check if it is time to update the stats about scratch buffers. Do not do that if ivykis is not initialized. This may happen in control threads and the debugger threads that do use scratch buffers but don't use ivykis. Signed-off-by: Balazs Scheidler --- lib/scratch-buffers.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/scratch-buffers.c b/lib/scratch-buffers.c index 494d2a73ce..6c5b6fa520 100644 --- a/lib/scratch-buffers.c +++ b/lib/scratch-buffers.c @@ -245,10 +245,17 @@ _thread_maintenance_update_time(void) void scratch_buffers_lazy_update_stats(void) { - if (_thread_maintenance_period_elapsed()) + if (iv_inited()) + { + if (_thread_maintenance_period_elapsed()) + { + scratch_buffers_update_stats(); + _thread_maintenance_update_time(); + } + } + else { scratch_buffers_update_stats(); - _thread_maintenance_update_time(); } } From d14cb21bd77f0566620cfb95bd4cccc82ab7a2be Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 17:21:25 +0200 Subject: [PATCH 02/42] control: make it possible to query if a worker relates to a connection A better solution would be to have a connection specific worker list, and a list of connections. But for now this suffices for my purposes of being able to cancel connection specific workers. Signed-off-by: Balazs Scheidler --- lib/control/control-command-thread.c | 6 ++++++ lib/control/control-command-thread.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/lib/control/control-command-thread.c b/lib/control/control-command-thread.c index 9d5ed8b976..f66ecf6220 100644 --- a/lib/control/control-command-thread.c +++ b/lib/control/control-command-thread.c @@ -44,6 +44,12 @@ struct _ControlCommandThread struct iv_event thread_finished; }; +gboolean +control_command_thread_relates_to_connection(ControlCommandThread *self, ControlConnection *cc) +{ + return self->connection == cc; +} + static void _on_thread_finished(gpointer user_data) { diff --git a/lib/control/control-command-thread.h b/lib/control/control-command-thread.h index 609247d320..cf8787694e 100644 --- a/lib/control/control-command-thread.h +++ b/lib/control/control-command-thread.h @@ -27,6 +27,8 @@ #include "control.h" +gboolean control_command_thread_relates_to_connection(ControlCommandThread *self, ControlConnection *cc); + void control_command_thread_run(ControlCommandThread *self); void control_command_thread_cancel(ControlCommandThread *self); const gchar *control_command_thread_get_command(ControlCommandThread *self); From 273eb9b61d40a14abc5b33a6f30f1d18d3a4a533 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 17:19:30 +0200 Subject: [PATCH 03/42] control: cancel connection related workers Up to now, control worker threads were only cancelled at exit. Truth be told we never really detected if the peer has disconnected either. This patch implements thread cancellation whenever a connection closes, to detect the closure of the connection comes in a separate patch. Signed-off-by: Balazs Scheidler --- lib/control/control-server.c | 23 ++++++++++++++++++++--- lib/control/control-server.h | 2 +- lib/mainloop.c | 2 +- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/control/control-server.c b/lib/control/control-server.c index 66a5c57867..18d1c04953 100644 --- a/lib/control/control-server.c +++ b/lib/control/control-server.c @@ -31,6 +31,16 @@ void _cancel_worker(gpointer data, gpointer user_data) { ControlCommandThread *thread = (ControlCommandThread *) data; + ControlConnection *cc = (ControlConnection *) user_data; + + if (cc && !control_command_thread_relates_to_connection(thread, cc)) + { + /* check if we relate to a specific connection and cancel only those. + * This is only used when a connection closed while the thread is + * still running. + */ + return; + } msg_warning("Requesting the cancellation of control command thread", evt_tag_str("control_command", control_command_thread_get_command(thread))); @@ -52,17 +62,23 @@ _cancel_worker(gpointer data, gpointer user_data) */ } -void -control_server_cancel_workers(ControlServer *self) +static void +control_server_cancel_workers(ControlServer *self, ControlConnection *cc) { if (self->worker_threads) { msg_debug("Cancelling control server worker threads"); - g_list_foreach(self->worker_threads, _cancel_worker, NULL); + g_list_foreach(self->worker_threads, _cancel_worker, cc); msg_debug("Control server worker threads have been cancelled"); } } +void +control_server_cancel_all_workers(ControlServer *self) +{ + control_server_cancel_workers(self, NULL); +} + void control_server_worker_started(ControlServer *self, ControlCommandThread *worker) { @@ -80,6 +96,7 @@ control_server_worker_finished(ControlServer *self, ControlCommandThread *worker void control_server_connection_closed(ControlServer *self, ControlConnection *cc) { + control_server_cancel_workers(self, cc); control_connection_stop_watches(cc); control_connection_unref(cc); } diff --git a/lib/control/control-server.h b/lib/control/control-server.h index 5c85e8c7f1..5d77c74279 100644 --- a/lib/control/control-server.h +++ b/lib/control/control-server.h @@ -38,7 +38,7 @@ struct _ControlServer void (*free_fn)(ControlServer *self); }; -void control_server_cancel_workers(ControlServer *self); +void control_server_cancel_all_workers(ControlServer *self); void control_server_connection_closed(ControlServer *self, ControlConnection *cc); void control_server_worker_started(ControlServer *self, ControlCommandThread *worker); void control_server_worker_finished(ControlServer *self, ControlCommandThread *worker); diff --git a/lib/mainloop.c b/lib/mainloop.c index 405625ff39..e126bd3395 100644 --- a/lib/mainloop.c +++ b/lib/mainloop.c @@ -463,7 +463,7 @@ main_loop_exit_initiate(gpointer user_data) if (main_loop_is_terminating(self)) return; - control_server_cancel_workers(self->control_server); + control_server_cancel_all_workers(self->control_server); app_pre_shutdown(); From b4020dd7f26a05afb831b25ed7818725e82567cc Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:28:48 +0200 Subject: [PATCH 04/42] control: call app_thread_start/stop from command threads Signed-off-by: Balazs Scheidler --- lib/control/control-command-thread.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/control/control-command-thread.c b/lib/control/control-command-thread.c index f66ecf6220..f97605f5f9 100644 --- a/lib/control/control-command-thread.c +++ b/lib/control/control-command-thread.c @@ -27,6 +27,7 @@ #include "messages.h" #include "secret-storage/secret-storage.h" #include "scratch-buffers.h" +#include "apphook.h" #include struct _ControlCommandThread @@ -70,7 +71,7 @@ _thread(gpointer user_data) ControlCommandThread *self = (ControlCommandThread *) user_data; iv_init(); - scratch_buffers_allocator_init(); + app_thread_start(); msg_debug("Control command thread has started", evt_tag_str("control_command", self->command->str)); @@ -88,8 +89,8 @@ _thread(gpointer user_data) evt_tag_str("control_command", self->command->str)); scratch_buffers_explicit_gc(); - scratch_buffers_allocator_deinit(); control_command_thread_unref(self); + app_thread_stop(); iv_deinit(); } From fe854f703149532ba3e892c0c6bc60db7a168809 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 14:23:04 +0200 Subject: [PATCH 05/42] control: add support for passing 3 fds through the control socket This will be used to pass over the stdio file descriptors from syslog-ng-ctl so we can attach to the syslog-ng process after it was started in the background. Signed-off-by: Balazs Scheidler --- lib/control/control-connection.c | 8 ++++ lib/control/control-connection.h | 2 + lib/control/control-server-unix.c | 74 ++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/lib/control/control-connection.c b/lib/control/control-connection.c index 03819103cf..2f19dbb570 100644 --- a/lib/control/control-connection.c +++ b/lib/control/control-connection.c @@ -36,6 +36,14 @@ _g_string_destroy(gpointer user_data) g_string_free(str, TRUE); } +gboolean +control_connection_get_attached_fds(ControlConnection *self, gint *fds, gsize *num_fds) +{ + if (self->get_attached_fds) + return self->get_attached_fds(self, fds, num_fds); + return FALSE; +} + static void _control_connection_free(ControlConnection *self) { diff --git a/lib/control/control-connection.h b/lib/control/control-connection.h index 7d61fa8400..5ad25dd75d 100644 --- a/lib/control/control-connection.h +++ b/lib/control/control-connection.h @@ -41,6 +41,7 @@ struct _ControlConnection GString *output_buffer; gsize pos; ControlServer *server; + gboolean (*get_attached_fds)(ControlConnection *self, gint *fds, gsize *num_fds); gboolean (*run_command)(ControlConnection *self, ControlCommand *command_desc, GString *command_string); int (*read)(ControlConnection *self, gpointer buffer, gsize size); int (*write)(ControlConnection *self, gpointer buffer, gsize size); @@ -56,6 +57,7 @@ struct _ControlConnection }; +gboolean control_connection_get_attached_fds(ControlConnection *self, gint *fds, gsize *num_fds); gboolean control_connection_run_command(ControlConnection *self, GString *command_string); void control_connection_send_reply(ControlConnection *self, GString *reply); void control_connection_send_batched_reply(ControlConnection *self, GString *reply); diff --git a/lib/control/control-server-unix.c b/lib/control/control-server-unix.c index 726453f720..4b9db5c1d5 100644 --- a/lib/control/control-server-unix.c +++ b/lib/control/control-server-unix.c @@ -42,8 +42,21 @@ typedef struct _ControlConnectionUnix ControlConnection super; struct iv_fd control_io; gint fd; + /* stdin, stdout, stderr as passed by syslog-ng-ctl */ + gint attached_fds[3]; } ControlConnectionUnix; +static gboolean +control_connection_unix_get_attached_fds(ControlConnection *s, gint *fds, gsize *num_fds) +{ + ControlConnectionUnix *self = (ControlConnectionUnix *)s; + + g_assert(*num_fds >= 3); + memcpy(fds, self->attached_fds, sizeof(self->attached_fds)); + *num_fds = 3; + return TRUE; +} + gint control_connection_unix_write(ControlConnection *s, gpointer buffer, gsize size) { @@ -51,11 +64,58 @@ control_connection_unix_write(ControlConnection *s, gpointer buffer, gsize size) return write(self->control_io.fd, buffer, size); } +static gint +_extract_ancillary_data(ControlConnectionUnix *self, gint rc, struct msghdr *msg) +{ + if (G_UNLIKELY(msg->msg_flags & MSG_CTRUNC)) + { + msg_warning_once("WARNING: recvmsg() on control socket returned truncated control data", + evt_tag_int("control_len", msg->msg_controllen)); + return -1; + } + + for (struct cmsghdr *cmsg = CMSG_FIRSTHDR(msg); cmsg != NULL; cmsg = CMSG_NXTHDR(msg, cmsg)) + { + if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) + { + gint header_len = CMSG_DATA(cmsg) - (unsigned char *) cmsg; + gint fd_array_size = (cmsg->cmsg_len - header_len); + + if (fd_array_size != sizeof(self->attached_fds)) + { + msg_warning_once("WARNING: invalid number of fds received on control socket", + evt_tag_int("fd_array_size", fd_array_size)); + return -1; + } + memcpy(&self->attached_fds, CMSG_DATA(cmsg), sizeof(self->attached_fds)); + break; + } + } + + return rc; +} + gint control_connection_unix_read(ControlConnection *s, gpointer buffer, gsize size) { ControlConnectionUnix *self = (ControlConnectionUnix *)s; - return read(self->control_io.fd, buffer, size); + gchar cmsg_buf[256]; + struct iovec iov[1] = + { + { .iov_base = buffer, .iov_len = size }, + }; + struct msghdr msg = + { + .msg_iov = iov, + .msg_iovlen = G_N_ELEMENTS(iov), + .msg_control = cmsg_buf, + .msg_controllen = sizeof(cmsg_buf), + }; + gint rc = recvmsg(self->control_io.fd, &msg, 0); + if (rc < 0) + return rc; + + return _extract_ancillary_data(self, rc, &msg); } static void @@ -114,6 +174,11 @@ control_connection_unix_free(ControlConnection *s) { ControlConnectionUnix *self = (ControlConnectionUnix *)s; close(self->control_io.fd); + for (gint i = 0; i < G_N_ELEMENTS(self->attached_fds); i++) + { + if (self->attached_fds[i] >= 0) + close(self->attached_fds[i]); + } } ControlConnection * @@ -129,6 +194,12 @@ control_connection_unix_new(ControlServer *server, gint sock) self->super.events.start_watches = control_connection_unix_start_watches; self->super.events.update_watches = control_connection_unix_update_watches; self->super.events.stop_watches = control_connection_unix_stop_watches; + self->super.get_attached_fds = control_connection_unix_get_attached_fds; + + for (gint i = 0; i < G_N_ELEMENTS(self->attached_fds); i++) + { + self->attached_fds[i] = -1; + } return &self->super; } @@ -152,7 +223,6 @@ _control_socket_accept(void *cookie) goto error; } - cc = control_connection_unix_new(&self->super, conn_socket); /* NOTE: with the call below, the reference to the control connection (cc) From 43f28e78c0d8f9ea8c4af82509647003133c6038 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 14:38:21 +0200 Subject: [PATCH 06/42] console: new module to track the interactive connection to a terminal Signed-off-by: Balazs Scheidler --- lib/CMakeLists.txt | 2 + lib/Makefile.am | 2 + lib/console.c | 132 +++++++++++++++++++++++++++++++++++++++++++++ lib/console.h | 38 +++++++++++++ 4 files changed, 174 insertions(+) create mode 100644 lib/console.c create mode 100644 lib/console.h diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 0d5d983658..050d0a2f23 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -80,6 +80,7 @@ set (LIB_HEADERS atomic-gssize.h block-ref-parser.h cache.h + console.h cfg.h cfg-lexer.h cfg-lexer-subst.h @@ -179,6 +180,7 @@ set(LIB_SOURCES apphook.c block-ref-parser.c cache.c + console.c cfg.c cfg-args.c cfg-block.c diff --git a/lib/Makefile.am b/lib/Makefile.am index 4a552ba080..995360761b 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -97,6 +97,7 @@ pkginclude_HEADERS += \ lib/atomic-gssize.h \ lib/block-ref-parser.h \ lib/cache.h \ + lib/console.h \ lib/cfg.h \ lib/cfg-grammar.h \ lib/cfg-grammar-internal.h \ @@ -195,6 +196,7 @@ lib_libsyslog_ng_la_SOURCES = \ lib/apphook.c \ lib/block-ref-parser.c \ lib/cache.c \ + lib/console.c \ lib/cfg.c \ lib/cfg-args.c \ lib/cfg-block.c \ diff --git a/lib/console.c b/lib/console.c new file mode 100644 index 0000000000..9d58e636f1 --- /dev/null +++ b/lib/console.c @@ -0,0 +1,132 @@ +/* + * Copyright (c) 2002-2012 Balabit + * Copyright (c) 1998-2012 Balázs Scheidler + * Copyright (c) 2024 Balázs Scheidler + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ +#include "console.h" +#include +#include +#include + +GMutex console_lock; +gboolean console_present = FALSE; +gboolean using_initial_console = TRUE; + +/* NOTE: this is not synced with any changes and is just an indication whether we have a console */ +gboolean +console_is_present(gboolean exclude_initial) +{ + gboolean result; + /* the lock only serves a memory barrier but is not a real synchronization */ + g_mutex_lock(&console_lock); + if (exclude_initial && using_initial_console) + result = FALSE; + else + result = console_present; + g_mutex_unlock(&console_lock); + return result; +} + +/* set the current console to be our current stdin/out/err after we start up */ +void +console_acquire_from_stdio(void) +{ + g_assert(!console_is_present(FALSE)); + + g_mutex_lock(&console_lock); + console_present = TRUE; + g_mutex_unlock(&console_lock); +} + +/* re-acquire a console after startup using an array of fds */ +void +console_acquire_from_fds(gint fds[3]) +{ + const gchar *takeover_message_on_old_console = "[Console taken over, no further output here]\n"; + g_assert(!console_is_present(TRUE)); + + if (using_initial_console) + { + (void) write(1, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); + } + + g_mutex_lock(&console_lock); + + dup2(fds[0], STDIN_FILENO); + dup2(fds[1], STDOUT_FILENO); + dup2(fds[2], STDERR_FILENO); + + console_present = TRUE; + using_initial_console = FALSE; + g_mutex_unlock(&console_lock); +} + +/** + * console_release: + * + * Use /dev/null as input/output/error. This function is idempotent, can be + * called any number of times without harm. + **/ +void +console_release(void) +{ + gint devnull_fd; + + g_mutex_lock(&console_lock); + + if (!console_present) + goto exit; + + devnull_fd = open("/dev/null", O_RDONLY); + if (devnull_fd >= 0) + { + dup2(devnull_fd, STDIN_FILENO); + close(devnull_fd); + } + devnull_fd = open("/dev/null", O_WRONLY); + if (devnull_fd >= 0) + { + dup2(devnull_fd, STDOUT_FILENO); + dup2(devnull_fd, STDERR_FILENO); + close(devnull_fd); + } + clearerr(stdin); + clearerr(stdout); + clearerr(stderr); + console_present = FALSE; + using_initial_console = FALSE; + +exit: + g_mutex_unlock(&console_lock); +} + +void +console_global_init(void) +{ + g_mutex_init(&console_lock); +} + +void +console_global_deinit(void) +{ + g_mutex_clear(&console_lock); +} diff --git a/lib/console.h b/lib/console.h new file mode 100644 index 0000000000..631d7eb642 --- /dev/null +++ b/lib/console.h @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2002-2012 Balabit + * Copyright (c) 1998-2012 Balázs Scheidler + * Copyright (c) 2024 Balázs Scheidler + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ +#ifndef SYSLOG_NG_CONSOLE_H_INCLUDED +#define SYSLOG_NG_CONSOLE_H_INCLUDED + +#include "syslog-ng.h" + +gboolean console_is_present(gboolean exclude_initial); +void console_acquire_from_fds(gint fds[3]); +void console_acquire_from_stdio(void); +void console_release(void); + +void console_global_init(void); +void console_global_deinit(void); + +#endif From aad0567f6f9d7fb1f0501acf2fcb9498956624f0 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 14:39:01 +0200 Subject: [PATCH 07/42] gprocess: integrate startup with the new console management module Signed-off-by: Balazs Scheidler --- lib/gprocess.c | 31 +++++++++++-------------------- syslog-ng/main.c | 3 +++ 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/gprocess.c b/lib/gprocess.c index 4ed3edec62..d922393d05 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -26,6 +26,7 @@ #include "userdb.h" #include "messages.h" #include "reloc.h" +#include "console.h" #include #include @@ -651,13 +652,13 @@ g_process_message(const gchar *fmt, ...) } /** - * g_process_detach_tty: + * g_process_setup_console: * - * This function is called from g_process_start() to detach from the - * controlling tty. + * This function is called from g_process_start() to acquire the terminal as + * console or to detach from it in case we are in the background. **/ static void -g_process_detach_tty(void) +g_process_setup_console(void) { if (process_opts.mode != G_PM_FOREGROUND) { @@ -670,6 +671,10 @@ g_process_detach_tty(void) } setsid(); } + else + { + console_acquire_from_stdio(); + } } /** @@ -710,23 +715,9 @@ g_process_change_limits(void) static void g_process_detach_stdio(void) { - gint devnull_fd; - if (process_opts.mode != G_PM_FOREGROUND && stderr_present) { - devnull_fd = open("/dev/null", O_RDONLY); - if (devnull_fd >= 0) - { - dup2(devnull_fd, STDIN_FILENO); - close(devnull_fd); - } - devnull_fd = open("/dev/null", O_WRONLY); - if (devnull_fd >= 0) - { - dup2(devnull_fd, STDOUT_FILENO); - dup2(devnull_fd, STDERR_FILENO); - close(devnull_fd); - } + console_release(); stderr_present = FALSE; } } @@ -1361,7 +1352,7 @@ g_process_start(void) { pid_t pid; - g_process_detach_tty(); + g_process_setup_console(); g_process_change_limits(); if (!g_process_resolve_names()) { diff --git a/syslog-ng/main.c b/syslog-ng/main.c index f0f8d2425f..8fd03bc804 100644 --- a/syslog-ng/main.c +++ b/syslog-ng/main.c @@ -37,6 +37,7 @@ #include "mainloop.h" #include "plugin.h" #include "reloc.h" +#include "console.h" #include "resolved-configurable-paths.h" #include @@ -235,6 +236,7 @@ main(int argc, char *argv[]) GOptionContext *ctx; GError *error = NULL; + console_global_init(); MainLoop *main_loop = main_loop_get_instance(); z_mem_trace_init("syslog-ng.trace"); @@ -348,6 +350,7 @@ main(int argc, char *argv[]) app_shutdown(); z_mem_trace_dump(); g_process_finish(); + console_global_deinit(); reloc_deinit(); return rc; } From 11fcfc854afb7455362a31e30ea50a37a7c6902e Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 14:23:25 +0200 Subject: [PATCH 08/42] mainloop-control: add support for ATTACH STDIO This new command allows to reconnect the stdio streams even if syslog-ng runs in the background. syslog-ng-ctl will be able to pass 3 fds to the syslog-ng process, which will be duplicated into the standard fds and with that syslog-ng will happily start displaying its stdout to that terminal. The ATTACH command itself is threaded and the control socket is only used to indicate that the peers are still alive. syslog-ng will start sending "ALIVE" messages to this stream every second or so. If sending this packet is not successful, the connection is closed and the thread is cancelled. Upon cancellation, the stdio fds are restored to point to /dev/null. Signed-off-by: Balazs Scheidler --- lib/mainloop-control.c | 81 +++++++++++++++++++++++++++++++++++++++ syslog-ng-ctl/Makefile.am | 2 + 2 files changed, 83 insertions(+) diff --git a/lib/mainloop-control.c b/lib/mainloop-control.c index 8b94f9db4b..81080a22dd 100644 --- a/lib/mainloop-control.c +++ b/lib/mainloop-control.c @@ -31,6 +31,7 @@ #include "secret-storage/secret-storage.h" #include "cfg-walker.h" #include "logpipe.h" +#include "console.h" #include @@ -104,6 +105,85 @@ control_connection_message_log(ControlConnection *cc, GString *command, gpointer control_connection_send_reply(cc, result); } +void +_wait_until_console_is_released_or_peer_disappears(ControlConnection *cc, gint max_seconds, gboolean *cancelled) +{ + while (console_is_present(FALSE) && max_seconds != 0 && !(*cancelled)) + { + sleep(1); + if (max_seconds > 0) + max_seconds--; + control_connection_send_batched_reply(cc, g_string_new("ALIVE\n")); + } + console_release(); +} + +static void +control_connection_attach(ControlConnection *cc, GString *command, gpointer user_data, gboolean *cancelled) +{ + gchar **cmds = g_strsplit(command->str, " ", 4); + GString *result = g_string_sized_new(128); + gint n_seconds = -1; + struct + { + gboolean log_stderr; + gint log_level; + } old_values; + + old_values.log_stderr = log_stderr; + old_values.log_level = msg_get_log_level(); + + if (!cmds[1]) + { + g_string_assign(result, "FAIL Invalid arguments received"); + goto exit; + } + + if (g_str_equal(cmds[1], "STDIO")) + { + ; + } + else if (g_str_equal(cmds[1], "LOGS")) + { + log_stderr = TRUE; + if (cmds[3] && !_control_process_log_level(cmds[3], result)) + goto exit; + } + else + { + g_string_assign(result, "FAIL This version of syslog-ng only supports attaching to STDIO"); + goto exit; + } + + if (cmds[2]) + n_seconds = atoi(cmds[2]); + + if (console_is_present(TRUE)) + { + g_string_assign(result, "FAIL A console is already attached to syslog-ng, only one console is allowed"); + goto exit; + } + + gint fds[3]; + gsize num_fds = G_N_ELEMENTS(fds); + if (!control_connection_get_attached_fds(cc, fds, &num_fds) || num_fds != 3) + { + g_string_assign(result, + "FAIL The underlying transport for syslog-ng-ctl does not support fd passing or incorrect number of fds received"); + goto exit; + } + console_acquire_from_fds(fds); + + _wait_until_console_is_released_or_peer_disappears(cc, n_seconds, cancelled); + g_string_assign(result, "OK [console output ends here]"); + log_stderr = old_values.log_stderr; + msg_set_log_level(old_values.log_level); +exit: + control_connection_send_batched_reply(cc, result); + control_connection_send_close_batch(cc); + g_strfreev(cmds); +} + static void control_connection_stop_process(ControlConnection *cc, GString *command, gpointer user_data, gboolean *cancelled) { @@ -452,6 +532,7 @@ export_config_graph(ControlConnection *cc, GString *command, gpointer user_data, ControlCommand default_commands[] = { + { "ATTACH", control_connection_attach, .threaded = TRUE }, { "LOG", control_connection_message_log }, { "STOP", control_connection_stop_process }, { "RELOAD", control_connection_reload }, diff --git a/syslog-ng-ctl/Makefile.am b/syslog-ng-ctl/Makefile.am index 601143fb4e..88c393e11c 100644 --- a/syslog-ng-ctl/Makefile.am +++ b/syslog-ng-ctl/Makefile.am @@ -6,6 +6,8 @@ syslog_ng_ctl_syslog_ng_ctl_SOURCES = \ syslog-ng-ctl/syslog-ng-ctl.c \ syslog-ng-ctl/commands/commands.h \ syslog-ng-ctl/commands/commands.c \ + syslog-ng-ctl/commands/attach.h \ + syslog-ng-ctl/commands/attach.c \ syslog-ng-ctl/commands/config.h \ syslog-ng-ctl/commands/config.c \ syslog-ng-ctl/commands/credentials.h \ From 624205664431aeb9f0f7a4cb647e8f8b9e71335f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 14:24:28 +0200 Subject: [PATCH 09/42] syslog-ng-ctl: add support for "attach" command Signed-off-by: Balazs Scheidler --- syslog-ng-ctl/CMakeLists.txt | 2 + syslog-ng-ctl/commands/attach.c | 88 +++++++++++++++++++++++++++++ syslog-ng-ctl/commands/attach.h | 32 +++++++++++ syslog-ng-ctl/commands/commands.c | 35 +++++++++++- syslog-ng-ctl/commands/commands.h | 2 +- syslog-ng-ctl/control-client-unix.c | 33 ++++++++++- syslog-ng-ctl/control-client.h | 2 +- syslog-ng-ctl/syslog-ng-ctl.c | 2 + tests/copyright/policy | 1 + 9 files changed, 190 insertions(+), 7 deletions(-) create mode 100644 syslog-ng-ctl/commands/attach.c create mode 100644 syslog-ng-ctl/commands/attach.h diff --git a/syslog-ng-ctl/CMakeLists.txt b/syslog-ng-ctl/CMakeLists.txt index 0deab8846f..162364f0cf 100644 --- a/syslog-ng-ctl/CMakeLists.txt +++ b/syslog-ng-ctl/CMakeLists.txt @@ -1,6 +1,8 @@ set(SYSLOG_NG_CTL_SOURCES syslog-ng-ctl.c control-client.h + commands/attach.h + commands/attach.c commands/commands.h commands/commands.c commands/credentials.h diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c new file mode 100644 index 0000000000..f1e1f4b655 --- /dev/null +++ b/syslog-ng-ctl/commands/attach.c @@ -0,0 +1,88 @@ +/* + * Copyright (c) 2024 Balazs Scheidler + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ + +#include "ctl-stats.h" +#include "syslog-ng.h" + +static gint attach_options_seconds; +static gchar *attach_options_log_level = NULL; +static gchar **attach_commands = NULL; + +static gboolean +_store_log_level(const gchar *option_name, + const gchar *value, + gpointer data, + GError **error) +{ + if (!attach_options_log_level) + { + attach_options_log_level = g_strdup(value); + return TRUE; + } + g_set_error(error, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, "You can't specify multiple log-levels at a time."); + return FALSE; +} + +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", "" }, + { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &attach_commands, "attach mode: logs, stdio", NULL }, + { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL } +}; + +gint +slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) +{ + GString *command = g_string_new("ATTACH"); + const gchar *attach_mode; + + if (attach_commands) + { + if (attach_commands[1]) + { + fprintf(stderr, "Too many arguments"); + return 1; + } + attach_mode = attach_commands[0]; + } + else + attach_mode = "stdio"; + + if (g_str_equal(attach_mode, "stdio")) + g_string_append(command, " STDIO"); + else if (g_str_equal(attach_mode, "logs")) + g_string_append(command, " LOGS"); + else + { + fprintf(stderr, "Unknown attach mode\n"); + return 1; + } + + g_string_append_printf(command, " %d", attach_options_seconds ? : -1); + if (attach_options_log_level) + g_string_append_printf(command, " %s", attach_options_log_level); + gint result = attach_command(command->str); + g_string_free(command, TRUE); + return result; +} diff --git a/syslog-ng-ctl/commands/attach.h b/syslog-ng-ctl/commands/attach.h new file mode 100644 index 0000000000..b544924ff4 --- /dev/null +++ b/syslog-ng-ctl/commands/attach.h @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2024 Balazs Scheidler + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ + +#ifndef SYSLOG_NG_CTL_ATTACH_H_INCLUDED +#define SYSLOG_NG_CTL_ATTACH_H_INCLUDED 1 + +#include "commands.h" + +extern GOptionEntry attach_options[]; +gint slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx); + +#endif diff --git a/syslog-ng-ctl/commands/commands.c b/syslog-ng-ctl/commands/commands.c index 2ff1add1e3..ec15828a70 100644 --- a/syslog-ng-ctl/commands/commands.c +++ b/syslog-ng-ctl/commands/commands.c @@ -57,14 +57,14 @@ process_response_status(GString *response) } static gboolean -slng_send_cmd(const gchar *cmd) +slng_send_cmd(const gchar *cmd, gboolean attach) { if (!control_client_connect(control_client)) { return FALSE; } - if (control_client_send_command(control_client, cmd) < 0) + if (control_client_send_command(control_client, cmd, attach) < 0) { return FALSE; } @@ -75,7 +75,16 @@ slng_send_cmd(const gchar *cmd) gint slng_run_command(const gchar *command, CommandResponseHandlerFunc cb, gpointer user_data) { - if (!slng_send_cmd(command)) + if (!slng_send_cmd(command, FALSE)) + return 1; + + return control_client_read_reply(control_client, cb, user_data); +} + +gint +slng_attach_command(const gchar *command, CommandResponseHandlerFunc cb, gpointer user_data) +{ + if (!slng_send_cmd(command, TRUE)) return 1; return control_client_read_reply(control_client, cb, user_data); @@ -117,6 +126,26 @@ dispatch_command(const gchar *cmd) return retval; } +static gint +_ignore_alive_messages(GString *reply, gpointer user_data) +{ + return 0; +} + +gint +attach_command(const gchar *cmd) +{ + gboolean first_response = TRUE; + gint retval = 0; + gchar *dispatchable_command = g_strdup_printf("%s\n", cmd); + retval = slng_attach_command(dispatchable_command, _ignore_alive_messages, &first_response); + + secret_storage_wipe(dispatchable_command, strlen(dispatchable_command)); + g_free(dispatchable_command); + + return retval; +} + gint run(const gchar *control_name, gint argc, gchar **argv, CommandDescriptor *mode, GOptionContext *ctx) { diff --git a/syslog-ng-ctl/commands/commands.h b/syslog-ng-ctl/commands/commands.h index 0072f5568e..f24c4f586a 100644 --- a/syslog-ng-ctl/commands/commands.h +++ b/syslog-ng-ctl/commands/commands.h @@ -43,7 +43,7 @@ typedef struct _CommandDescriptor typedef gint (*CommandResponseHandlerFunc)(GString *response, gpointer user_data); gint dispatch_command(const gchar *cmd); -gint slng_run_command(const gchar *command, CommandResponseHandlerFunc cb, gpointer user_data); +gint attach_command(const gchar *cmd); gint process_response_status(GString *response); gboolean is_syslog_ng_running(void); diff --git a/syslog-ng-ctl/control-client-unix.c b/syslog-ng-ctl/control-client-unix.c index 0bcd8bc99e..f960dab67c 100644 --- a/syslog-ng-ctl/control-client-unix.c +++ b/syslog-ng-ctl/control-client-unix.c @@ -79,9 +79,38 @@ control_client_connect(ControlClient *self) } gint -control_client_send_command(ControlClient *self, const gchar *cmd) +control_client_send_command(ControlClient *self, const gchar *cmd, gboolean attach) { - return fwrite(cmd, strlen(cmd), 1, self->control_socket); + struct iovec iov[1] = + { + { .iov_base = (gchar *) cmd, .iov_len = strlen(cmd) }, + }; + gint fds[3] = { 0, 1, 2 }; + union + { + char buf[CMSG_SPACE(sizeof(fds))]; + struct cmsghdr align; + } u; + struct msghdr msg = + { + .msg_iov = iov, + .msg_iovlen = G_N_ELEMENTS(iov), + 0 + }; + if (attach) + { + msg.msg_control = u.buf; + msg.msg_controllen = sizeof(u.buf); + + struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(fds)); + memcpy(CMSG_DATA(cmsg), fds, sizeof(fds)); + } + + return sendmsg(self->control_fd, &msg, 0); +// return fwrite(cmd, strlen(cmd), 1, self->control_socket); } #define BUFF_LEN 8192 diff --git a/syslog-ng-ctl/control-client.h b/syslog-ng-ctl/control-client.h index c3efbf15a5..081e1ca988 100644 --- a/syslog-ng-ctl/control-client.h +++ b/syslog-ng-ctl/control-client.h @@ -32,7 +32,7 @@ typedef struct _ControlClient ControlClient; ControlClient *control_client_new(const gchar *path); gboolean control_client_connect(ControlClient *self); -gint control_client_send_command(ControlClient *self, const gchar *cmd); +gint control_client_send_command(ControlClient *self, const gchar *cmd, gboolean attach); gint control_client_read_reply(ControlClient *self, CommandResponseHandlerFunc cb, gpointer user_data); void control_client_free(ControlClient *self); diff --git a/syslog-ng-ctl/syslog-ng-ctl.c b/syslog-ng-ctl/syslog-ng-ctl.c index d029a1a591..0e2fd45aa6 100644 --- a/syslog-ng-ctl/syslog-ng-ctl.c +++ b/syslog-ng-ctl/syslog-ng-ctl.c @@ -37,6 +37,7 @@ #include "commands/query.h" #include "commands/license.h" #include "commands/healthcheck.h" +#include "commands/attach.h" #include #include @@ -111,6 +112,7 @@ slng_export_config_graph(int argc, char *argv[], const gchar *mode, GOptionConte static CommandDescriptor modes[] = { + { "attach", attach_options, "Attach to a running syslog-ng instance", slng_attach, NULL }, { "stats", stats_options, "Get syslog-ng statistics. Possible commands: csv, prometheus; default: csv", slng_stats, NULL }, { "verbose", verbose_options, "Enable/query verbose messages", slng_verbose, NULL }, { "debug", verbose_options, "Enable/query debug messages", slng_verbose, NULL }, diff --git a/tests/copyright/policy b/tests/copyright/policy index 57b05879a4..59778b5321 100644 --- a/tests/copyright/policy +++ b/tests/copyright/policy @@ -121,6 +121,7 @@ lib/tests/test_generic_number\.c lib/severity-aliases\.table lib/transport/transport-adapter\.[ch] syslog-ng-ctl/commands/log-level.[ch] +syslog-ng-ctl/commands/attach.[ch] modules/afsocket/afsocket-signals.h syslog-ng-ctl/commands/healthcheck.[ch] modules/python-modules/syslogng/confgen\.py From 66e575e505fde2d0cefe141d58e44b70a00d254a Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 19:03:50 +0200 Subject: [PATCH 10/42] control: remove and bump verbosity for some control socket related messages Signed-off-by: Balazs Scheidler --- lib/control/control-connection.c | 2 +- lib/control/control-server.c | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/control/control-connection.c b/lib/control/control-connection.c index 2f19dbb570..ed58460e81 100644 --- a/lib/control/control-connection.c +++ b/lib/control/control-connection.c @@ -223,7 +223,7 @@ control_connection_io_input(void *s) } else if (rc == 0) { - msg_debug("EOF on control channel, closing connection"); + msg_trace("EOF on control channel, closing connection"); goto destroy_connection; } else diff --git a/lib/control/control-server.c b/lib/control/control-server.c index 18d1c04953..058237467d 100644 --- a/lib/control/control-server.c +++ b/lib/control/control-server.c @@ -67,9 +67,7 @@ control_server_cancel_workers(ControlServer *self, ControlConnection *cc) { if (self->worker_threads) { - msg_debug("Cancelling control server worker threads"); g_list_foreach(self->worker_threads, _cancel_worker, cc); - msg_debug("Control server worker threads have been cancelled"); } } From 6dcf9ab6eccc9fa9f697b2b9f3820a05b766135e Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 5 Oct 2024 19:15:40 +0200 Subject: [PATCH 11/42] gprocess: get rid off the stderr_present variable Signed-off-by: Balazs Scheidler --- lib/gprocess.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/gprocess.c b/lib/gprocess.c index d922393d05..e3cb11edb0 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -100,7 +100,6 @@ static gint startup_result_pipe[2] = { -1, -1 }; /* pipe used to deliver initialization result to the supervisor */ static gint init_result_pipe[2] = { -1, -1 }; static GProcessKind process_kind = G_PK_STARTUP; -static gboolean stderr_present = TRUE; #if SYSLOG_NG_ENABLE_LINUX_CAPS static int have_capsyslog = FALSE; static cap_value_t cap_syslog; @@ -638,7 +637,7 @@ g_process_message(const gchar *fmt, ...) va_start(ap, fmt); g_vsnprintf(buf, sizeof(buf), fmt, ap); va_end(ap); - if (stderr_present) + if (console_is_present(FALSE)) fprintf(stderr, "%s: %s\n", process_opts.name, buf); else { @@ -715,10 +714,9 @@ g_process_change_limits(void) static void g_process_detach_stdio(void) { - if (process_opts.mode != G_PM_FOREGROUND && stderr_present) + if (process_opts.mode != G_PM_FOREGROUND) { console_release(); - stderr_present = FALSE; } } From 1f68f2b5b5e6ea1e0b1ee3998423af1b21a44ec7 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 15:52:03 +0200 Subject: [PATCH 12/42] logpipe: uninline log_pipe_queue() This is never inlined anyway, but duplicated into all modules using log_pipe_queue(), it is then better to just have it in one location. Signed-off-by: Balazs Scheidler --- lib/logpipe.c | 61 +++++++++++++++++++++++++++++++++++++++++++++ lib/logpipe.h | 69 +++------------------------------------------------ 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/lib/logpipe.c b/lib/logpipe.c index 202d39f048..ad555a4f55 100644 --- a/lib/logpipe.c +++ b/lib/logpipe.c @@ -28,6 +28,67 @@ gboolean (*pipe_single_step_hook)(LogPipe *pipe, LogMessage *msg, const LogPathOptions *path_options); +void +log_pipe_forward_msg(LogPipe *self, LogMessage *msg, const LogPathOptions *path_options) +{ + if (self->pipe_next) + { + log_pipe_queue(self->pipe_next, msg, path_options); + } + else + { + log_msg_drop(msg, path_options, AT_PROCESSED); + } +} + +void +log_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) +{ + LogPathOptions local_path_options; + g_assert((s->flags & PIF_INITIALIZED) != 0); + + if (G_UNLIKELY(pipe_single_step_hook)) + { + if (!pipe_single_step_hook(s, msg, path_options)) + { + log_msg_drop(msg, path_options, AT_PROCESSED); + return; + } + } + + if ((s->flags & PIF_SYNC_FILTERX)) + filterx_eval_sync_message(path_options->filterx_context, &msg, path_options); + + if (G_UNLIKELY(s->flags & (PIF_HARD_FLOW_CONTROL | PIF_JUNCTION_END | PIF_CONDITIONAL_MIDPOINT))) + { + path_options = log_path_options_chain(&local_path_options, path_options); + if (s->flags & PIF_HARD_FLOW_CONTROL) + { + local_path_options.flow_control_requested = 1; + msg_trace("Requesting flow control", log_pipe_location_tag(s)); + } + if (s->flags & PIF_JUNCTION_END) + { + log_path_options_pop_junction(&local_path_options); + } + if (s->flags & PIF_CONDITIONAL_MIDPOINT) + { + log_path_options_pop_conditional(&local_path_options); + } + } + + if (s->queue) + { + s->queue(s, msg, path_options); + } + else + { + log_pipe_forward_msg(s, msg, path_options); + } + +} + + EVTTAG * log_pipe_location_tag(LogPipe *pipe) { diff --git a/lib/logpipe.h b/lib/logpipe.h index b955be567c..252ca014d3 100644 --- a/lib/logpipe.h +++ b/lib/logpipe.h @@ -428,68 +428,7 @@ log_pipe_post_config_init(LogPipe *s) return TRUE; } -static inline void -log_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options); - -static inline void -log_pipe_forward_msg(LogPipe *self, LogMessage *msg, const LogPathOptions *path_options) -{ - if (self->pipe_next) - { - log_pipe_queue(self->pipe_next, msg, path_options); - } - else - { - log_msg_drop(msg, path_options, AT_PROCESSED); - } -} - -static inline void -log_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) -{ - LogPathOptions local_path_options; - g_assert((s->flags & PIF_INITIALIZED) != 0); - - if (G_UNLIKELY(pipe_single_step_hook)) - { - if (!pipe_single_step_hook(s, msg, path_options)) - { - log_msg_drop(msg, path_options, AT_PROCESSED); - return; - } - } - if ((s->flags & PIF_SYNC_FILTERX)) - filterx_eval_sync_message(path_options->filterx_context, &msg, path_options); - - if (G_UNLIKELY(s->flags & (PIF_HARD_FLOW_CONTROL | PIF_JUNCTION_END | PIF_CONDITIONAL_MIDPOINT))) - { - path_options = log_path_options_chain(&local_path_options, path_options); - if (s->flags & PIF_HARD_FLOW_CONTROL) - { - local_path_options.flow_control_requested = 1; - msg_trace("Requesting flow control", log_pipe_location_tag(s)); - } - if (s->flags & PIF_JUNCTION_END) - { - log_path_options_pop_junction(&local_path_options); - } - if (s->flags & PIF_CONDITIONAL_MIDPOINT) - { - log_path_options_pop_conditional(&local_path_options); - } - } - - if (s->queue) - { - s->queue(s, msg, path_options); - } - else - { - log_pipe_forward_msg(s, msg, path_options); - } - -} static inline LogPipe * log_pipe_clone(LogPipe *self) @@ -511,11 +450,11 @@ log_pipe_append(LogPipe *s, LogPipe *next) s->pipe_next = next; } -void -log_pipe_set_persist_name(LogPipe *self, const gchar *persist_name); +void log_pipe_set_persist_name(LogPipe *self, const gchar *persist_name); +const gchar *log_pipe_get_persist_name(const LogPipe *self); -const gchar * -log_pipe_get_persist_name(const LogPipe *self); +void log_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options); +void log_pipe_forward_msg(LogPipe *self, LogMessage *msg, const LogPathOptions *path_options); void log_pipe_set_options(LogPipe *self, const LogPipeOptions *options); void log_pipe_set_internal(LogPipe *self, gboolean internal); From 44a8275872e43a99bd7a9af4e4d14e77a295da80 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 15:58:47 +0200 Subject: [PATCH 13/42] logpipe: eliminate s->queue == NULL trick log_pipe_forward_msg() is not inlined anyway, so it will just add an extra branch. Signed-off-by: Balazs Scheidler --- lib/logpipe.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/logpipe.c b/lib/logpipe.c index ad555a4f55..c05a85f471 100644 --- a/lib/logpipe.c +++ b/lib/logpipe.c @@ -77,15 +77,7 @@ log_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) } } - if (s->queue) - { - s->queue(s, msg, path_options); - } - else - { - log_pipe_forward_msg(s, msg, path_options); - } - + s->queue(s, msg, path_options); } @@ -140,7 +132,7 @@ log_pipe_init_instance(LogPipe *self, GlobalConfig *cfg) * log_msg_forward_msg. Since this is a common case, it is better * inlined (than to use an indirect call) for performance. */ - self->queue = NULL; + self->queue = log_pipe_forward_msg; self->free_fn = log_pipe_free_method; self->arcs = _arcs; } From 78cf282f215b940f44fddb0f567eda2543a3e2a3 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 16:35:14 +0200 Subject: [PATCH 14/42] logpipe: whitespace fix Signed-off-by: Balazs Scheidler --- lib/logpipe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/logpipe.c b/lib/logpipe.c index c05a85f471..49eb2bae0c 100644 --- a/lib/logpipe.c +++ b/lib/logpipe.c @@ -93,7 +93,8 @@ log_pipe_attach_expr_node(LogPipe *self, LogExprNode *expr_node) self->expr_node = log_expr_node_ref(expr_node); } -void log_pipe_detach_expr_node(LogPipe *self) +void +log_pipe_detach_expr_node(LogPipe *self) { if (!self->expr_node) return; From 695b3c071913f995625cf1096b0e1879278b695d Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 16:36:37 +0200 Subject: [PATCH 15/42] logpipe: move PIF_CONFIG_RELATED check to the call site Checking the global variable pipe_single_step_hook is quite complicated as it is addressed $rip relative, so let's filter out the config related pipes first and remove the same check from the debugger. Signed-off-by: Balazs Scheidler --- lib/debugger/debugger-main.c | 3 --- lib/logpipe.c | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c index 66309a5dff..088078a0cb 100644 --- a/lib/debugger/debugger-main.c +++ b/lib/debugger/debugger-main.c @@ -30,9 +30,6 @@ static Debugger *current_debugger; static gboolean _pipe_hook(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) { - if ((s->flags & PIF_CONFIG_RELATED) == 0) - return TRUE; - if (msg->flags & LF_STATE_TRACING) return debugger_perform_tracing(current_debugger, s, msg); else diff --git a/lib/logpipe.c b/lib/logpipe.c index 49eb2bae0c..7e844fecc2 100644 --- a/lib/logpipe.c +++ b/lib/logpipe.c @@ -47,7 +47,7 @@ log_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) LogPathOptions local_path_options; g_assert((s->flags & PIF_INITIALIZED) != 0); - if (G_UNLIKELY(pipe_single_step_hook)) + if (G_UNLIKELY((s->flags & PIF_CONFIG_RELATED) != 0 && pipe_single_step_hook)) { if (!pipe_single_step_hook(s, msg, path_options)) { From 0031f3e146fd7fd3eb497423d3db9a367689e3f0 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:34:11 +0200 Subject: [PATCH 16/42] 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 --- 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 79d2f3c0d7fda9a5ba64979e02b0792bd3937562 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:36:48 +0200 Subject: [PATCH 17/42] 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 --- 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 #include @@ -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 f43d0fbc61db8bff1c3f33b10c423f747ee820b6 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 18:15:44 +0200 Subject: [PATCH 18/42] debugger: rename interactive_thread to debugger_thread Signed-off-by: Balazs Scheidler --- 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 abe1d39ff60b5582383aac53e1ea0be72f4d20d0 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:39:43 +0200 Subject: [PATCH 19/42] 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 --- 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 4f15ff12c55d39bfa1fcab047bac6a69d6f3fd0e Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:40:46 +0200 Subject: [PATCH 20/42] debugger: add debugger_stop() call alongside debugger_start() Signed-off-by: Balazs Scheidler --- 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 d24ee743ea9df82ab828613cc0f62db34fc441f6 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 18:15:08 +0200 Subject: [PATCH 21/42] debugger: make sure that debugger_start_console and debugger_exit() run from the main thread Signed-off-by: Balazs Scheidler --- 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 b3ecc34d85db8abf9b55aa6dea29baf3210057bd Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 13:58:47 +0200 Subject: [PATCH 22/42] tracer: move the Tracer struct to the implementation file Signed-off-by: Balazs Scheidler --- 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 3a402c36fc20ff5b0c59f2c3ddd966c89512d59d Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:41:45 +0200 Subject: [PATCH 23/42] 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 --- 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 ca8b0363b4ebdd2bdf4adbef9e8bbb230c720c1f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 10:31:09 +0200 Subject: [PATCH 24/42] syslog-ng-ctl: add support for "attach debugger" Signed-off-by: Balazs Scheidler --- lib/debugger/debugger-main.c | 5 +++++ lib/debugger/debugger-main.h | 1 + lib/mainloop-control.c | 17 +++++++++++++++++ syslog-ng-ctl/commands/attach.c | 4 +++- 4 files changed, 26 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 81080a22dd..b0404e4d70 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 @@ -121,9 +122,12 @@ _wait_until_console_is_released_or_peer_disappears(ControlConnection *cc, gint m 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; @@ -149,6 +153,10 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user if (cmds[3] && !_control_process_log_level(cmds[3], result)) 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"); @@ -174,7 +182,16 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user } console_acquire_from_fds(fds); + 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_console_is_released_or_peer_disappears(cc, n_seconds, cancelled); + if (start_debugger && debugger_is_running()) + { + debugger_stop(); + } g_string_assign(result, "OK [console output ends here]"); 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", "" }, - { 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"); From de5c76e06346dcaa29cb3bfd8bb5d092a22d992e Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 18:49:23 +0200 Subject: [PATCH 25/42] mainloop: do not register signal handlers as exclusive As long as we have a single signal registrations, these will be executed anyway. IV_SIGNAL_FLAG_EXCLUSIVE are for registrations that want to suppress previous registrations. I intend to use SIGINT in the debugger to break into the debugger to allow executing commands even outside of the pipeline (e.g. setting breakpoints for the future). Signed-off-by: Balazs Scheidler --- lib/mainloop.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/mainloop.c b/lib/mainloop.c index e126bd3395..f03490277b 100644 --- a/lib/mainloop.c +++ b/lib/mainloop.c @@ -550,7 +550,6 @@ _register_signal_handler(struct iv_signal *signal_poll, gint signum, void (*hand { IV_SIGNAL_INIT(signal_poll); signal_poll->signum = signum; - signal_poll->flags = IV_SIGNAL_FLAG_EXCLUSIVE; signal_poll->cookie = user_data; signal_poll->handler = handler; iv_signal_register(signal_poll); From 45672ea7aa3ec026b5d3e0b6248652e5fa3eb7df Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 18:51:12 +0200 Subject: [PATCH 26/42] tracer: implement interrupt support Signed-off-by: Balazs Scheidler --- lib/debugger/tracer.c | 26 +++++++++++++++++++++----- lib/debugger/tracer.h | 5 +++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lib/debugger/tracer.c b/lib/debugger/tracer.c index 5ad8e1c748..915e5e5f40 100644 --- a/lib/debugger/tracer.c +++ b/lib/debugger/tracer.c @@ -62,10 +62,20 @@ tracer_stop_on_breakpoint(Tracer *self, BreakpointSite *breakpoint_site) g_mutex_unlock(&self->breakpoint_mutex); } +void +tracer_stop_on_interrupt(Tracer *self) +{ + g_mutex_lock(&self->breakpoint_mutex); + /* send interrupt signal as a NULL */ + g_queue_push_tail(self->waiting_breakpoints, NULL); + g_cond_signal(&self->breakpoint_cond); + g_mutex_unlock(&self->breakpoint_mutex); +} + /* 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, BreakpointSite **breakpoint_site) +tracer_wait_for_event(Tracer *self, BreakpointSite **breakpoint_site) { gboolean cancelled = FALSE; g_mutex_lock(&self->breakpoint_mutex); @@ -92,13 +102,19 @@ tracer_wait_for_breakpoint(Tracer *self, BreakpointSite **breakpoint_site) /* NOTE: called by the interactive debugger to resume the worker after a breakpoint */ void -tracer_resume_after_breakpoint(Tracer *self, BreakpointSite *breakpoint_site) +tracer_resume_after_event(Tracer *self, BreakpointSite *breakpoint_site) { g_mutex_lock(&self->breakpoint_mutex); g_assert(self->pending_breakpoint == breakpoint_site); - self->pending_breakpoint->resume_requested = TRUE; - self->pending_breakpoint = NULL; - g_cond_broadcast(&self->resume_cond); + if (self->pending_breakpoint) + { + /* we might be returning from an interrupt in which case + * pending_breakpoint is NULL, nothing to resume */ + + self->pending_breakpoint->resume_requested = TRUE; + self->pending_breakpoint = NULL; + g_cond_broadcast(&self->resume_cond); + } g_mutex_unlock(&self->breakpoint_mutex); } diff --git a/lib/debugger/tracer.h b/lib/debugger/tracer.h index de2a8a9da5..b0b46f8bba 100644 --- a/lib/debugger/tracer.h +++ b/lib/debugger/tracer.h @@ -38,9 +38,10 @@ typedef struct _BreakpointSite } BreakpointSite; +void tracer_stop_on_interrupt(Tracer *self); 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); +gboolean tracer_wait_for_event(Tracer *self, BreakpointSite **breakpoint_site); +void tracer_resume_after_event(Tracer *self, BreakpointSite *breakpoint_site); void tracer_cancel(Tracer *self); Tracer *tracer_new(GlobalConfig *cfg); From 677d350319c76431edd25a9079aa1a4599049b36 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 6 Oct 2024 22:02:29 +0200 Subject: [PATCH 27/42] debugger: get into the debugger on SIGINT Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 62 ++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index 89e6781224..3f54b4be56 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -31,12 +31,14 @@ #include "compat/time.h" #include "scratch-buffers.h" +#include #include #include struct _Debugger { Tracer *tracer; + struct iv_signal sigint; MainLoop *main_loop; GlobalConfig *cfg; gchar *command_buffer; @@ -210,7 +212,8 @@ static gboolean _cmd_quit(Debugger *self, gint argc, gchar *argv[]) { main_loop_exit(self->main_loop); - self->breakpoint_site->drop = TRUE; + if (self->breakpoint_site) + self->breakpoint_site->drop = TRUE; return FALSE; } @@ -245,6 +248,7 @@ struct { const gchar *name; DebuggerCommandFunc command; + gboolean requires_breakpoint_site; } command_table[] = { { "help", _cmd_help }, @@ -252,15 +256,15 @@ struct { "?", _cmd_help }, { "continue", _cmd_continue }, { "c", _cmd_continue }, - { "print", _cmd_print }, - { "p", _cmd_print }, + { "print", _cmd_print, .requires_breakpoint_site = TRUE }, + { "p", _cmd_print, .requires_breakpoint_site = TRUE }, { "display", _cmd_display }, - { "drop", _cmd_drop }, + { "drop", _cmd_drop, .requires_breakpoint_site = TRUE }, { "quit", _cmd_quit }, { "q", _cmd_quit }, - { "trace", _cmd_trace }, - { "info", _cmd_info }, - { "i", _cmd_info }, + { "trace", _cmd_trace, .requires_breakpoint_site = TRUE }, + { "info", _cmd_info, .requires_breakpoint_site = TRUE }, + { "i", _cmd_info, .requires_breakpoint_site = TRUE }, { NULL, NULL } }; @@ -319,6 +323,7 @@ _handle_command(Debugger *self) gint argc; gchar **argv; GError *error = NULL; + gboolean requires_breakpoint_site = TRUE; DebuggerCommandFunc command = NULL; if (!g_shell_parse_argv(self->command_buffer ? : "", &argc, &argv, &error)) @@ -333,6 +338,7 @@ _handle_command(Debugger *self) if (strcmp(command_table[i].name, argv[0]) == 0) { command = command_table[i].command; + requires_breakpoint_site = command_table[i].requires_breakpoint_site; break; } } @@ -341,6 +347,11 @@ _handle_command(Debugger *self) printf("Undefined command %s, try \"help\"\n", argv[0]); return TRUE; } + else if (requires_breakpoint_site && self->breakpoint_site == NULL) + { + printf("Running in interrupt context, command %s requires pipeline context\n", argv[0]); + return TRUE; + } gboolean result = command(self, argc, argv); g_strfreev(argv); return result; @@ -350,11 +361,20 @@ static void _handle_interactive_prompt(Debugger *self) { gchar buf[1024]; - LogPipe *current_pipe = self->breakpoint_site->pipe; + LogPipe *current_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->breakpoint_site->msg, self->display_template); + if (self->breakpoint_site) + { + 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->breakpoint_site->msg, self->display_template); + } + else + { + printf("Stopping on interrupt, message related commands are unavailable...\n"); + } while (1) { _fetch_command(self); @@ -374,22 +394,37 @@ _debugger_thread_func(Debugger *self) while (1) { self->breakpoint_site = NULL; - if (!tracer_wait_for_breakpoint(self->tracer, &self->breakpoint_site)) + if (!tracer_wait_for_event(self->tracer, &self->breakpoint_site)) break; _handle_interactive_prompt(self); - tracer_resume_after_breakpoint(self->tracer, self->breakpoint_site); + tracer_resume_after_event(self->tracer, self->breakpoint_site); } scratch_buffers_explicit_gc(); app_thread_stop(); return NULL; } +static void +_interrupt(gpointer user_data) +{ + Debugger *self = (Debugger *) user_data; + + tracer_stop_on_interrupt(self->tracer); +} + void debugger_start_console(Debugger *self) { main_loop_assert_main_thread(); + IV_SIGNAL_INIT(&self->sigint); + self->sigint.signum = SIGINT; + self->sigint.flags = IV_SIGNAL_FLAG_EXCLUSIVE; + self->sigint.cookie = self; + self->sigint.handler = _interrupt; + iv_signal_register(&self->sigint); + self->debugger_thread = g_thread_new(NULL, (GThreadFunc) _debugger_thread_func, self); } @@ -429,6 +464,7 @@ debugger_exit(Debugger *self) { main_loop_assert_main_thread(); + iv_signal_unregister(&self->sigint); tracer_cancel(self->tracer); g_thread_join(self->debugger_thread); } From 5ba6d4d437b96cee28c00459660aab9dc3705822 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Tue, 15 Oct 2024 10:13:41 +0200 Subject: [PATCH 28/42] logreader: retry on main_loop_io_worker_job_submit() failure Signed-off-by: Balazs Scheidler --- lib/logreader.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/logreader.c b/lib/logreader.c index c8747e4635..f93393542d 100644 --- a/lib/logreader.c +++ b/lib/logreader.c @@ -573,7 +573,11 @@ log_reader_io_handle_in(gpointer s) log_reader_disable_watches(self); if ((self->options->flags & LR_THREADED)) { - main_loop_io_worker_job_submit(&self->io_job, NULL); + if (!main_loop_io_worker_job_submit(&self->io_job, NULL)) + { + log_reader_set_immediate_check(self); + log_reader_update_watches(self); + } } else { From 3228a54871fa5f8beb1a51602c969d3b300fa11e Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 20 Oct 2024 20:54:07 +0200 Subject: [PATCH 29/42] console: extract console_printf from gprocess Signed-off-by: Balazs Scheidler --- lib/console.c | 37 +++++++++++++++- lib/console.h | 4 +- lib/gprocess.c | 113 +++++++++++++++++------------------------------ lib/gprocess.h | 2 - syslog-ng/main.c | 4 +- 5 files changed, 81 insertions(+), 79 deletions(-) diff --git a/lib/console.c b/lib/console.c index 9d58e636f1..9a3ecb8b2e 100644 --- a/lib/console.c +++ b/lib/console.c @@ -26,10 +26,44 @@ #include #include #include +#include GMutex console_lock; gboolean console_present = FALSE; gboolean using_initial_console = TRUE; +const gchar *console_prefix; + +/** + * console_printf: + * @fmt: format string + * @...: arguments to @fmt + * + * This function sends a message to the client preferring to use the stderr + * channel as long as it is available and switching to using syslog() if it + * isn't. Generally the stderr channell will be available in the startup + * process and in the beginning of the first startup in the + * supervisor/daemon processes. Later on the stderr fd will be closed and we + * have to fall back to using the system log. + **/ +void +console_printf(const gchar *fmt, ...) +{ + gchar buf[2048]; + va_list ap; + + va_start(ap, fmt); + g_vsnprintf(buf, sizeof(buf), fmt, ap); + va_end(ap); + if (console_is_present(FALSE)) + fprintf(stderr, "%s: %s\n", console_prefix, buf); + else + { + openlog(console_prefix, LOG_PID, LOG_DAEMON); + syslog(LOG_CRIT, "%s\n", buf); + closelog(); + } +} + /* NOTE: this is not synced with any changes and is just an indication whether we have a console */ gboolean @@ -120,9 +154,10 @@ console_release(void) } void -console_global_init(void) +console_global_init(const gchar *console_prefix_) { g_mutex_init(&console_lock); + console_prefix = console_prefix_; } void diff --git a/lib/console.h b/lib/console.h index 631d7eb642..f1c6b719f6 100644 --- a/lib/console.h +++ b/lib/console.h @@ -27,12 +27,14 @@ #include "syslog-ng.h" +void console_printf(const gchar *fmt, ...) __attribute__ ((format (printf, 1, 2))); + gboolean console_is_present(gboolean exclude_initial); void console_acquire_from_fds(gint fds[3]); void console_acquire_from_stdio(void); void console_release(void); -void console_global_init(void); +void console_global_init(const gchar *console_prefix); void console_global_deinit(void); #endif diff --git a/lib/gprocess.c b/lib/gprocess.c index e3cb11edb0..68cdd1a452 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -615,41 +615,6 @@ g_process_set_check(gint check_period, gboolean (*check_fn)(void)) process_opts.check_fn = check_fn; } - -/** - * g_process_message: - * @fmt: format string - * @...: arguments to @fmt - * - * This function sends a message to the client preferring to use the stderr - * channel as long as it is available and switching to using syslog() if it - * isn't. Generally the stderr channell will be available in the startup - * process and in the beginning of the first startup in the - * supervisor/daemon processes. Later on the stderr fd will be closed and we - * have to fall back to using the system log. - **/ -void -g_process_message(const gchar *fmt, ...) -{ - gchar buf[2048]; - va_list ap; - - va_start(ap, fmt); - g_vsnprintf(buf, sizeof(buf), fmt, ap); - va_end(ap); - if (console_is_present(FALSE)) - fprintf(stderr, "%s: %s\n", process_opts.name, buf); - else - { - gchar name[32]; - - g_snprintf(name, sizeof(name), "%s/%s", process_kind == G_PK_SUPERVISOR ? "supervise" : "daemon", process_opts.name); - openlog(name, LOG_PID, LOG_DAEMON); - syslog(LOG_CRIT, "%s\n", buf); - closelog(); - } -} - /** * g_process_setup_console: * @@ -701,8 +666,8 @@ g_process_change_limits(void) } if (setrlimit(RLIMIT_NOFILE, &limit) < 0) - g_process_message("Error setting file number limit; limit='%d'; error='%s'", process_opts.fd_limit_min, - g_strerror(errno)); + console_printf("Error setting file number limit; limit='%d'; error='%s'", process_opts.fd_limit_min, + g_strerror(errno)); } /** @@ -731,7 +696,7 @@ g_process_set_dumpable(void) rc = prctl(PR_SET_DUMPABLE, 1, 0, 0, 0); if (rc < 0) - g_process_message("Cannot set process to be dumpable; error='%s'", g_strerror(errno)); + console_printf("Cannot set process to be dumpable; error='%s'", g_strerror(errno)); } #endif } @@ -753,7 +718,7 @@ g_process_enable_core(void) limit.rlim_cur = limit.rlim_max = RLIM_INFINITY; if (setrlimit(RLIMIT_CORE, &limit) < 0) - g_process_message("Error setting core limit to infinity; error='%s'", g_strerror(errno)); + console_printf("Error setting core limit to infinity; error='%s'", g_strerror(errno)); } } @@ -811,7 +776,7 @@ g_process_write_pidfile(pid_t pid) } else { - g_process_message("Error creating pid file; file='%s', error='%s'", pidfile, g_strerror(errno)); + console_printf("Error creating pid file; file='%s', error='%s'", pidfile, g_strerror(errno)); } } @@ -831,7 +796,7 @@ g_process_remove_pidfile(void) if (unlink(pidfile) < 0) { - g_process_message("Error removing pid file; file='%s', error='%s'", pidfile, g_strerror(errno)); + console_printf("Error removing pid file; file='%s', error='%s'", pidfile, g_strerror(errno)); } } @@ -851,13 +816,13 @@ g_process_change_root(void) { if (chroot(process_opts.chroot_dir) < 0) { - g_process_message("Error in chroot(); chroot='%s', error='%s'\n", process_opts.chroot_dir, g_strerror(errno)); + console_printf("Error in chroot(); chroot='%s', error='%s'\n", process_opts.chroot_dir, g_strerror(errno)); return FALSE; } if (chdir("/") < 0) { - g_process_message("Error in chdir() after chroot; chroot='%s', error='%s'\n", process_opts.chroot_dir, - g_strerror(errno)); + console_printf("Error in chdir() after chroot; chroot='%s', error='%s'\n", process_opts.chroot_dir, + g_strerror(errno)); return FALSE; } } @@ -891,14 +856,14 @@ g_process_change_user(void) { if (setgid((gid_t) process_opts.gid) < 0) { - g_process_message("Error in setgid(); group='%s', gid='%d', error='%s'", process_opts.group, process_opts.gid, - g_strerror(errno)); + console_printf("Error in setgid(); group='%s', gid='%d', error='%s'", process_opts.group, process_opts.gid, + g_strerror(errno)); if (getuid() == 0) return FALSE; } if (process_opts.user && initgroups(process_opts.user, (gid_t) process_opts.gid) < 0) { - g_process_message("Error in initgroups(); user='%s', error='%s'", process_opts.user, g_strerror(errno)); + console_printf("Error in initgroups(); user='%s', error='%s'", process_opts.user, g_strerror(errno)); if (getuid() == 0) return FALSE; } @@ -908,8 +873,8 @@ g_process_change_user(void) { if (setuid((uid_t) process_opts.uid) < 0) { - g_process_message("Error in setuid(); user='%s', uid='%d', error='%s'", process_opts.user, process_opts.uid, - g_strerror(errno)); + console_printf("Error in setuid(); user='%s', uid='%d', error='%s'", process_opts.user, process_opts.uid, + g_strerror(errno)); if (getuid() == 0) return FALSE; } @@ -938,7 +903,7 @@ g_process_change_caps(void) if (cap == NULL) { - g_process_message("Error parsing capabilities: %s", process_opts.caps); + console_printf("Error parsing capabilities: %s", process_opts.caps); g_process_disable_caps(); return FALSE; } @@ -946,7 +911,7 @@ g_process_change_caps(void) { if (cap_set_proc(cap) == -1) { - g_process_message("Error setting capabilities, capability management disabled; error='%s'", g_strerror(errno)); + console_printf("Error setting capabilities, capability management disabled; error='%s'", g_strerror(errno)); g_process_disable_caps(); } @@ -972,13 +937,13 @@ g_process_resolve_names(void) gboolean result = TRUE; if (process_opts.user && !resolve_user(process_opts.user, &process_opts.uid)) { - g_process_message("Error resolving user; user='%s'", process_opts.user); + console_printf("Error resolving user; user='%s'", process_opts.user); process_opts.uid = -1; result = FALSE; } if (process_opts.group && !resolve_group(process_opts.group, &process_opts.gid)) { - g_process_message("Error resolving group; group='%s'", process_opts.group); + console_printf("Error resolving group; group='%s'", process_opts.group); process_opts.gid = -1; result = FALSE; } @@ -1007,8 +972,10 @@ g_process_change_dir(void) cwd = get_installation_path_for(SYSLOG_NG_PATH_PIDFILEDIR); if (cwd) - if (chdir(cwd)) - g_process_message("Error changing to directory=%s, errcode=%d", cwd, errno); + { + if (chdir(cwd)) + console_printf("Error changing to directory=%s, errcode=%d", cwd, errno); + } } /* this check is here to avoid having to change directory early in the startup process */ @@ -1018,8 +985,8 @@ g_process_change_dir(void) if (!getcwd(buf, sizeof(buf))) strncpy(buf, "unable-to-query", sizeof(buf)); - g_process_message("Unable to write to current directory, core dumps will not be generated; dir='%s', error='%s'", buf, - g_strerror(errno)); + console_printf("Unable to write to current directory, core dumps will not be generated; dir='%s', error='%s'", buf, + g_strerror(errno)); } } @@ -1159,14 +1126,14 @@ g_process_perform_supervise(void) { if (pipe(init_result_pipe) != 0) { - g_process_message("Error daemonizing process, cannot open pipe; error='%s'", g_strerror(errno)); + console_printf("Error daemonizing process, cannot open pipe; error='%s'", g_strerror(errno)); g_process_startup_failed(1, TRUE); } /* fork off a child process */ if ((pid = fork()) < 0) { - g_process_message("Error forking child process; error='%s'", g_strerror(errno)); + console_printf("Error forking child process; error='%s'", g_strerror(errno)); g_process_startup_failed(1, TRUE); } else if (pid != 0) @@ -1203,8 +1170,8 @@ g_process_perform_supervise(void) i++; } if (i == 6) - g_process_message("Initialization failed but the daemon did not exit, even when forced to, trying to recover; pid='%d'", - pid); + console_printf("Initialization failed but the daemon did not exit, even when forced to, trying to recover; pid='%d'", + pid); continue; } @@ -1226,7 +1193,7 @@ g_process_perform_supervise(void) if (!exited) { gint j = 0; - g_process_message("Daemon deadlock detected, killing process;"); + console_printf("Daemon deadlock detected, killing process;"); deadlock = TRUE; while (j < 6 && waitpid(pid, &rc, WNOHANG) == 0) @@ -1237,7 +1204,7 @@ g_process_perform_supervise(void) j++; } if (j == 6) - g_process_message("The daemon did not exit after deadlock, even when forced to, trying to recover; pid='%d'", pid); + console_printf("The daemon did not exit after deadlock, even when forced to, trying to recover; pid='%d'", pid); } } else @@ -1257,14 +1224,14 @@ g_process_perform_supervise(void) switch (npid) { case -1: - g_process_message("Could not fork for external notification; reason='%s'", strerror(errno)); + console_printf("Could not fork for external notification; reason='%s'", strerror(errno)); break; case 0: switch(fork()) { case -1: - g_process_message("Could not fork for external notification; reason='%s'", strerror(errno)); + console_printf("Could not fork for external notification; reason='%s'", strerror(errno)); exit(1); break; case 0: @@ -1292,7 +1259,7 @@ g_process_perform_supervise(void) argbuf, (deadlock || !WIFSIGNALED(rc) || WTERMSIG(rc) != SIGKILL) ? "restarting" : "not-restarting", (gchar *) NULL); - g_process_message("Could not execute external notification; reason='%s'", strerror(errno)); + console_printf("Could not execute external notification; reason='%s'", strerror(errno)); break; default: @@ -1306,18 +1273,18 @@ g_process_perform_supervise(void) } if (deadlock || !WIFSIGNALED(rc) || WTERMSIG(rc) != SIGKILL) { - g_process_message("Daemon exited due to a deadlock/signal/failure, restarting; exitcode='%d'", rc); + console_printf("Daemon exited due to a deadlock/signal/failure, restarting; exitcode='%d'", rc); sleep(1); } else { - g_process_message("Daemon was killed, not restarting; exitcode='%d'", rc); + console_printf("Daemon was killed, not restarting; exitcode='%d'", rc); break; } } else { - g_process_message("Daemon exited gracefully, not restarting; exitcode='%d'", rc); + console_printf("Daemon exited gracefully, not restarting; exitcode='%d'", rc); break; } } @@ -1362,13 +1329,13 @@ g_process_start(void) /* no supervisor, sends result to startup process directly */ if (pipe(init_result_pipe) != 0) { - g_process_message("Error daemonizing process, cannot open pipe; error='%s'", g_strerror(errno)); + console_printf("Error daemonizing process, cannot open pipe; error='%s'", g_strerror(errno)); exit(1); } if ((pid = fork()) < 0) { - g_process_message("Error forking child process; error='%s'", g_strerror(errno)); + console_printf("Error forking child process; error='%s'", g_strerror(errno)); exit(1); } else if (pid != 0) @@ -1399,13 +1366,13 @@ g_process_start(void) /* full blown startup/supervisor/daemon */ if (pipe(startup_result_pipe) != 0) { - g_process_message("Error daemonizing process, cannot open pipe; error='%s'", g_strerror(errno)); + console_printf("Error daemonizing process, cannot open pipe; error='%s'", g_strerror(errno)); exit(1); } /* first fork off supervisor process */ if ((pid = fork()) < 0) { - g_process_message("Error forking child process; error='%s'", g_strerror(errno)); + console_printf("Error forking child process; error='%s'", g_strerror(errno)); exit(1); } else if (pid != 0) diff --git a/lib/gprocess.h b/lib/gprocess.h index 534be0bba2..e699782e7a 100644 --- a/lib/gprocess.h +++ b/lib/gprocess.h @@ -58,8 +58,6 @@ typedef gpointer cap_t; #endif -void g_process_message(const gchar *fmt, ...) G_GNUC_PRINTF(1, 2); - void g_process_set_mode(GProcessMode mode); GProcessMode g_process_get_mode(void); void g_process_set_name(const gchar *name); diff --git a/syslog-ng/main.c b/syslog-ng/main.c index 8fd03bc804..e446a0aa04 100644 --- a/syslog-ng/main.c +++ b/syslog-ng/main.c @@ -236,7 +236,7 @@ main(int argc, char *argv[]) GOptionContext *ctx; GError *error = NULL; - console_global_init(); + console_global_init("syslog-ng"); MainLoop *main_loop = main_loop_get_instance(); z_mem_trace_init("syslog-ng.trace"); @@ -295,7 +295,7 @@ main(int argc, char *argv[]) if (debug_flag && !log_stderr) { - g_process_message("The -d/--debug option no longer implies -e/--stderr, if you want to redirect internal() source to stderr please also include -e/--stderr option"); + fprintf(stderr, "The -d/--debug option no longer implies -e/--stderr, if you want to redirect internal() source to stderr please also include -e/--stderr option"); } gboolean exit_before_main_loop_run = main_loop_options.syntax_only From a136792d882d45898da23847b23f84dec94123c6 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Tue, 15 Oct 2024 10:13:10 +0200 Subject: [PATCH 30/42] gprocess: add fatal signal logging with a backtrace Signed-off-by: Balazs Scheidler --- lib/CMakeLists.txt | 2 + lib/Makefile.am | 2 + lib/gprocess.c | 11 ++ lib/stackdump.c | 245 +++++++++++++++++++++++++++++++++++++++++++++ lib/stackdump.h | 31 ++++++ 5 files changed, 291 insertions(+) create mode 100644 lib/stackdump.c create mode 100644 lib/stackdump.h diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 050d0a2f23..ba23d749a5 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -153,6 +153,7 @@ set (LIB_HEADERS serialize.h service-management.h seqnum.h + stackdump.h str-format.h str-utils.h syslog-names.h @@ -249,6 +250,7 @@ set(LIB_SOURCES scratch-buffers.c serialize.c service-management.c + stackdump.c str-format.c str-utils.c syslog-names.c diff --git a/lib/Makefile.am b/lib/Makefile.am index 995360761b..2e65e2562f 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -172,6 +172,7 @@ pkginclude_HEADERS += \ lib/service-management.h \ lib/seqnum.h \ lib/signal-handler.h \ + lib/stackdump.h \ lib/str-format.h \ lib/str-utils.h \ lib/syslog-names.h \ @@ -264,6 +265,7 @@ lib_libsyslog_ng_la_SOURCES = \ lib/scratch-buffers.c \ lib/serialize.c \ lib/service-management.c \ + lib/stackdump.c \ lib/str-format.c \ lib/str-utils.c \ lib/syslog-names.c \ diff --git a/lib/gprocess.c b/lib/gprocess.c index 68cdd1a452..0e69c2c9ed 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -27,6 +27,7 @@ #include "messages.h" #include "reloc.h" #include "console.h" +#include "stackdump.h" #include #include @@ -1306,6 +1307,14 @@ g_process_perform_supervise(void) exit(0); } + +static void +g_process_setup_fatal_signal_handler(void) +{ + stackdump_setup_signal(SIGSEGV); + stackdump_setup_signal(SIGABRT); +} + /** * g_process_start: * @@ -1409,6 +1418,8 @@ g_process_start(void) g_assert_not_reached(); } + g_process_setup_fatal_signal_handler(); + /* daemon process, we should return to the caller to perform work */ /* Only call setsid() for backgrounded processes. */ if (process_opts.mode != G_PM_FOREGROUND) diff --git a/lib/stackdump.c b/lib/stackdump.c new file mode 100644 index 0000000000..2e7f323168 --- /dev/null +++ b/lib/stackdump.c @@ -0,0 +1,245 @@ +/* + * Copyright (c) 2024 Balázs Scheidler + * Copyright (c) 2024 Axoflow + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ +#include "stackdump.h" +#include "console.h" + +#include +#include +#include +#include + +#ifdef __linux__ + +/* this is Linux only for now */ + +static void +_stackdump_print_stack(gpointer stack_pointer) +{ + guint8 *p = stack_pointer; + + for (gint i = 0; i < 16; i++) + { + gchar line[51] = {0}; + for (gint j = 0; j < 8; j++) + { + sprintf(&line[j*3], "%02x ", (guint) *(p+j)); + } + line[8*3] = ' '; + for (gint j = 8; j < 16; j++) + { + sprintf(&line[j*3 + 1], "%02x ", (guint) *(p+j)); + } + + console_printf("Stack %p: %s", p, line); + p += 16; + } +} + +/* should not do any allocation to allow this to work even if our heap is corrupted */ +static void +_stackdump_print_maps(void) +{ + int fd; + + console_printf("Maps file follows"); + fd = open("/proc/self/maps", O_RDONLY); + if (fd < 0) + { + console_printf("Error opening /proc/self/maps"); + return; + } + + gchar buf[1024]; + int rc; + gchar *p, *eol; + gint avail, end = 0; + + while (1) + { + avail = sizeof(buf) - end; + rc = read(fd, buf + end, avail); + + if (rc < 0) + break; + + end += rc; + + if (rc == 0) + break; + + p = buf; + while (*p && p < (buf + end)) + { + eol = memchr(p, '\n', buf + end - p); + if (eol) + { + *eol = 0; + console_printf("%s", p); + p = eol + 1; + } + else + { + end = end - (p - buf); + memmove(buf, p, end); + break; + } + } + } + if (end) + console_printf("%.*s", end, buf); + close(fd); +} + +static inline void +_stackdump_print_backtrace(void) +{ + void *bt[256]; + gint count; + + count = backtrace(bt, 256); + console_printf("Raw backtrace dump, count=%d", count); + for (gint i = 0; i < count; i++) + { + console_printf("[%d]: %p", i, bt[i]); + } + if (count) + { + gchar **symbols; + + console_printf("Symbol backtrace dump, count=%d", count); + symbols = backtrace_symbols(bt, count); + for (gint i = 0; i < count; i++) + { + console_printf("[%d]: %s", i, symbols[i]); + } + } +} + + +#ifdef __x86_64__ +/**************************************************************************** + * + * + * x86_64 support + * + * + ****************************************************************************/ + +void +_stackdump_print_registers(struct sigcontext *p) +{ + console_printf( + "Registers: " + "rax=%016lx rbx=%016lx rcx=%016lx rdx=%016lx rsi=%016lx rdi=%016lx " + "rbp=%016lx rsp=%016lx r8=%016lx r9=%016lx r10=%016lx r11=%016lx " + "r12=%016lx r13=%016lx r14=%016lx r15=%016lx rip=%016lx", + p->rax, p->rbx, p->rcx, p->rdx, p->rsi, p->rdi, p->rbp, p->rsp, p->r8, p->r9, p->r10, p->r11, p->r12, p->r13, p->r14, p->r15, p->rip); + _stackdump_print_stack((gpointer) p->rsp); +} + +#elif __x86__ +/**************************************************************************** + * + * + * i386 support + * + * + ****************************************************************************/ + +void +_stackdump_print_registers(struct sigcontext *p) +{ + console_printf( + "Registers: eax=%08lx ebx=%08lx ecx=%08lx edx=%08lx esi=%08lx edi=%08lx ebp=%08lx esp=%08lx eip=%08lx", + p->eax, p->ebx, p->ecx, p->edx, p->esi, p->edi, p->ebp, p->esp, p->eip); + _stackdump_print_stack((gpointer) p->esp); +} + +#else +/**************************************************************************** + * + * + * unsupported platform + * + * + ****************************************************************************/ + +static void +_stackdump_print_registers(struct sigcontext *p) +{ +} + +#endif + + +static inline void +_stackdump_log(struct sigcontext *p) +{ + /* the order is important here, even if it might be illogical. The + * backtrace function is the most fragile (as backtrace_symbols() may + * allocate memory). Let's log everything else first, and then we can + * produce the backtrace, which is potentially causing another crash. */ + + _stackdump_print_registers(p); + _stackdump_print_maps(); + _stackdump_print_backtrace(); +} + +static void +_fatal_signal_handler(int signo, siginfo_t *info, void *uc) +{ + struct ucontext_t *ucontext = (struct ucontext_t *) uc; + struct sigcontext *p = (struct sigcontext *) &ucontext->uc_mcontext; + struct sigaction act; + + memset(&act, 0, sizeof(act)); + act.sa_handler = SIG_DFL; + sigaction(signo, &act, NULL); + + console_printf("Fatal signal received, stackdump follows, signal=%d", signo); + _stackdump_log(p); + /* let's get a stacktrace as well */ + kill(getpid(), signo); +} + +void +stackdump_setup_signal(gint signal_number) +{ + struct sigaction act; + + memset(&act, 0, sizeof(act)); + act.sa_flags = SA_SIGINFO; + act.sa_sigaction = _fatal_signal_handler; + + sigaction(signal_number, &act, NULL); +} + +#else + +void +stackdump_setup_signal(gint signal_number) +{ +} + +#endif diff --git a/lib/stackdump.h b/lib/stackdump.h new file mode 100644 index 0000000000..7942f651c9 --- /dev/null +++ b/lib/stackdump.h @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024 Balázs Scheidler + * Copyright (c) 2024 Axoflow + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ +#ifndef STACKDUMP_H_INCLUDED +#define STACKDUMP_H_INCLUDED + +#include "syslog-ng.h" + +void stackdump_setup_signal(gint signal_number); + +#endif From 8949ea5cea221d9951db5a45be14a48c174edd49 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Mon, 14 Oct 2024 19:02:09 +0200 Subject: [PATCH 31/42] logreader: add exit-on-eof flag() With the exit-on-eof flag, you can request syslog-ng to exit whenever the first EOF is hit on the specific source, similarly to how the stdin() source behaves. Signed-off-by: Balazs Scheidler --- lib/logreader.c | 6 ++++ lib/logreader.h | 1 + modules/affile/affile-source.c | 2 +- modules/affile/affile-source.h | 1 + modules/affile/file-reader.c | 2 -- modules/affile/file-reader.h | 1 - modules/affile/named-pipe.c | 58 +++++++++++++++++++++++++--------- modules/affile/named-pipe.h | 2 +- modules/affile/stdin.c | 2 +- 9 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/logreader.c b/lib/logreader.c index f93393542d..0f43d34752 100644 --- a/lib/logreader.c +++ b/lib/logreader.c @@ -389,6 +389,11 @@ log_reader_work_finished(void *s, gpointer arg) self->notify_code = 0; log_pipe_notify(self->control, notify_code, self); + + if (notify_code == NC_CLOSE && (self->options->flags & LR_EXIT_ON_EOF)) + { + cfg_shutdown(log_pipe_get_config(s)); + } } if ((self->super.super.flags & PIF_INITIALIZED) && self->proto) { @@ -872,6 +877,7 @@ CfgFlagHandler log_reader_flag_handlers[] = { "empty-lines", CFH_SET, offsetof(LogReaderOptions, flags), LR_EMPTY_LINES }, { "threaded", CFH_SET, offsetof(LogReaderOptions, flags), LR_THREADED }, { "ignore-aux-data", CFH_SET, offsetof(LogReaderOptions, flags), LR_IGNORE_AUX_DATA }, + { "exit-on-eof", CFH_SET, offsetof(LogReaderOptions, flags), LR_EXIT_ON_EOF }, { NULL }, }; diff --git a/lib/logreader.h b/lib/logreader.h index 6c633314c8..2d3b51f75e 100644 --- a/lib/logreader.h +++ b/lib/logreader.h @@ -39,6 +39,7 @@ #define LR_EMPTY_LINES 0x0004 #define LR_IGNORE_AUX_DATA 0x0008 #define LR_THREADED 0x0040 +#define LR_EXIT_ON_EOF 0x0080 /* options */ diff --git a/modules/affile/affile-source.c b/modules/affile/affile-source.c index 6d84d33a06..db8d605875 100644 --- a/modules/affile/affile-source.c +++ b/modules/affile/affile-source.c @@ -91,7 +91,7 @@ affile_sd_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) log_src_driver_queue_method(s, msg, path_options); } -static gboolean +gboolean affile_sd_init(LogPipe *s) { AFFileSourceDriver *self = (AFFileSourceDriver *) s; diff --git a/modules/affile/affile-source.h b/modules/affile/affile-source.h index 50e9e1257e..1e673239e8 100644 --- a/modules/affile/affile-source.h +++ b/modules/affile/affile-source.h @@ -41,6 +41,7 @@ typedef struct _AFFileSourceDriver gsize transport_name_len; } AFFileSourceDriver; +gboolean affile_sd_init(LogPipe *s); void affile_sd_set_transport_name(AFFileSourceDriver *s, const gchar *transport_name); AFFileSourceDriver *affile_sd_new_instance(gchar *filename, GlobalConfig *cfg); LogDriver *affile_sd_new(gchar *filename, GlobalConfig *cfg); diff --git a/modules/affile/file-reader.c b/modules/affile/file-reader.c index 4c6af0ae29..7439d6dd53 100644 --- a/modules/affile/file-reader.c +++ b/modules/affile/file-reader.c @@ -267,8 +267,6 @@ file_reader_notify_method(LogPipe *s, gint notify_code, gpointer user_data) switch (notify_code) { case NC_CLOSE: - if (self->options->exit_on_eof) - cfg_shutdown(log_pipe_get_config(s)); break; case NC_FILE_MOVED: diff --git a/modules/affile/file-reader.h b/modules/affile/file-reader.h index 0091908242..05a8795c05 100644 --- a/modules/affile/file-reader.h +++ b/modules/affile/file-reader.h @@ -32,7 +32,6 @@ typedef struct _FileReaderOptions gint multi_line_timeout; gboolean restore_state; LogReaderOptions reader_options; - gboolean exit_on_eof; } FileReaderOptions; typedef struct _FileReader diff --git a/modules/affile/named-pipe.c b/modules/affile/named-pipe.c index 9f203c8721..86b2248d49 100644 --- a/modules/affile/named-pipe.c +++ b/modules/affile/named-pipe.c @@ -35,6 +35,12 @@ #include #include +typedef struct _FileOpenerNamedPipe +{ + FileOpener super; + gboolean suppress_eof; +} FileOpenerNamedPipe; + static gboolean _prepare_open(FileOpener *self, const gchar *name) { @@ -64,11 +70,19 @@ _prepare_open(FileOpener *self, const gchar *name) } static gint -_get_open_flags(FileOpener *self, FileDirection dir) +_get_open_flags(FileOpener *s, FileDirection dir) { + FileOpenerNamedPipe *self = (FileOpenerNamedPipe *) s; switch (dir) { case AFFILE_DIR_READ: + /* if a named pipe is opened for read write, we won't get an EOF, as + * there's always a writer (us, having opened in RW mode). EOF is only + * indicated if no writers remain */ + + if (self->suppress_eof) + return (O_RDWR | O_NOCTTY | O_NONBLOCK | O_LARGEFILE); + return (O_RDONLY | O_NOCTTY | O_NONBLOCK | O_LARGEFILE); case AFFILE_DIR_WRITE: return (O_RDWR | O_NOCTTY | O_NONBLOCK | O_LARGEFILE); default: @@ -77,11 +91,13 @@ _get_open_flags(FileOpener *self, FileDirection dir) } static LogTransport * -_construct_transport(FileOpener *self, gint fd) +_construct_transport(FileOpener *s, gint fd) { + FileOpenerNamedPipe *self = (FileOpenerNamedPipe *) s; LogTransport *transport = log_transport_pipe_new(fd); - transport->read = log_transport_file_read_and_ignore_eof_method; + if (self->suppress_eof) + transport->read = log_transport_file_read_and_ignore_eof_method; return transport; } @@ -106,16 +122,29 @@ pipe_sd_set_create_dirs(LogDriver *s, gboolean create_dirs) } FileOpener * -file_opener_for_named_pipes_new(void) +file_opener_for_named_pipes_new(gboolean suppress_eof) { - FileOpener *self = file_opener_new(); - - self->prepare_open = _prepare_open; - self->get_open_flags = _get_open_flags; - self->construct_transport = _construct_transport; - self->construct_src_proto = _construct_src_proto; - self->construct_dst_proto = _construct_dst_proto; - return self; + FileOpenerNamedPipe *self = g_new0(FileOpenerNamedPipe, 1); + + file_opener_init_instance(&self->super); + + self->super.prepare_open = _prepare_open; + self->super.get_open_flags = _get_open_flags; + self->super.construct_transport = _construct_transport; + self->super.construct_src_proto = _construct_src_proto; + self->super.construct_dst_proto = _construct_dst_proto; + + self->suppress_eof = suppress_eof; + return &self->super; +} + +static gboolean +_init(LogPipe *s) +{ + AFFileSourceDriver *self = (AFFileSourceDriver *) s; + if (!self->file_opener) + self->file_opener = file_opener_for_named_pipes_new((self->file_reader_options.reader_options.flags & LR_EXIT_ON_EOF) == 0); + return affile_sd_init(s); } LogDriver * @@ -123,6 +152,7 @@ pipe_sd_new(gchar *filename, GlobalConfig *cfg) { AFFileSourceDriver *self = affile_sd_new_instance(filename, cfg); + self->super.super.super.init = _init; self->file_reader_options.reader_options.super.stats_source = stats_register_type("pipe"); if (cfg_is_config_version_older(cfg, VERSION_VALUE_3_2)) @@ -137,8 +167,6 @@ pipe_sd_new(gchar *filename, GlobalConfig *cfg) self->file_reader_options.reader_options.parse_options.flags &= ~LP_EXPECT_HOSTNAME; } - self->file_opener = file_opener_for_named_pipes_new(); - affile_sd_set_transport_name(self, "local+pipe"); return &self->super.super; } @@ -149,6 +177,6 @@ pipe_dd_new(LogTemplate *filename_template, GlobalConfig *cfg) AFFileDestDriver *self = affile_dd_new_instance(filename_template, cfg); self->writer_options.stats_source = stats_register_type("pipe"); - self->file_opener = file_opener_for_named_pipes_new(); + self->file_opener = file_opener_for_named_pipes_new(FALSE); return &self->super.super; } diff --git a/modules/affile/named-pipe.h b/modules/affile/named-pipe.h index f988f649f0..65fddbb8a2 100644 --- a/modules/affile/named-pipe.h +++ b/modules/affile/named-pipe.h @@ -30,7 +30,7 @@ void pipe_sd_set_create_dirs(LogDriver *s, gboolean create_dirs); -FileOpener *file_opener_for_named_pipes_new(void); +FileOpener *file_opener_for_named_pipes_new(gboolean open_for_readonly); LogDriver *pipe_sd_new(gchar *filename, GlobalConfig *cfg); LogDriver *pipe_dd_new(LogTemplate *filename_template, GlobalConfig *cfg); diff --git a/modules/affile/stdin.c b/modules/affile/stdin.c index 9dcc07d24b..b2be7e1be9 100644 --- a/modules/affile/stdin.c +++ b/modules/affile/stdin.c @@ -60,7 +60,7 @@ stdin_sd_new(GlobalConfig *cfg) { AFFileSourceDriver *self = affile_sd_new_instance("-", cfg); - self->file_reader_options.exit_on_eof = TRUE; + self->file_reader_options.reader_options.flags |= LR_EXIT_ON_EOF; self->file_reader_options.reader_options.super.stats_source = stats_register_type("stdin"); self->file_opener = file_opener_for_stdin_new(); affile_sd_set_transport_name(self, "local+stdin"); From 5ce77a264df8aed39b6e9d5975f8d6d1aa04622b Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 12 Oct 2024 07:38:36 +0200 Subject: [PATCH 32/42] cfg-source: add num_lines argument to _print_underlined_source_block() Instead of using "NULL" as the indicator for the end of the lines to print, just pass the number of entries in the lines array. Signed-off-by: Balazs Scheidler --- lib/cfg-source.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/cfg-source.c b/lib/cfg-source.c index 4695c40225..505cb7d868 100644 --- a/lib/cfg-source.c +++ b/lib/cfg-source.c @@ -55,13 +55,13 @@ _print_underline(const gchar *line, gint whitespace_before, gint number_of_caret } static void -_print_underlined_source_block(const CFG_LTYPE *yylloc, gchar **lines, gint error_index) +_print_underlined_source_block(const CFG_LTYPE *yylloc, gchar **lines, gsize num_lines, gint error_index) { gint line_ndx; gchar line_prefix[12]; gint error_length = yylloc->last_line - yylloc->first_line + 1; - for (line_ndx = 0; lines[line_ndx]; line_ndx++) + for (line_ndx = 0; line_ndx < num_lines; line_ndx++) { gint lineno = yylloc->first_line + line_ndx - error_index; const gchar *line = lines[line_ndx]; @@ -84,8 +84,6 @@ _print_underlined_source_block(const CFG_LTYPE *yylloc, gchar **lines, gint erro multi_line ? strlen(&line[yylloc->first_column]) + 1 : yylloc->last_column - yylloc->first_column); } - else if (line_ndx >= error_index + CONTEXT) - break; } } @@ -116,11 +114,10 @@ _report_file_location(const gchar *filename, const CFG_LTYPE *yylloc) /* NOTE: do we have the appropriate number of lines? */ if (lineno <= yylloc->first_line) goto exit; - g_ptr_array_add(context, NULL); fclose(f); } if (context->len > 0) - _print_underlined_source_block(yylloc, (gchar **) context->pdata, error_index); + _print_underlined_source_block(yylloc, (gchar **) context->pdata, context->len, error_index); exit: g_free(buf); @@ -144,7 +141,7 @@ _report_buffer_location(const gchar *buffer_content, const CFG_LTYPE *file_lloc, error_index += start; start = 0; } - _print_underlined_source_block(file_lloc, &lines[start], error_index); + _print_underlined_source_block(file_lloc, &lines[start], num_lines - start, error_index); exit: g_strfreev(lines); From 00996cbb53267f5560f056fa595e491f1c086f59 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 12 Oct 2024 11:54:20 +0200 Subject: [PATCH 33/42] cfg-source: refactor source file printing to use line numbers Signed-off-by: Balazs Scheidler --- lib/cfg-source.c | 67 +++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/lib/cfg-source.c b/lib/cfg-source.c index 505cb7d868..889453839f 100644 --- a/lib/cfg-source.c +++ b/lib/cfg-source.c @@ -55,25 +55,24 @@ _print_underline(const gchar *line, gint whitespace_before, gint number_of_caret } static void -_print_underlined_source_block(const CFG_LTYPE *yylloc, gchar **lines, gsize num_lines, gint error_index) +_print_underlined_source_block(const CFG_LTYPE *yylloc, gchar **lines, gsize num_lines, gint start_line) { gint line_ndx; gchar line_prefix[12]; - gint error_length = yylloc->last_line - yylloc->first_line + 1; for (line_ndx = 0; line_ndx < num_lines; line_ndx++) { - gint lineno = yylloc->first_line + line_ndx - error_index; + gint lineno = start_line + line_ndx; const gchar *line = lines[line_ndx]; gint line_len = strlen(line); gboolean line_ends_with_newline = line_len > 0 && line[line_len - 1] == '\n'; _format_source_prefix(line_prefix, sizeof(line_prefix), lineno, - line_ndx >= error_index && line_ndx < error_index + error_length); + lineno >= yylloc->first_line && lineno <= yylloc->last_line); fprintf(stderr, "%-8s%s%s", line_prefix, line, line_ends_with_newline ? "" : "\n"); - if (line_ndx == error_index) + if (lineno == yylloc->first_line) { /* print the underline right below the source line we just printed */ fprintf(stderr, "%-8s", line_prefix); @@ -88,14 +87,20 @@ _print_underlined_source_block(const CFG_LTYPE *yylloc, gchar **lines, gsize num } static void -_report_file_location(const gchar *filename, const CFG_LTYPE *yylloc) +_report_file_location(const gchar *filename, const CFG_LTYPE *yylloc, gint start_line) { FILE *f; gint lineno = 0; gsize buflen = 65520; gchar *buf = g_malloc(buflen); GPtrArray *context = g_ptr_array_new(); - gint error_index = 0; + gint end_line = start_line + 2*CONTEXT; + + if (start_line <= 0) + { + start_line = yylloc->first_line > CONTEXT ? yylloc->first_line - CONTEXT : 1; + end_line = yylloc->first_line + CONTEXT; + } f = fopen(filename, "r"); if (f) @@ -103,48 +108,52 @@ _report_file_location(const gchar *filename, const CFG_LTYPE *yylloc) while (fgets(buf, buflen, f)) { lineno++; - if (lineno > (gint) yylloc->first_line + CONTEXT) + if (lineno > end_line) break; - else if (lineno < (gint) yylloc->first_line - CONTEXT) + else if (lineno < start_line) continue; - else if (lineno == yylloc->first_line) - error_index = context->len; g_ptr_array_add(context, g_strdup(buf)); } - /* NOTE: do we have the appropriate number of lines? */ - if (lineno <= yylloc->first_line) - goto exit; fclose(f); } if (context->len > 0) - _print_underlined_source_block(yylloc, (gchar **) context->pdata, context->len, error_index); + _print_underlined_source_block(yylloc, (gchar **) context->pdata, context->len, start_line); -exit: g_free(buf); g_ptr_array_foreach(context, (GFunc) g_free, NULL); g_ptr_array_free(context, TRUE); } +/* this will report source content from the buffer, but use the line numbers + * of the file where the block was defined. + * + * buffer_* => tracks buffer related information + * file_* => tracks file related information + */ static void _report_buffer_location(const gchar *buffer_content, const CFG_LTYPE *file_lloc, const CFG_LTYPE *buf_lloc) { - gchar **lines = g_strsplit(buffer_content, "\n", buf_lloc->first_line + CONTEXT + 1); - gint num_lines = g_strv_length(lines); + gchar **buffer_lines = g_strsplit(buffer_content, "\n", buf_lloc->first_line + CONTEXT + 1); + gint buffer_num_lines = g_strv_length(buffer_lines); - if (num_lines <= buf_lloc->first_line) + if (buffer_num_lines <= buf_lloc->first_line) goto exit; - gint start = buf_lloc->first_line - 1 - CONTEXT; - gint error_index = CONTEXT; - if (start < 0) - { - error_index += start; - start = 0; - } - _print_underlined_source_block(file_lloc, &lines[start], num_lines - start, error_index); + /* the line number in the file, which we report in the source dump, 1 based */ + gint range_backwards = CONTEXT; + if (file_lloc->first_line <= range_backwards) + range_backwards = file_lloc->first_line - 1; + + /* the index of the line in the buffer where we start printing 0-based */ + gint buffer_start_index = buf_lloc->first_line - 1 - range_backwards; + if (buffer_start_index < 0) + buffer_start_index = 0; + + _print_underlined_source_block(file_lloc, &buffer_lines[buffer_start_index], buffer_num_lines - buffer_start_index, + file_lloc->first_line - range_backwards); exit: - g_strfreev(lines); + g_strfreev(buffer_lines); } gboolean @@ -152,7 +161,7 @@ cfg_source_print_source_context(CfgLexer *lexer, CfgIncludeLevel *level, const C { if (level->include_type == CFGI_FILE) { - _report_file_location(yylloc->name, yylloc); + _report_file_location(yylloc->name, yylloc, -1); } else if (level->include_type == CFGI_BUFFER) { From e02b15a6661705215d2504c0aa0e07384e7abcf1 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 11 Oct 2024 22:07:01 +0200 Subject: [PATCH 34/42] cfg-source: add cfg_source_print_source_text() function Signed-off-by: Balazs Scheidler --- lib/cfg-source.c | 12 ++++++++++++ lib/cfg-source.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/lib/cfg-source.c b/lib/cfg-source.c index 889453839f..50fec12965 100644 --- a/lib/cfg-source.c +++ b/lib/cfg-source.c @@ -156,6 +156,18 @@ _report_buffer_location(const gchar *buffer_content, const CFG_LTYPE *file_lloc, g_strfreev(buffer_lines); } +gboolean +cfg_source_print_source_text(const gchar *filename, gint line, gint column, gint start_line) +{ + CFG_LTYPE yylloc = {0}; + + yylloc.name = filename; + yylloc.first_line = yylloc.last_line = line; + yylloc.first_column = yylloc.last_column = column; + _report_file_location(yylloc.name, &yylloc, start_line); + return TRUE; +} + gboolean cfg_source_print_source_context(CfgLexer *lexer, CfgIncludeLevel *level, const CFG_LTYPE *yylloc) { diff --git a/lib/cfg-source.h b/lib/cfg-source.h index 51e81fbd7e..55313b3939 100644 --- a/lib/cfg-source.h +++ b/lib/cfg-source.h @@ -26,6 +26,8 @@ #include "cfg-lexer.h" +gboolean cfg_source_print_source_text(const gchar *filename, gint line, gint column, gint offset); + /* These functions are only available during parsing */ gboolean cfg_source_print_source_context(CfgLexer *lexer, CfgIncludeLevel *level, const CFG_LTYPE *yylloc); gboolean cfg_source_extract_source_text(CfgLexer *lexer, const CFG_LTYPE *yylloc, GString *result); From f263108122111b43db9d3fb33d6a7ecbbc41eef5 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 11 Oct 2024 22:08:02 +0200 Subject: [PATCH 35/42] debugger: add source display using the cfg-source module Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 102 +++++++++++++++++++++++++++++----------- 1 file changed, 74 insertions(+), 28 deletions(-) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index 3f54b4be56..e8861b6e12 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -30,10 +30,12 @@ #include "timeutils/misc.h" #include "compat/time.h" #include "scratch-buffers.h" +#include "cfg-source.h" #include #include #include +#include struct _Debugger { @@ -41,11 +43,20 @@ struct _Debugger struct iv_signal sigint; MainLoop *main_loop; GlobalConfig *cfg; - gchar *command_buffer; - LogTemplate *display_template; + GThread *debugger_thread; BreakpointSite *breakpoint_site; struct timespec last_trace_event; - GThread *debugger_thread; + + /* user interface related state */ + gchar *command_buffer; + struct + { + gchar *filename; + gint line; + gint column; + gint list_start; + } current_location; + LogTemplate *display_template; }; static gboolean @@ -108,43 +119,41 @@ _display_msg_with_template_string(Debugger *self, LogMessage *msg, const gchar * } static void -_display_source_line(LogExprNode *expr_node) +_set_current_location(Debugger *self, LogExprNode *expr_node) { - FILE *f; - gint lineno = 1; - gchar buf[1024]; - - if (!expr_node || !expr_node->filename) - return; - - f = fopen(expr_node->filename, "r"); - if (f) + g_free(self->current_location.filename); + if (expr_node) { - while (fgets(buf, sizeof(buf), f) && lineno < expr_node->line) - lineno++; - if (lineno != expr_node->line) - buf[0] = 0; - fclose(f); + self->current_location.filename = g_strdup(expr_node->filename); + self->current_location.line = expr_node->line; + self->current_location.column = expr_node->column; + self->current_location.list_start = expr_node->line - 5; } else { - buf[0] = 0; + memset(&self->current_location, 0, sizeof(self->current_location)); } - printf("%-8d %s", expr_node->line, buf); - if (buf[0] == 0 || buf[strlen(buf) - 1] != '\n') - putc('\n', stdout); - fflush(stdout); } +static void +_display_source_line(Debugger *self) +{ + if (self->current_location.filename) + cfg_source_print_source_text(self->current_location.filename, self->current_location.line, + self->current_location.column, self->current_location.list_start); + else + puts("Unable to list source, no current location set"); +} static gboolean _cmd_help(Debugger *self, gint argc, gchar *argv[]) { printf("syslog-ng interactive console, the following commands are available\n\n" " help, h, or ? Display this help\n" - " info Display information about the current execution state\n" " continue or c Continue until the next breakpoint\n" " trace Display timing information as the message traverses the config\n" + " info Display information about the current execution state\n" + " list or l Display source code at the current location\n" " print, p Print the current log message\n" " drop, d Drop the current message\n" " quit, q Tell syslog-ng to exit\n" @@ -223,7 +232,7 @@ _cmd_info_pipe(Debugger *self, LogPipe *pipe) gchar buf[1024]; printf("LogPipe %p at %s\n", pipe, log_expr_node_format_location(pipe->expr_node, buf, sizeof(buf))); - _display_source_line(pipe->expr_node); + _display_source_line(self); return TRUE; } @@ -242,6 +251,38 @@ _cmd_info(Debugger *self, gint argc, gchar *argv[]) return TRUE; } +static gboolean +_cmd_list(Debugger *self, gint argc, gchar *argv[]) +{ + gint shift = 11; + if (argc >= 2) + { + if (strcmp(argv[1], "+") == 0) + shift = 11; + else if (strcmp(argv[1], "-") == 0) + shift = -11; + else if (strcmp(argv[1], ".") == 0) + { + shift = 0; + if (self->breakpoint_site) + _set_current_location(self, self->breakpoint_site->pipe->expr_node); + } + else if (isdigit(argv[1][0])) + { + gint target_lineno = atoi(argv[1]); + if (target_lineno <= 0) + target_lineno = 1; + self->current_location.list_start = target_lineno; + } + /* drop any arguments for repeated execution */ + _set_command(self, "l"); + } + _display_source_line(self); + if (shift) + self->current_location.list_start += shift; + return TRUE; +} + typedef gboolean (*DebuggerCommandFunc)(Debugger *self, gint argc, gchar *argv[]); struct @@ -258,6 +299,8 @@ struct { "c", _cmd_continue }, { "print", _cmd_print, .requires_breakpoint_site = TRUE }, { "p", _cmd_print, .requires_breakpoint_site = TRUE }, + { "list", _cmd_list, }, + { "l", _cmd_list, }, { "display", _cmd_display }, { "drop", _cmd_drop, .requires_breakpoint_site = TRUE }, { "quit", _cmd_quit }, @@ -298,6 +341,7 @@ debugger_register_command_fetcher(FetchCommandFunc fetcher) fetch_command_func = fetcher; } + static void _fetch_command(Debugger *self) { @@ -361,18 +405,19 @@ static void _handle_interactive_prompt(Debugger *self) { gchar buf[1024]; - LogPipe *current_pipe; if (self->breakpoint_site) { - current_pipe = self->breakpoint_site->pipe; + LogPipe *current_pipe = self->breakpoint_site->pipe; + _set_current_location(self, current_pipe->expr_node); 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_source_line(self); _display_msg_with_template(self, self->breakpoint_site->msg, self->display_template); } else { + _set_current_location(self, NULL); printf("Stopping on interrupt, message related commands are unavailable...\n"); } while (1) @@ -486,6 +531,7 @@ debugger_new(MainLoop *main_loop, GlobalConfig *cfg) void debugger_free(Debugger *self) { + g_free(self->current_location.filename); log_template_unref(self->display_template); tracer_free(self->tracer); g_free(self->command_buffer); From 34e2e1fa90f082dd4d5608ded3e39ab23d39c732 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 12 Oct 2024 08:28:53 +0200 Subject: [PATCH 36/42] debugger: extract _set_command() Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index e8861b6e12..766521f3a1 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -59,6 +59,14 @@ struct _Debugger LogTemplate *display_template; }; +static void +_set_command(Debugger *self, gchar *new_command) +{ + if (self->command_buffer) + g_free(self->command_buffer); + self->command_buffer = g_strdup(new_command); +} + static gboolean _format_nvpair(NVHandle handle, const gchar *name, @@ -349,16 +357,8 @@ _fetch_command(Debugger *self) command = fetch_command_func(); if (command && strlen(command) > 0) - { - if (self->command_buffer) - g_free(self->command_buffer); - self->command_buffer = command; - } - else - { - if (command) - g_free(command); - } + _set_command(self, command); + g_free(command); } static gboolean @@ -523,7 +523,7 @@ debugger_new(MainLoop *main_loop, GlobalConfig *cfg) self->tracer = tracer_new(cfg); self->cfg = cfg; self->display_template = log_template_new(cfg, NULL); - self->command_buffer = g_strdup("help"); + _set_command(self, "help"); log_template_compile(self->display_template, "$DATE $HOST $MSGHDR$MSG", NULL); return self; } From f67c4bf04ae8302c542739160ba623d3bd4acaba Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 13 Oct 2024 08:06:29 +0200 Subject: [PATCH 37/42] logpipe: add PIF_BREAKPOINT flag Signed-off-by: Balazs Scheidler --- lib/logpipe.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/logpipe.h b/lib/logpipe.h index 252ca014d3..8d7bcd03ae 100644 --- a/lib/logpipe.h +++ b/lib/logpipe.h @@ -75,6 +75,8 @@ /* sync filterx state and message in right before calling queue() */ #define PIF_SYNC_FILTERX 0x0200 +#define PIF_BREAKPOINT 0x0400 + /* private flags range, to be used by other LogPipe instances for their own purposes */ #define PIF_PRIVATE(x) ((x) << 16) From 709de3b58330200bafad6c23158d9723559a1ae4 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 11 Oct 2024 21:30:00 +0200 Subject: [PATCH 38/42] debugger: add debugger mode The mode field in debugger determines how we respond to trace and breakpoints and is used by the hook to trigger various debugging scenarios. Signed-off-by: Balazs Scheidler --- lib/debugger/debugger-main.c | 7 +++-- lib/debugger/debugger.c | 60 ++++++++++++++++++++++++------------ lib/debugger/debugger.h | 54 +++++++++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 24 deletions(-) diff --git a/lib/debugger/debugger-main.c b/lib/debugger/debugger-main.c index 7f2dc932d4..275043f659 100644 --- a/lib/debugger/debugger-main.c +++ b/lib/debugger/debugger-main.c @@ -32,10 +32,11 @@ static Debugger *current_debugger; static gboolean _pipe_hook(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options) { - if (msg->flags & LF_STATE_TRACING) - return debugger_perform_tracing(current_debugger, s, msg); - else + if (debugger_is_to_stop(current_debugger, s, msg)) return debugger_stop_at_breakpoint(current_debugger, s, msg); + else if (debugger_is_to_trace(current_debugger, s, msg)) + return debugger_perform_tracing(current_debugger, s, msg); + return TRUE; } gboolean diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index 766521f3a1..539150e6d3 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -39,6 +39,8 @@ struct _Debugger { + /* debugger_get_mode() assumes this comes as the first field */ + DebuggerMode mode; Tracer *tracer; struct iv_signal sigint; MainLoop *main_loop; @@ -169,11 +171,6 @@ _cmd_help(Debugger *self, gint argc, gchar *argv[]) return TRUE; } -static gboolean -_cmd_continue(Debugger *self, gint argc, gchar *argv[]) -{ - return FALSE; -} static gboolean _cmd_print(Debugger *self, gint argc, gchar *argv[]) @@ -218,21 +215,6 @@ _cmd_drop(Debugger *self, gint argc, gchar *argv[]) return FALSE; } -static gboolean -_cmd_trace(Debugger *self, gint argc, gchar *argv[]) -{ - self->breakpoint_site->msg->flags |= LF_STATE_TRACING; - return FALSE; -} - -static gboolean -_cmd_quit(Debugger *self, gint argc, gchar *argv[]) -{ - main_loop_exit(self->main_loop); - if (self->breakpoint_site) - self->breakpoint_site->drop = TRUE; - return FALSE; -} static gboolean _cmd_info_pipe(Debugger *self, LogPipe *pipe) @@ -291,6 +273,44 @@ _cmd_list(Debugger *self, gint argc, gchar *argv[]) return TRUE; } +static inline void +_set_mode(Debugger *self, DebuggerMode new_mode, gboolean trace_message) +{ + self->mode = new_mode; + if (self->breakpoint_site) + { + if (trace_message) + self->breakpoint_site->msg->flags |= LF_STATE_TRACING; + else + self->breakpoint_site->msg->flags &= ~LF_STATE_TRACING; + } +} + +static gboolean +_cmd_continue(Debugger *self, gint argc, gchar *argv[]) +{ + _set_mode(self, DBG_WAITING_FOR_BREAKPOINT, FALSE); + return FALSE; +} + +static gboolean +_cmd_trace(Debugger *self, gint argc, gchar *argv[]) +{ + clock_gettime(CLOCK_MONOTONIC, &self->last_trace_event); + _set_mode(self, DBG_FOLLOW_AND_TRACE, TRUE); + return FALSE; +} + +static gboolean +_cmd_quit(Debugger *self, gint argc, gchar *argv[]) +{ + _set_mode(self, DBG_QUIT, FALSE); + if (self->breakpoint_site) + self->breakpoint_site->drop = TRUE; + main_loop_exit(self->main_loop); + return FALSE; +} + typedef gboolean (*DebuggerCommandFunc)(Debugger *self, gint argc, gchar *argv[]); struct diff --git a/lib/debugger/debugger.h b/lib/debugger/debugger.h index a1c2b38ac3..12506fc8aa 100644 --- a/lib/debugger/debugger.h +++ b/lib/debugger/debugger.h @@ -27,11 +27,27 @@ #include "syslog-ng.h" #include "cfg.h" #include "mainloop.h" +#include "logpipe.h" + +typedef enum +{ + DBG_WAITING_FOR_STEP, + DBG_WAITING_FOR_BREAKPOINT, + DBG_FOLLOW_AND_BREAK, + DBG_FOLLOW_AND_TRACE, + DBG_QUIT, +} DebuggerMode; typedef struct _Debugger Debugger; -typedef gchar *(*FetchCommandFunc)(void); +static inline DebuggerMode +debugger_get_mode(Debugger *self) +{ + return *(DebuggerMode *) self; +} + +typedef gchar *(*FetchCommandFunc)(void); gchar *debugger_builtin_fetch_command(void); void debugger_register_command_fetcher(FetchCommandFunc fetcher); @@ -43,5 +59,41 @@ gboolean debugger_stop_at_breakpoint(Debugger *self, LogPipe *pipe, LogMessage * Debugger *debugger_new(MainLoop *main_loop, GlobalConfig *cfg); void debugger_free(Debugger *self); +static inline gboolean +debugger_is_to_stop(Debugger *self, LogPipe *pipe, LogMessage *msg) +{ + DebuggerMode mode = debugger_get_mode(self); + + switch (mode) + { + case DBG_WAITING_FOR_BREAKPOINT: + return (pipe->flags & PIF_BREAKPOINT); + + case DBG_WAITING_FOR_STEP: + return TRUE; + + case DBG_FOLLOW_AND_BREAK: + return (msg->flags & LF_STATE_TRACING); + + case DBG_FOLLOW_AND_TRACE: + return FALSE; + + case DBG_QUIT: + return FALSE; + + default: + g_assert_not_reached(); + } + return FALSE; +} + +static inline gboolean +debugger_is_to_trace(Debugger *self, LogPipe *pipe, LogMessage *msg) +{ + DebuggerMode mode = debugger_get_mode(self); + + return (mode == DBG_FOLLOW_AND_TRACE) && (msg->flags & LF_STATE_TRACING); + +} #endif From 11144b7920906f65a4046ed5d766c5f164bbce09 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 13 Oct 2024 08:16:10 +0200 Subject: [PATCH 39/42] debugger: filter out uninteresting stop events Some breakpoints are not interesting once the mode changes, even though they were submitted by the pipe hook and are waiting to be resumed. Let's quickly acknowledge them and don't bother the user. Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index 539150e6d3..cd7b76b9c9 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -451,6 +451,34 @@ _handle_interactive_prompt(Debugger *self) printf("(continuing)\n"); } +static gboolean +_debugger_wait_for_event(Debugger *self) +{ + while (1) + { + if (!tracer_wait_for_event(self->tracer, &self->breakpoint_site)) + return FALSE; + + /* this is an interrupt, let's handle it now */ + if (!self->breakpoint_site) + return TRUE; + + /* is this an event we are still interested in? */ + if (debugger_is_to_stop(self, self->breakpoint_site->pipe, self->breakpoint_site->msg)) + return TRUE; + + /* not interesting now, let's resume and wait for another */ + tracer_resume_after_event(self->tracer, self->breakpoint_site); + } + return TRUE; +} + +static void +_debugger_ack_event(Debugger *self) +{ + tracer_resume_after_event(self->tracer, self->breakpoint_site); +} + static gpointer _debugger_thread_func(Debugger *self) { @@ -458,13 +486,14 @@ _debugger_thread_func(Debugger *self) printf("Waiting for breakpoint...\n"); while (1) { - self->breakpoint_site = NULL; - if (!tracer_wait_for_event(self->tracer, &self->breakpoint_site)) + if (!_debugger_wait_for_event(self)) break; _handle_interactive_prompt(self); - tracer_resume_after_event(self->tracer, self->breakpoint_site); + + _debugger_ack_event(self); } + scratch_buffers_explicit_gc(); app_thread_stop(); return NULL; From bd2cdb63c7ce74bf92f59922765d2eefeb50fec7 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 13 Oct 2024 08:23:24 +0200 Subject: [PATCH 40/42] debugger: clean up trace command Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index cd7b76b9c9..3aa483995f 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -161,7 +161,7 @@ _cmd_help(Debugger *self, gint argc, gchar *argv[]) printf("syslog-ng interactive console, the following commands are available\n\n" " help, h, or ? Display this help\n" " continue or c Continue until the next breakpoint\n" - " trace Display timing information as the message traverses the config\n" + " trace or t Trace this message along the configuration\n" " info Display information about the current execution state\n" " list or l Display source code at the current location\n" " print, p Print the current log message\n" @@ -334,6 +334,7 @@ struct { "quit", _cmd_quit }, { "q", _cmd_quit }, { "trace", _cmd_trace, .requires_breakpoint_site = TRUE }, + { "t", _cmd_trace, .requires_breakpoint_site = TRUE }, { "info", _cmd_info, .requires_breakpoint_site = TRUE }, { "i", _cmd_info, .requires_breakpoint_site = TRUE }, { NULL, NULL } From 34151375d866c208641e9ead383076ab7146f58d Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 13 Oct 2024 08:29:03 +0200 Subject: [PATCH 41/42] debugger: add welcome blurb on startup Also, stop at the prompt immediately, instead of wait for a message. Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index 3aa483995f..d305bb6945 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -48,6 +48,7 @@ struct _Debugger GThread *debugger_thread; BreakpointSite *breakpoint_site; struct timespec last_trace_event; + gboolean starting_up; /* user interface related state */ gchar *command_buffer; @@ -436,10 +437,10 @@ _handle_interactive_prompt(Debugger *self) _display_source_line(self); _display_msg_with_template(self, self->breakpoint_site->msg, self->display_template); } - else + else if (!self->starting_up) { _set_current_location(self, NULL); - printf("Stopping on interrupt, message related commands are unavailable...\n"); + printf(" Stopping on Interrupt...\n"); } while (1) { @@ -484,7 +485,16 @@ static gpointer _debugger_thread_func(Debugger *self) { app_thread_start(); - printf("Waiting for breakpoint...\n"); + self->breakpoint_site = NULL; + + printf("axosyslog interactive debugger\n" + "Copyright (c) 2024 Axoflow and contributors\n" + "License LGPLV2.1+ and GPLv2+\n\n" + "For help, type \"help\".\n"); + + self->starting_up = TRUE; + _handle_interactive_prompt(self); + self->starting_up = FALSE; while (1) { if (!_debugger_wait_for_event(self)) From d1d818892dcc719a4517d8e131162399b786ad48 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 13 Oct 2024 08:30:23 +0200 Subject: [PATCH 42/42] debugger: add step and follow commands Signed-off-by: Balazs Scheidler --- lib/debugger/debugger.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/debugger/debugger.c b/lib/debugger/debugger.c index d305bb6945..1f3e404059 100644 --- a/lib/debugger/debugger.c +++ b/lib/debugger/debugger.c @@ -162,6 +162,8 @@ _cmd_help(Debugger *self, gint argc, gchar *argv[]) printf("syslog-ng interactive console, the following commands are available\n\n" " help, h, or ? Display this help\n" " continue or c Continue until the next breakpoint\n" + " step or s Single step\n" + " follow or f Follow this message, ignoring any other breakpoints\n" " trace or t Trace this message along the configuration\n" " info Display information about the current execution state\n" " list or l Display source code at the current location\n" @@ -294,6 +296,13 @@ _cmd_continue(Debugger *self, gint argc, gchar *argv[]) return FALSE; } +static gboolean +_cmd_step(Debugger *self, gint argc, gchar *argv[]) +{ + _set_mode(self, DBG_WAITING_FOR_STEP, FALSE); + return FALSE; +} + static gboolean _cmd_trace(Debugger *self, gint argc, gchar *argv[]) { @@ -302,6 +311,13 @@ _cmd_trace(Debugger *self, gint argc, gchar *argv[]) return FALSE; } +static gboolean +_cmd_follow(Debugger *self, gint argc, gchar *argv[]) +{ + _set_mode(self, DBG_FOLLOW_AND_BREAK, TRUE); + return FALSE; +} + static gboolean _cmd_quit(Debugger *self, gint argc, gchar *argv[]) { @@ -326,6 +342,10 @@ struct { "?", _cmd_help }, { "continue", _cmd_continue }, { "c", _cmd_continue }, + { "step", _cmd_step }, + { "s", _cmd_step }, + { "follow", _cmd_follow, .requires_breakpoint_site = TRUE }, + { "f", _cmd_follow, .requires_breakpoint_site = TRUE }, { "print", _cmd_print, .requires_breakpoint_site = TRUE }, { "p", _cmd_print, .requires_breakpoint_site = TRUE }, { "list", _cmd_list, },