From 30741f9f15db7c4681755c1234d8c4335013a487 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Tue, 7 Oct 2014 19:56:01 +0000 Subject: [PATCH 1/1] Improve signal init and shutdown. Currently all users of the signal subsystem define their own signal_task structure and initialize the ->fd field from the return value of para_signal_init(). It is more natural to let the signal subsystem perform the allocation and the initialization. This commit renames para_signal_init() to signal_init_or_die() and changes the function to allocate, initialize and return a signal task structure (rather than only the file descriptor of the signal pipe as para_signal_init() did). Similarly, para_signal_shutdown() is renamed to signal_shutdown() and now takes a pointer to the signal_task structure which was obtained in an earlier call to signal_init_or_die(), and frees it. Conversion of all users is straight forward. The patch also adds a missing call to signal_shutdown() to audiod.c, closing an fd leak. --- afs.c | 11 ++++------- audiod.c | 10 +++++----- gui.c | 9 +++++---- server.c | 15 ++++++--------- signal.c | 21 +++++++++++++++------ signal.h | 4 ++-- 6 files changed, 37 insertions(+), 33 deletions(-) diff --git a/afs.c b/afs.c index 1d23c9ff..96621e4b 100644 --- a/afs.c +++ b/afs.c @@ -89,7 +89,7 @@ extern struct misc_meta_data *mmd; static int server_socket; static struct command_task command_task_struct; -static struct signal_task signal_task_struct; +static struct signal_task *signal_task; static enum play_mode current_play_mode; static char *current_mop; /* mode or playlist specifier. NULL means dummy mood */ @@ -747,20 +747,17 @@ shutdown: static void register_signal_task(struct sched *s) { - struct signal_task *st = &signal_task_struct; - para_sigaction(SIGPIPE, SIG_IGN); - st->fd = para_signal_init(); - PARA_INFO_LOG("signal pipe: fd %d\n", st->fd); + signal_task = signal_init_or_die(); para_install_sighandler(SIGINT); para_install_sighandler(SIGTERM); para_install_sighandler(SIGHUP); - st->task = task_register(&(struct task_info) { + signal_task->task = task_register(&(struct task_info) { .name = "signal", .pre_select = signal_pre_select, .post_select = afs_signal_post_select, - .context = st, + .context = signal_task, }, s); } diff --git a/audiod.c b/audiod.c index 4285dd5e..3f83b984 100644 --- a/audiod.c +++ b/audiod.c @@ -144,7 +144,7 @@ struct audiod_args_info conf; static char *socket_name; static struct audio_format_info afi[NUM_AUDIO_FORMATS]; -static struct signal_task signal_task_struct, *sig_task = &signal_task_struct; +static struct signal_task *signal_task; static struct status_task status_task_struct; @@ -347,8 +347,7 @@ err: static void setup_signal_handling(void) { - sig_task->fd = para_signal_init(); - PARA_INFO_LOG("signal pipe: fd %d\n", sig_task->fd); + signal_task = signal_init_or_die(); para_install_sighandler(SIGINT); para_install_sighandler(SIGTERM); para_install_sighandler(SIGHUP); @@ -1406,11 +1405,11 @@ int main(int argc, char *argv[]) if (conf.daemon_given) daemonize(false /* parent exits immediately */); - sig_task->task = task_register(&(struct task_info) { + signal_task->task = task_register(&(struct task_info) { .name = "signal", .pre_select = signal_pre_select, .post_select = signal_post_select, - .context = sig_task, + .context = signal_task, }, &sched); sched.default_timeout.tv_sec = 2; @@ -1418,6 +1417,7 @@ int main(int argc, char *argv[]) ret = schedule(&sched); audiod_cleanup(); sched_shutdown(&sched); + signal_shutdown(signal_task); if (ret < 0) PARA_EMERG_LOG("%s\n", para_strerror(-ret)); diff --git a/gui.c b/gui.c index 620367e5..c82cd8c3 100644 --- a/gui.c +++ b/gui.c @@ -1406,7 +1406,7 @@ static int setup_tasks_and_schedule(void) struct exec_task exec_task = {.task = NULL}; struct status_task status_task = {.fd = -1}; struct input_task input_task = {.task = NULL}; - struct signal_task signal_task = {.task = NULL}; + struct signal_task *signal_task; struct sched sched = { .default_timeout = { .tv_sec = conf.timeout_arg / 1000, @@ -1435,19 +1435,20 @@ static int setup_tasks_and_schedule(void) .context = &input_task, }, &sched); - signal_task.fd = para_signal_init(); + signal_task = signal_init_or_die(); para_install_sighandler(SIGINT); para_install_sighandler(SIGTERM); para_install_sighandler(SIGCHLD); para_install_sighandler(SIGUSR1); - signal_task.task = task_register(&(struct task_info) { + signal_task->task = task_register(&(struct task_info) { .name = "signal", .pre_select = signal_pre_select, .post_select = signal_post_select, - .context = &signal_task, + .context = signal_task, }, &sched); ret = schedule(&sched); sched_shutdown(&sched); + signal_shutdown(signal_task); return ret; } diff --git a/server.c b/server.c index a048cfda..68434e94 100644 --- a/server.c +++ b/server.c @@ -100,6 +100,7 @@ int mmd_mutex; static char *user_list_file = NULL; static struct sched sched; +static struct signal_task *signal_task; /** The task responsible for server command handling. */ struct server_command_task { @@ -295,22 +296,18 @@ cleanup: static void init_signal_task(void) { - static struct signal_task signal_task_struct, - *st = &signal_task_struct; - - PARA_NOTICE_LOG("setting up signal handling\n"); - st->fd = para_signal_init(); /* always successful */ + signal_task = signal_init_or_die(); para_install_sighandler(SIGINT); para_install_sighandler(SIGTERM); para_install_sighandler(SIGHUP); para_install_sighandler(SIGCHLD); para_sigaction(SIGPIPE, SIG_IGN); - add_close_on_fork_list(st->fd); - st->task = task_register(&(struct task_info) { + add_close_on_fork_list(signal_task->fd); + signal_task->task = task_register(&(struct task_info) { .name = "signal", .pre_select = signal_pre_select, .post_select = signal_post_select, - .context = st, + .context = signal_task, }, &sched); } @@ -364,7 +361,7 @@ static int command_post_select(struct sched *s, void *context) free(chunk_table); alarm(ALARM_TIMEOUT); close_listed_fds(); - para_signal_shutdown(); + signal_shutdown(signal_task); /* * put info on who we are serving into argv[0] to make * client ip visible in top/ps diff --git a/signal.c b/signal.c index 780c8326..5d6e6e45 100644 --- a/signal.c +++ b/signal.c @@ -7,11 +7,13 @@ #include #include +#include #include "para.h" #include "error.h" #include "fd.h" #include "list.h" +#include "string.h" #include "sched.h" #include "signal.h" @@ -32,12 +34,14 @@ static int signal_pipe[2]; * ready for reading, see select(2). * * \return This function either succeeds or calls exit(2) to terminate the - * current process. On success, the file descriptor of the read end of the - * signal pipe is returned. + * current process. On success, a signal task structure is returned. */ -int para_signal_init(void) +struct signal_task *signal_init_or_die(void) { + struct signal_task *st; int ret; + + PARA_NOTICE_LOG("setting up signal handling\n"); if (pipe(signal_pipe) < 0) { ret = -ERRNO_TO_PARA_ERROR(errno); goto err_out; @@ -48,7 +52,9 @@ int para_signal_init(void) ret = mark_fd_nonblocking(signal_pipe[1]); if (ret < 0) goto err_out; - return signal_pipe[0]; + st = para_calloc(sizeof(*st)); + st->fd = signal_pipe[0]; + return st; err_out: PARA_EMERG_LOG("%s\n", para_strerror(-ret)); exit(EXIT_FAILURE); @@ -223,9 +229,12 @@ int para_next_signal(fd_set *rfds) } /** - * Close the write end of the signal pipe. + * Close the write end of the signal pipe, deallocate resources. + * + * \param st The pointer obtained earlier from signal_init_or_die(). */ -void para_signal_shutdown(void) +void signal_shutdown(struct signal_task *st) { close(signal_pipe[1]); + free(st); } diff --git a/signal.h b/signal.h index 643c148f..b5b06f35 100644 --- a/signal.h +++ b/signal.h @@ -22,11 +22,11 @@ _static_inline_ void signal_pre_select(struct sched *s, void *context) para_fd_set(st->fd, &s->rfds, &s->max_fileno); } -int para_signal_init(void); +struct signal_task *signal_init_or_die(void); void para_sigaction(int sig, void (*handler)(int)); void para_install_sighandler(int); int para_reap_child(pid_t *pid); int para_next_signal(fd_set *rfds); -void para_signal_shutdown(void); +void signal_shutdown(struct signal_task *st); void para_block_signal(int sig); void para_unblock_signal(int sig); -- 2.39.2