diff options
| author | Denys Fedoryshchenko <denys.f@collabora.com> | 2026-07-08 01:57:08 +0300 |
|---|---|---|
| committer | Denys Fedoryshchenko <denys.f@collabora.com> | 2026-07-08 02:53:26 +0300 |
| commit | 844812959beb1a6bbcc64099a4f85b3db127a8a4 (patch) | |
| tree | 0c494e3b141ac5e53e1f18d05809b84be95f3ca3 /accel-pppd | |
| parent | f4014a4a2c9e654646faeb81cd9ac5841b1c9b0f (diff) | |
| download | accel-ppp-844812959beb1a6bbcc64099a4f85b3db127a8a4.tar.gz accel-ppp-844812959beb1a6bbcc64099a4f85b3db127a8a4.zip | |
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.
Diffstat (limited to 'accel-pppd')
| -rw-r--r-- | accel-pppd/cli/std_cmd.c | 41 | ||||
| -rw-r--r-- | accel-pppd/main.c | 20 | ||||
| -rw-r--r-- | accel-pppd/triton/triton.c | 30 | ||||
| -rw-r--r-- | accel-pppd/triton/triton.h | 2 |
4 files changed, 67 insertions, 26 deletions
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) diff --git a/accel-pppd/main.c b/accel-pppd/main.c index 515b8036..221be7be 100644 --- a/accel-pppd/main.c +++ b/accel-pppd/main.c @@ -41,6 +41,8 @@ static int term; static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +static volatile sig_atomic_t need_reload; + #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) static pthread_mutex_t *ssl_lock_cs; @@ -100,7 +102,7 @@ static void change_limits(void) log_emerg("main: setrlimit: %s\n", strerror(errno)); } -static void config_reload_notify(int r) +static void config_reload_notify(int r, void *arg) { if (!r) triton_event_fire(EV_CONFIG_RELOAD, NULL); @@ -108,7 +110,7 @@ static void config_reload_notify(int r) static void config_reload(int num) { - triton_conf_reload(config_reload_notify); + need_reload = 1; } static void close_all_fd(void) @@ -437,7 +439,19 @@ int main(int _argc, char **_argv) backup_restore(internal); #endif - sigwait(&set, &sig); + { + struct timespec ts = { .tv_sec = 1 }; + while (1) { + sig = sigtimedwait(&set, NULL, &ts); + if (sig > 0) + break; + if (need_reload) { + need_reload = 0; + if (triton_conf_reload(config_reload_notify, NULL)) + log_warn("main: config reload is already in progress\n"); + } + } + } log_info1("terminate, sig = %i\n", sig); ap_shutdown_soft(shutdown_cb, 1); diff --git a/accel-pppd/triton/triton.c b/accel-pppd/triton/triton.c index 5c7231b7..105f810a 100644 --- a/accel-pppd/triton/triton.c +++ b/accel-pppd/triton/triton.c @@ -32,8 +32,10 @@ static LIST_HEAD(init_list); static int terminate; static int need_terminate; +/* 0 - idle, 1 - reload requested, 2 - reload running */ static int need_config_reload; -static void (*config_reload_notify)(int); +static void (*config_reload_notify)(int, void *); +static void *config_reload_arg; static mempool_t *ctx_pool; static mempool_t *call_pool; @@ -210,14 +212,14 @@ void triton_thread_wakeup(struct _triton_thread_t *thread) pthread_kill(thread->thread, SIGUSR1); } -static void __config_reload(void (*notify)(int)) +static void __config_reload(void) { struct _triton_thread_t *t; int r; log_debug2("config_reload: enter\n"); r = conf_reload(NULL); - notify(r); + config_reload_notify(r, config_reload_arg); spin_lock(&threads_lock); need_config_reload = 0; @@ -301,9 +303,10 @@ static void* triton_thread(struct _triton_thread_t *thread) if (!terminate) list_add(&thread->entry2, &sleep_threads); - if (triton_stat_thread_active_dec() == 0 && need_config_reload) { + if (triton_stat_thread_active_dec() == 0 && need_config_reload == 1) { + need_config_reload = 2; spin_unlock(&threads_lock); - __config_reload(config_reload_notify); + __config_reload(); } else spin_unlock(&threads_lock); @@ -856,16 +859,25 @@ int __export triton_load_modules(const char *mod_sect) return 0; } -void __export triton_conf_reload(void (*notify)(int)) +int __export triton_conf_reload(void (*notify)(int, void *), void *arg) { spin_lock(&threads_lock); - need_config_reload = 1; + if (need_config_reload) { + spin_unlock(&threads_lock); + return -1; + } config_reload_notify = notify; + config_reload_arg = arg; if (triton_stat_thread_active() == 0) { + need_config_reload = 2; spin_unlock(&threads_lock); - __config_reload(notify); - } else + __config_reload(); + } else { + need_config_reload = 1; spin_unlock(&threads_lock); + } + + return 0; } void __export triton_run() diff --git a/accel-pppd/triton/triton.h b/accel-pppd/triton/triton.h index aef77c2b..63dc9188 100644 --- a/accel-pppd/triton/triton.h +++ b/accel-pppd/triton/triton.h @@ -104,7 +104,7 @@ void triton_event_fire(int ev_id, void *arg); struct conf_sect_t *conf_get_section(const char *name); char *conf_get_opt(const char *sect, const char *name); -void triton_conf_reload(void (*notify)(int)); +int triton_conf_reload(void (*notify)(int, void *), void *arg); void triton_collect_cpu_usage(void); void triton_stop_collect_cpu_usage(void); |
