Improve signal handling

This patch replaces the signalfd based signal handling with a more traditional
approach. Since this is not only more reliable, but also more portable, the
compile time option to turn off signal handling has been removed. It was only
optional before based on signalf being a linux-ism.

1 files changed, 100 insertions, 69 deletions

diff --git a/src/wlclock.c b/src/wlclock.c
index 7257f3d..e3d4de7 100644
--- a/src/wlclock.c
+++ b/src/wlclock.c
@@ -6,9 +6,10 @@
 #include<stdlib.h>
 #include<string.h>
 #include<unistd.h>
-#ifdef HANDLE_SIGNALS
-#include<sys/signalfd.h>
 #include<signal.h>
+
+#ifndef BSD
+#include<execinfo.h>
 #endif
 
 #include<wayland-server.h>
@@ -27,6 +28,84 @@
 
 struct Wlclock_context context = {0};
 
+/**
+ * Intercept user signals so that they don't kill us.
+ */
+static void handle_user_signal (int signum)
+{
+	clocklog(0, "ERROR: wlclock does not handle user signals.\n");
+}
+
+/**
+ * Intercept error signals (like SIGSEGV and SIGFPE) so that we can try to
+ * print a fancy error message and a backtracke before letting the system kill us.
+ */
+static void handle_error (int signum)
+{
+	const char *msg =
+		"\n"
+		"┌──────────────────────────────────────────┐\n"
+		"│                                          │\n"
+		"│         wlclock has crashed.             │\n"
+		"│                                          │\n"
+		"│    This is likely a bug, so please       │\n"
+		"│    report this to the mailing list.      │\n"
+		"│                                          │\n"
+		"│  ~leon_plickat/public-inbox@lists.sr.ht  │\n"
+		"│                                          │\n"
+		"└──────────────────────────────────────────┘\n"
+		"\n";
+	fputs(msg, stderr);
+
+#ifndef BSD
+	fputs("Attempting to get backtrace:\n", stderr);
+
+	/* In some rare cases, getting a backtrace can also cause a segfault.
+	 * There is nothing we can or should do about that. All hope is lost at
+	 * that point.
+	 */
+	void *buffer[255];
+	const int calls = backtrace(buffer, sizeof(buffer) / sizeof(void *));
+	backtrace_symbols_fd(buffer, calls, fileno(stderr));
+	fputs("\n", stderr);
+#endif
+
+	/* Let the default handlers deal with the rest. */
+	signal(signum, SIG_DFL);
+	kill(getpid(), signum);
+}
+
+/**
+ * Intercept soft kills (like SIGINT and SIGTERM) so we can attempt to clean up
+ * and exit gracefully.
+ */
+static void handle_term (int signum)
+{
+	clocklog(1, "[signal] Soft kill received.\n");
+
+	/* If cleanup fails or hangs and causes this signal to be recieved again,
+	 * let the default signal handler kill us.
+	 */
+	signal(signum, SIG_DFL);
+
+	context.loop = false;
+}
+
+/**
+ * Set up signal handlers.
+ */
+static void init_signals (void)
+{
+	signal(SIGSEGV, handle_error);
+	signal(SIGFPE, handle_error);
+
+	signal(SIGINT, handle_term);
+	signal(SIGTERM, handle_term);
+
+	signal(SIGUSR1, handle_user_signal);
+	signal(SIGUSR2, handle_user_signal);
+}
+
 static void registry_handle_global (void *data, struct wl_registry *registry,
 		uint32_t name, const char *interface, uint32_t version)
 {
@@ -496,45 +575,17 @@ static void clock_run ()
 	clocklog(1, "[main] Starting loop.\n");
 	context.ret = EXIT_SUCCESS;
 
-	struct pollfd fds[2] = { 0 };
+	struct pollfd fds[1] = {0};
 	size_t wayland_fd = 0;
-#ifdef HANDLE_SIGNALS
-	size_t signal_fd = 1;
-	size_t fd_count = 2;
-#else
-	size_t fd_count = 1;
-#endif
 
 	fds[wayland_fd].events = POLLIN;
 	if ( -1 ==  (fds[wayland_fd].fd = wl_display_get_fd(context.display)) )
 	{
 		clocklog(0, "ERROR: Unable to open Wayland display fd.\n");
-		goto error;
+		context.ret = EXIT_FAILURE;
+		goto exit;
 	}
 
-#ifdef HANDLE_SIGNALS
-	sigset_t mask;
-	struct signalfd_siginfo fdsi;
-	sigemptyset(&mask);
-	sigaddset(&mask, SIGINT);
-	sigaddset(&mask, SIGTERM);
-	sigaddset(&mask, SIGQUIT);
-	sigaddset(&mask, SIGUSR1);
-	sigaddset(&mask, SIGUSR2);
-	if ( sigprocmask(SIG_BLOCK, &mask, NULL) == -1 )
-	{
-		clocklog(0, "ERROR: sigprocmask() failed.\n");
-		goto error;
-	}
-	fds[signal_fd].events = POLLIN;
-	if ( -1 ==  (fds[signal_fd].fd = signalfd(-1, &mask, 0)) )
-	{
-		clocklog(0, "ERROR: Unable to open signal fd.\n"
-				"ERROR: signalfd: %s\n", strerror(errno));
-		goto error;
-	}
-#endif
-
 	while (context.loop)
 	{
 		/* Flush Wayland events. */
@@ -548,7 +599,7 @@ static void clock_run ()
 			}
 		} while ( errno == EAGAIN );
 
-		int ret = poll(fds, fd_count, get_timeout());
+		int ret = poll(fds, 1, get_timeout());
 
 		if ( ret == 0 ) /* Timeout -> update clock hands. */
 		{
@@ -557,7 +608,10 @@ static void clock_run ()
 		}
 		else if ( ret < 0 )
 		{
-			clocklog(0, "ERROR: poll: %s\n", strerror(errno));
+			if ( errno == EINTR )
+				clocklog(1, "[loop] Interrupted by signal.\n");
+			else
+				clocklog(0, "ERROR: poll: %s\n", strerror(errno));
 			continue;
 		}
 	
@@ -565,44 +619,18 @@ static void clock_run ()
 		if ( fds[wayland_fd].revents & POLLIN && wl_display_dispatch(context.display) == -1 )
 		{
 			clocklog(0, "ERROR: wl_display_flush: %s\n", strerror(errno));
-			goto error;
+			context.ret = EXIT_FAILURE;
+			break;
 		}
 		if ( fds[wayland_fd].revents & POLLOUT && wl_display_flush(context.display) == -1 )
 		{
 			clocklog(0, "ERROR: wl_display_flush: %s\n", strerror(errno));
-			goto error;
-		}
-
-#ifdef HANDLE_SIGNALS
-		/* Signal events. */
-		if ( fds[signal_fd].revents & POLLIN )
-		{
-			if ( read(fds[signal_fd].fd, &fdsi, sizeof(struct signalfd_siginfo))
-					!= sizeof(struct signalfd_siginfo) )
-			{
-				clocklog(0, "ERROR: Can not read signal info.\n");
-				goto error;
-			}
-
-			if ( fdsi.ssi_signo == SIGINT || fdsi.ssi_signo == SIGQUIT || fdsi.ssi_signo == SIGTERM )
-			{
-				clocklog(1, "[main] Received SIGINT, SIGQUIT or SIGTERM; Exiting.\n");
-				goto exit;
-			}
-			else if ( fdsi.ssi_signo == SIGUSR1 || fdsi.ssi_signo == SIGUSR2 )
-				clocklog(1, "[main] Received SIGUSR; Ignoring.\n");
+			context.ret = EXIT_FAILURE;
+			break;
 		}
-#endif
 	}
 
-	return;
-error:
-	context.ret = EXIT_FAILURE;
-#ifdef HANDLE_SIGNALS
 exit:
-	if ( fds[signal_fd].fd != -1 )
-		close(fds[signal_fd].fd);
-#endif
 	if ( fds[wayland_fd].fd != -1 )
 		close(fds[wayland_fd].fd);
 	return;
@@ -610,18 +638,17 @@ exit:
 
 int main (int argc, char *argv[])
 {
-	wl_list_init(&context.outputs);
+	init_signals();
+
 	context.ret = EXIT_FAILURE;
 	context.loop = true;
 	context.verbosity = 0;
-
 	context.dimensions.center_size = 165; /* About the size of xclock, at least on my machine. */
 	context.exclusive_zone = -1;
 	context.input = true;
 	context.snap = false;
 	context.layer = ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY;
 	context.anchor = 0; /* Center */
-	set_string(&context.namespace, "wlclock");
 	context.border_bottom = context.border_top
 		= context.border_left = context.border_right = 1;
 	context.radius_bottom_left = context.radius_bottom_right
@@ -630,6 +657,10 @@ int main (int argc, char *argv[])
 		= context.margin_left = context.margin_right = 0;
 	context.marking_width = 1;
 	context.hand_width = 0;
+
+	wl_list_init(&context.outputs);
+	set_string(&context.namespace, "wlclock");
+
 	colour_from_string(&context.background_colour, "#FFFFFF");
 	colour_from_string(&context.border_colour,     "#000000");
 	colour_from_string(&context.clock_colour,      "#000000");
@@ -644,7 +675,6 @@ int main (int argc, char *argv[])
 	context.dimensions.center_x = context.border_left;
 	context.dimensions.center_y = context.border_top;
 
-
 	clocklog(1, "[main] wlclock: version=%s\n"
 			"[main] Default dimensions: size=%d cx=%d cy=%d w=%d h=%d\n",
 			VERSION, context.dimensions.center_size,
@@ -660,6 +690,7 @@ exit:
 	finish_wayland();
 	free_if_set(context.output);
 	free_if_set(context.namespace);
+	clocklog(1, "[main] Bye!\n");
 	return context.ret;
 }