[PATCH obexd 1/3] core: Use signalfd to handle unix signals

[PATCH obexd 1/3] core: Use signalfd to handle unix signals

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Writing anything to syslog in the signal handler can cause a deadlock
with any ongoing syslog write.

This is also aligned with what BlueZ and other projects has been doing.
---
 src/main.c |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/src/main.c b/src/main.c
index 992a646..a2a94f7 100644
--- a/src/main.c
+++ b/src/main.c
@@ -35,6 +35,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <sys/signalfd.h>
 #include <fcntl.h>
 #include <termios.h>
 #include <getopt.h>
@@ -53,15 +54,82 @@
 
 static GMainLoop *main_loop = NULL;
 
-static void sig_term(int sig)
+static unsigned int __terminated = 0;
+
+static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
+							gpointer user_data)
 {
-	info("Terminating due to signal %d", sig);
-	g_main_loop_quit(main_loop);
+	struct signalfd_siginfo si;
+	ssize_t result;
+	int fd;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
+		return FALSE;
+
+	fd = g_io_channel_unix_get_fd(channel);
+
+	result = read(fd, &si, sizeof(si));
+	if (result != sizeof(si))
+		return FALSE;
+
+	switch (si.ssi_signo) {
+	case SIGINT:
+	case SIGTERM:
+		if (__terminated == 0) {
+			info("Terminating");
+			g_main_loop_quit(main_loop);
+		}
+
+		__terminated = 1;
+		break;
+	case SIGUSR2:
+		__obex_log_enable_debug();
+		break;
+	case SIGPIPE:
+		/* ignore */
+		break;
+	}
+
+	return TRUE;
 }
 
-static void sig_debug(int sig)
+static guint setup_signalfd(void)
 {
-	__obex_log_enable_debug();
+	GIOChannel *channel;
+	guint source;
+	sigset_t mask;
+	int fd;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGINT);
+	sigaddset(&mask, SIGTERM);
+	sigaddset(&mask, SIGUSR2);
+	sigaddset(&mask, SIGPIPE);
+
+	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
+		perror("Failed to set signal mask");
+		return 0;
+	}
+
+	fd = signalfd(-1, &mask, 0);
+	if (fd < 0) {
+		perror("Failed to create signal descriptor");
+		return 0;
+	}
+
+	channel = g_io_channel_unix_new(fd);
+
+	g_io_channel_set_close_on_unref(channel, TRUE);
+	g_io_channel_set_encoding(channel, NULL, NULL);
+	g_io_channel_set_buffered(channel, FALSE);
+
+	source = g_io_add_watch(channel,
+				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				signal_handler, NULL);
+
+	g_io_channel_unref(channel);
+
+	return source;
 }
 
 static gboolean option_detach = TRUE;
@@ -179,7 +247,7 @@ int main(int argc, char *argv[])
 {
 	GOptionContext *context;
 	GError *err = NULL;
-	struct sigaction sa;
+	guint signal;
 
 #ifdef NEED_THREADS
 	if (g_thread_supported() == FALSE)
@@ -213,6 +281,8 @@ int main(int argc, char *argv[])
 
 	main_loop = g_main_loop_new(NULL, FALSE);
 
+	signal = setup_signalfd();
+
 #ifdef NEED_THREADS
 	if (dbus_threads_init_default() == FALSE) {
 		fprintf(stderr, "Can't init usage of threads\n");
@@ -251,16 +321,10 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	memset(&sa, 0, sizeof(sa));
-	sa.sa_handler = sig_term;
-	sigaction(SIGINT, &sa, NULL);
-	sigaction(SIGTERM, &sa, NULL);
-
-	sa.sa_handler = sig_debug;
-	sigaction(SIGUSR2, &sa, NULL);
-
 	g_main_loop_run(main_loop);
 
+	g_source_remove(signal);
+
 	obex_server_exit();
 
 	plugin_cleanup();
-- 
1.7.7.6

[PATCH obexd 2/3] client: Use signalfd to handle unix signals

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 client/main.c |   91 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/client/main.c b/client/main.c
index 80f9413..6423f32 100644
--- a/client/main.c
+++ b/client/main.c
@@ -30,6 +30,8 @@
 #include <string.h>
 #include <signal.h>
 #include <syslog.h>
+#include <unistd.h>
+#include <sys/signalfd.h>
 
 #include <glib.h>
 #include <gdbus.h>
@@ -39,6 +41,80 @@
 
 static GMainLoop *event_loop = NULL;
 
+static unsigned int __terminated = 0;
+
+static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
+							gpointer user_data)
+{
+	struct signalfd_siginfo si;
+	ssize_t result;
+	int fd;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
+		return FALSE;
+
+	fd = g_io_channel_unix_get_fd(channel);
+
+	result = read(fd, &si, sizeof(si));
+	if (result != sizeof(si))
+		return FALSE;
+
+	switch (si.ssi_signo) {
+	case SIGINT:
+	case SIGTERM:
+		if (__terminated == 0) {
+			info("Terminating");
+			g_main_loop_quit(event_loop);
+		}
+
+		__terminated = 1;
+		break;
+	case SIGPIPE:
+		/* ignore */
+		break;
+	}
+
+	return TRUE;
+}
+
+static guint setup_signalfd(void)
+{
+	GIOChannel *channel;
+	guint source;
+	sigset_t mask;
+	int fd;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGINT);
+	sigaddset(&mask, SIGTERM);
+	sigaddset(&mask, SIGPIPE);
+
+	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
+		perror("Failed to set signal mask");
+		return 0;
+	}
+
+	fd = signalfd(-1, &mask, 0);
+	if (fd < 0) {
+		perror("Failed to create signal descriptor");
+		return 0;
+	}
+
+	channel = g_io_channel_unix_new(fd);
+
+	g_io_channel_set_close_on_unref(channel, TRUE);
+	g_io_channel_set_encoding(channel, NULL, NULL);
+	g_io_channel_set_buffered(channel, FALSE);
+
+	source = g_io_add_watch(channel,
+				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				signal_handler, NULL);
+
+	g_io_channel_unref(channel);
+
+	return source;
+}
+
 static char *option_debug = NULL;
 static gboolean option_stderr = FALSE;
 
@@ -62,16 +138,12 @@ static GOptionEntry options[] = {
 	{ NULL },
 };
 
-static void sig_term(int sig)
-{
-	g_main_loop_quit(event_loop);
-}
 
 int main(int argc, char *argv[])
 {
 	GOptionContext *context;
-	struct sigaction sa;
 	GError *gerr = NULL;
+	guint signal;
 
 	context = g_option_context_new(NULL);
 	g_option_context_add_main_entries(context, options, NULL);
@@ -87,6 +159,8 @@ int main(int argc, char *argv[])
 
 	event_loop = g_main_loop_new(NULL, FALSE);
 
+	signal = setup_signalfd();
+
 	__obex_log_init("obex-client", option_debug, !option_stderr);
 
 	if (manager_init() < 0)
@@ -94,13 +168,10 @@ int main(int argc, char *argv[])
 
 	DBG("Entering main loop");
 
-	memset(&sa, 0, sizeof(sa));
-	sa.sa_handler = sig_term;
-	sigaction(SIGINT, &sa, NULL);
-	sigaction(SIGTERM, &sa, NULL);
-
 	g_main_loop_run(event_loop);
 
+	g_source_remove(signal);
+
 	manager_exit();
 
 	g_main_loop_unref(event_loop);
-- 
1.7.7.6

[PATCH obexd 3/3] client: Add handler for SIGUSR2

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This signal enables debug for obed so just do the same for obex-client
---
 client/main.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/client/main.c b/client/main.c
index 6423f32..3eabfda 100644
--- a/client/main.c
+++ b/client/main.c
@@ -69,6 +69,9 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
 
 		__terminated = 1;
 		break;
+	case SIGUSR2:
+		__obex_log_enable_debug();
+		break;
 	case SIGPIPE:
 		/* ignore */
 		break;
@@ -87,6 +90,7 @@ static guint setup_signalfd(void)
 	sigemptyset(&mask);
 	sigaddset(&mask, SIGINT);
 	sigaddset(&mask, SIGTERM);
+	sigaddset(&mask, SIGUSR2);
 	sigaddset(&mask, SIGPIPE);
 
 	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
-- 
1.7.7.6

Re: [PATCH obexd 1/3] core: Use signalfd to handle unix signals

[Johan Hedberg]

Hi Luiz,

On Mon, May 07, 2012, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> Writing anything to syslog in the signal handler can cause a deadlock
> with any ongoing syslog write.
> 
> This is also aligned with what BlueZ and other projects has been doing.
> ---
>  src/main.c |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 78 insertions(+), 14 deletions(-)

All three patches have been applied. Thanks.

Johan

Re: [PATCH obexd 3/3] client: Add handler for SIGUSR2

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2012-05-07 17:00:53 -0700]:

> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This signal enables debug for obed so just do the same for obex-client

I think you meant "obexd" here.

	Gustavo