tests/sighook: Fix Valgrind compatibility

Valgrind does not deliver async signals in a timely manner; by default,
it polls for new signals every 1,000 basic blocks.  That means we can
get SIGALRM delivered after timer_delete(), or kill(SIGSEGV) never
delivered after pause().

Fix the timer_delete() issue by reordering the cleanup path.  Valgrind
always polls pending signals after pthread_sigmask(), so call that
between timer_delete() and sigunhook().

Fix the pause() issue by sleeping in a loop instead.

Note that --fair-sched=yes is required to avoid starvation of the
background thread.

Link: https://bugs.kde.org/show_bug.cgi?id=492678
Link: https://bugs.kde.org/show_bug.cgi?id=343357
Link: https://bugs.kde.org/show_bug.cgi?id=498936

1 files changed, 43 insertions, 39 deletions

diff --git a/tests/sighook.c b/tests/sighook.c
index ba1c424..82e0ae5 100644
--- a/tests/sighook.c
+++ b/tests/sighook.c
@@ -20,11 +20,6 @@
 /** Counts SIGALRM deliveries. */
 static atomic size_t count = 0;
 
-/** Keeps the background thread alive. */
-static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
-static bool done = false;
-
 /** SIGALRM handler. */
 static void alrm_hook(int sig, siginfo_t *info, void *arg) {
 	fetch_add(&count, 1, relaxed);
@@ -38,6 +33,11 @@ static void alrm_oneshot(int sig, siginfo_t *info, void *arg) {
 	fetch_add(&shots, 1, relaxed);
 }
 
+/** Keeps the background thread alive. */
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+static bool done = false;
+
 /** Background thread that receives signals. */
 static void *hook_thread(void *ptr) {
 	mutex_lock(&mutex);
@@ -68,55 +68,58 @@ static int block_signal(int sig, sigset_t *old) {
 
 /** Tests for sighook(). */
 static void check_hooks(void) {
-	struct sighook *hook = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
+	struct sighook *hook = NULL;
+	struct sighook *oneshot = NULL;
+
+	hook = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
 	if (!bfs_echeck(hook, "sighook(SIGALRM)")) {
 		return;
 	}
 
+	// Create a background thread to receive SIGALRM
+	pthread_t thread;
+	if (!bfs_echeck(thread_create(&thread, NULL, hook_thread, NULL) == 0)) {
+		goto unhook;
+	}
+
+	// Block SIGALRM in this thread so the handler runs concurrently with
+	// sighook()/sigunhook()
+	sigset_t mask;
+	if (!bfs_echeck(block_signal(SIGALRM, &mask) == 0)) {
+		goto unthread;
+	}
+
 	// Check that we can unregister and re-register a hook
 	sigunhook(hook);
 	hook = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
 	if (!bfs_echeck(hook, "sighook(SIGALRM)")) {
-		return;
+		goto unblock;
 	}
 
 	// Test SH_ONESHOT
-	struct sighook *oneshot_hook = sighook(SIGALRM, alrm_oneshot, NULL, SH_ONESHOT);
-	if (!bfs_echeck(oneshot_hook, "sighook(SH_ONESHOT)")) {
-		goto unhook;
+	oneshot = sighook(SIGALRM, alrm_oneshot, NULL, SH_ONESHOT);
+	if (!bfs_echeck(oneshot, "sighook(SH_ONESHOT)")) {
+		goto unblock;
 	}
 
 	// Create a timer that sends SIGALRM every 100 microseconds
-	struct timespec ival = { .tv_nsec = 100 * 1000 };
+	const struct timespec ival = { .tv_nsec = 100 * 1000 };
 	struct timer *timer = xtimer_start(&ival);
-	if (!bfs_echeck(timer)) {
-		goto unhook;
-	}
-
-	// Create a background thread to receive signals
-	pthread_t thread;
-	if (!bfs_echeck(thread_create(&thread, NULL, hook_thread, NULL) == 0)) {
-		goto untime;
-	}
-
-	// Block SIGALRM in this thread so the handler runs concurrently with
-	// sighook()/sigunhook()
-	sigset_t mask;
-	if (!bfs_echeck(block_signal(SIGALRM, &mask) == 0)) {
-		goto untime;
+	if (!bfs_echeck(timer, "xtimer_start()")) {
+		goto unblock;
 	}
 
 	// Rapidly register/unregister SIGALRM hooks
 	size_t alarms;
 	while (alarms = load(&count, relaxed), alarms < 1000) {
 		size_t nshots = load(&shots, relaxed);
-		bfs_echeck(nshots <= 1);
+		bfs_check(nshots <= 1);
 		if (alarms > 1) {
-			bfs_echeck(nshots == 1);
+			bfs_check(nshots == 1);
 		}
 		if (alarms >= 500) {
-			sigunhook(oneshot_hook);
-			oneshot_hook = NULL;
+			sigunhook(oneshot);
+			oneshot = NULL;
 		}
 
 		struct sighook *next = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
@@ -128,22 +131,22 @@ static void check_hooks(void) {
 		hook = next;
 	}
 
+	// Stop the timer
+	xtimer_stop(timer);
+unblock:
+	// Restore the old signal mask
+	errno = pthread_sigmask(SIG_SETMASK, &mask, NULL);
+	bfs_echeck(errno == 0, "pthread_sigmask()");
+unthread:
 	// Quit the background thread
 	mutex_lock(&mutex);
 	done = true;
 	mutex_unlock(&mutex);
 	cond_signal(&cond);
 	thread_join(thread, NULL);
-
-	// Restore the old signal mask
-	errno = pthread_sigmask(SIG_SETMASK, &mask, NULL);
-	bfs_echeck(errno == 0, "pthread_sigmask()");
-untime:
-	// Stop the timer
-	xtimer_stop(timer);
 unhook:
 	// Unregister the SIGALRM hooks
-	sigunhook(oneshot_hook);
+	sigunhook(oneshot);
 	sigunhook(hook);
 }
 
@@ -204,8 +207,9 @@ static void check_sigexit(int sig) {
 		bfs_everify(xwrite(ready[1], "A", 1) == 1);
 
 		// Wait until we're killed
+		const struct timespec dur = { .tv_nsec = 1 };
 		while (true) {
-			pause();
+			nanosleep(&dur, NULL);
 		}
 	}
 }