Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
rpki-client: fix possible pthread deadlock on exit
To:
tech@openbsd.org
Date:
Tue, 21 Jul 2026 10:55:05 +0200

Download raw body.

Thread
The interaction between quit and the pthread_cond_broadcast calls in the
exit path are not properly sequenced. Calling the cond_broadcast without
holding its mutex can lead to a case where the broadcast is lost and so
that thread would not wake up. The process would then hang forever in
pthread_join().

Grab the mutex before sending the pthread_cond_broadcast() fixes this
issue since now the receiver is either properly parked or it will
see that quit is true and exit before calling pthread_cond_wait().

Also do an early exit from the event loop if quit is true. There is
no need work through the full event loop in that case.
-- 
:wq Claudio

Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.182 parser.c
--- parser.c	26 Jun 2026 08:05:22 -0000	1.182
+++ parser.c	9 Jul 2026 20:44:10 -0000
@@ -1072,6 +1072,12 @@ parse_worker(void *arg)
 				errx(1, "pthread_cond_wait: %s",
 				    strerror(error));
 		}
+		if (quit) {
+			if ((error = pthread_mutex_unlock(&globalq_mtx)) != 0)
+				errx(1, "pthread_mutex_unlock: %s",
+				    strerror(error));
+			break;
+		}
 		n = 0;
 		while ((entp = TAILQ_FIRST(&globalq)) != NULL) {
 			TAILQ_REMOVE(&globalq, entp, entries);
@@ -1141,6 +1147,8 @@ parse_writer(void *arg)
 			if (error != 0)
 				errx(1, "pthread_mutex_lock: %s",
 				    strerror(error));
+			if (quit)
+				break;
 		}
 
 		if (msgbuf_queuelen(myq) > 0) {
@@ -1279,13 +1287,19 @@ proc_parser(int fd, int nthreads)
 	}
 
 	/* signal all threads */
+	if ((error = pthread_mutex_lock(&globalq_mtx)) != 0)
+		errx(1, "pthread_mutex_lock: %s", strerror(error));
+	if ((error = pthread_mutex_lock(&globalmsgq_mtx)) != 0)
+		errx(1, "pthread_mutex_lock: %s", strerror(error));
+
 	if ((error = pthread_cond_broadcast(&globalq_cond)) != 0)
 		errx(1, "pthread_cond_broadcast: %s", strerror(error));
 	if ((error = pthread_cond_broadcast(&globalmsgq_cond)) != 0)
 		errx(1, "pthread_cond_broadcast: %s", strerror(error));
 
-	if ((error = pthread_mutex_lock(&globalq_mtx)) != 0)
-		errx(1, "pthread_mutex_lock: %s", strerror(error));
+	if ((error = pthread_mutex_unlock(&globalmsgq_mtx)) != 0)
+		errx(1, "pthread_mutex_unlock: %s", strerror(error));
+
 	while ((entp = TAILQ_FIRST(&globalq)) != NULL) {
 		TAILQ_REMOVE(&globalq, entp, entries);
 		entity_free(entp);