Merge pull request #333 from nuclearcat/triton-reload-raceHEADmaster

triton: reject concurrent config reload requests

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);