From 844812959beb1a6bbcc64099a4f85b3db127a8a4 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Wed, 8 Jul 2026 01:57:08 +0300 Subject: triton: reject concurrent config reload requests triton_conf_reload() kept the notify callback in a single global slot, and the CLI reload command likewise stored its wakeup context in a global. A second reload issued while the first was still pending (from another CLI connection or SIGUSR1) overwrote both, so only the last requester was notified when the reload completed; the earlier CLI context slept forever in triton_context_schedule() and that connection hung while the rest of the daemon kept running. Make triton_conf_reload() return -1 when a reload is already pending, checked atomically under threads_lock, and pass a caller-provided arg through to the notify callback so each requester keeps its own state instead of sharing globals. The CLI reload's request struct is heap-allocated rather than kept on reload_exec's stack, since triton_context_schedule() can migrate a suspended context onto a different worker thread's stack before the notify callback runs, which would otherwise leave conf_reload_notify() writing through a stale stack pointer. Also mark the reload as running (need_config_reload = 2) before dropping threads_lock to call __config_reload(). Previously a worker woken during the reload (e.g. by the notify callback waking the CLI context, or any stray context wakeup) could loop through the idle path, decrement the active count back to zero and re-enter __config_reload() while need_config_reload was still set, running a second concurrent conf_reload() and invoking the notify callback twice - corrupting the wakeup list and, with the CLI's heap request, writing through freed memory. The SIGUSR1 handler used to call triton_conf_reload() directly from signal context, taking spinlocks and potentially running the whole config parse inside the handler. It now only sets a flag; the main thread waits with sigtimedwait() and performs the reload (and logs a warning when one is already in progress) from normal thread context. The CLI now replies "reload is already in progress" instead of losing the first requester's wakeup. --- accel-pppd/cli/std_cmd.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) (limited to 'accel-pppd/cli/std_cmd.c') diff --git a/accel-pppd/cli/std_cmd.c b/accel-pppd/cli/std_cmd.c index 951b10da..1d15a28c 100644 --- a/accel-pppd/cli/std_cmd.c +++ b/accel-pppd/cli/std_cmd.c @@ -340,26 +340,41 @@ static int shutdown_exec(const char *cmd, char * const *f, int f_cnt, void *cli) } //========================== -static int conf_reload_res; -static struct triton_context_t *conf_reload_ctx; -static void conf_reload_notify(int r) +struct conf_reload_req { + struct triton_context_t *ctx; + int res; +}; +static void conf_reload_notify(int r, void *arg) { + struct conf_reload_req *req = arg; + if (!r) triton_event_fire(EV_CONFIG_RELOAD, NULL); - conf_reload_res = r; - triton_context_wakeup(conf_reload_ctx); + req->res = r; + triton_context_wakeup(req->ctx); } static int reload_exec(const char *cmd, char * const *f, int f_cnt, void *cli) { - if (f_cnt == 1) { - conf_reload_ctx = triton_context_self(); - triton_conf_reload(conf_reload_notify); - triton_context_schedule(); - if (conf_reload_res) - cli_send(cli, "failed\r\n"); - return CLI_CMD_OK; - } else + struct conf_reload_req *req; + + if (f_cnt != 1) return CLI_CMD_SYNTAX; + + /* heap-allocated: triton_context_schedule() can migrate this + * context to another worker thread's stack before notify runs */ + req = _malloc(sizeof(*req)); + req->ctx = triton_context_self(); + + if (triton_conf_reload(conf_reload_notify, req)) { + _free(req); + cli_send(cli, "reload is already in progress\r\n"); + return CLI_CMD_OK; + } + triton_context_schedule(); + if (req->res) + cli_send(cli, "failed\r\n"); + _free(req); + return CLI_CMD_OK; } static void reload_help(char * const *fields, int fields_cnt, void *client) -- cgit v1.2.3