From: Andre Noll <maan@systemlinux.org>
Date: Wed, 1 Jan 2014 22:15:02 +0000 (+0000)
Subject: audiod: Clean up by notifying tasks.
X-Git-Tag: v0.5.4~49^2
X-Git-Url: http://git.tue.mpg.de/?a=commitdiff_plain;h=751fface082c907983978023bdcb43540a462192;p=paraslash.git

audiod: Clean up by notifying tasks.

There are two ways to terminate para_audiod in a controlled way:
by executing the "term" command and by sending SIGINT or SIGTERM
to the process. Currently both code paths call clean_exit() from
some ->post_select function, which terminates para_audiod by calling
exit(3). Despite the name, this is "unclean" because tasks are not shut
down properly, so not all memory can be freed by this approach. While
this is not a big problem, it makes it more difficult to debug real
memory leaks.

This patch tries to overcome this problem by using notifications to
shut down the audiod tasks. Two new error codes E_AUDIOD_TERM and
E_AUDIOD_SIGNAL are introduced for the notification values. All tasks
are modified to check for notifications and now return the (negative)
notification value from their ->post_select() method if a notification
was received. Hence schedule() returns to main() and we may clean up
the resources allocated by the scheduler by calling sched_shutdown(),
along with the usual cleanup performed by clean_exit(). The latter
function is renamed to audiod_cleanup(), which is more to the point.
---

diff --git a/audiod.c b/audiod.c
index 6fd101a0..74298e3c 100644
--- a/audiod.c
+++ b/audiod.c
@@ -1006,16 +1006,22 @@ static void signal_pre_select(struct sched *s, void *context)
 	para_fd_set(st->fd, &s->rfds, &s->max_fileno);
 }
 
-static int signal_post_select(struct sched *s, __a_unused void *context)
+static int signal_post_select(struct sched *s, void *context)
 {
-	int signum;
+	struct signal_task *st = context;
+	int ret, signum;
+
+	ret = task_get_notification(st->task);
+	if (ret < 0)
+		return ret;
 	signum = para_next_signal(&s->rfds);
 	switch (signum) {
 	case SIGINT:
 	case SIGTERM:
 	case SIGHUP:
 		PARA_NOTICE_LOG("received signal %d\n", signum);
-		clean_exit(EXIT_FAILURE, "caught deadly signal");
+		task_notify_all(s, E_AUDIOD_SIGNAL);
+		return -E_AUDIOD_SIGNAL;
 	}
 	return 0;
 }
@@ -1034,10 +1040,17 @@ static int command_post_select(struct sched *s, void *context)
 	struct timeval tmp, delay;
 	bool force = true;
 
-	ret = handle_connect(ct->fd, &s->rfds);
+	ret = task_get_notification(ct->task);
 	if (ret < 0)
+		return ret;
+	ret = handle_connect(ct->fd, &s->rfds);
+	if (ret < 0) {
 		PARA_ERROR_LOG("%s\n", para_strerror(-ret));
-	else if (ret > 0)
+		if (ret == -E_AUDIOD_TERM) {
+			task_notify_all(s, -ret);
+			return ret;
+		}
+	} else if (ret > 0)
 		goto dump;
 
 	/* if last status dump was less than 500ms ago, do nothing */
@@ -1140,19 +1153,13 @@ static void close_unused_slots(void)
 			close_slot(i);
 }
 
-/**
- * Close the connection to para_server and exit.
- *
- * \param status The exit status which is passed to exit(3).
- * \param msg The log message
- *
- * Log \a msg with loglevel \p EMERG, close the connection to para_server and
- * all slots, and call \p exit(status). \a status should be either EXIT_SUCCESS
- * or EXIT_FAILURE.
+/*
+ * Cleanup all resources.
  *
- * \sa exit(3).
+ * This performs various cleanups, removes the audiod socket and closes the
+ * connection to para_server.
  */
-void __noreturn clean_exit(int status, const char *msg)
+static void audiod_cleanup(void)
 {
 	if (socket_name)
 		unlink(socket_name);
@@ -1160,8 +1167,6 @@ void __noreturn clean_exit(int status, const char *msg)
 	close_unused_slots();
 	audiod_cmdline_parser_free(&conf);
 	close_stat_clients();
-	PARA_EMERG_LOG("%s\n", msg);
-	exit(status);
 }
 
 /*
@@ -1226,7 +1231,11 @@ min_delay:
 static int status_post_select(struct sched *s, void *context)
 {
 	struct status_task *st = context;
+	int ret;
 
+	ret = task_get_notification(st->task);
+	if (ret < 0)
+		return ret;
 	if (audiod_status == AUDIOD_OFF) {
 		if (!st->ct)
 			goto out;
@@ -1241,7 +1250,6 @@ static int status_post_select(struct sched *s, void *context)
 	if (st->ct) {
 		char *buf;
 		size_t sz;
-		int ret;
 
 		ret = btr_node_status(st->btrn, st->min_iqs, BTR_NT_LEAF);
 		if (ret < 0) {
@@ -1435,8 +1443,10 @@ int main(int argc, char *argv[])
 	sched.default_timeout.tv_sec = 2;
 	sched.default_timeout.tv_usec = 999 * 1000;
 	ret = schedule(&sched);
+	audiod_cleanup();
 	sched_shutdown(&sched);
 
-	PARA_EMERG_LOG("%s\n", para_strerror(-ret));
-	return EXIT_FAILURE;
+	if (ret < 0)
+		PARA_EMERG_LOG("%s\n", para_strerror(-ret));
+	return ret < 0? EXIT_FAILURE : EXIT_SUCCESS;
 }
diff --git a/audiod_command.c b/audiod_command.c
index 56d922e6..2f2f3062 100644
--- a/audiod_command.c
+++ b/audiod_command.c
@@ -376,10 +376,10 @@ static int com_grab(int fd, int argc, char **argv)
 	return grab_client_new(fd, argc, argv, &sched);
 }
 
-__noreturn static int com_term(int fd, __a_unused int argc, __a_unused char **argv)
+static int com_term(int fd, __a_unused int argc, __a_unused char **argv)
 {
 	close(fd);
-	clean_exit(EXIT_SUCCESS, "terminating on user request");
+	return -E_AUDIOD_TERM;
 }
 
 static int com_on(int fd, __a_unused int argc, __a_unused char **argv)
diff --git a/error.h b/error.h
index e91f49b4..8bc87335 100644
--- a/error.h
+++ b/error.h
@@ -343,6 +343,8 @@ extern const char **para_errlist[];
 	PARA_ERROR(NOT_PLAYING, "not playing"), \
 	PARA_ERROR(AUDIOD_OFF, "audiod switched off"), \
 	PARA_ERROR(STATUS_TIMEOUT, "status item timeout"), \
+	PARA_ERROR(AUDIOD_SIGNAL, "caught deadly signal"), \
+	PARA_ERROR(AUDIOD_TERM, "terminating on user request"), \
 
 
 #define AUDIOD_COMMAND_ERRORS \