Hmm, as long as the Boolean being checked (quit?) is both checked and
changed with the mutex held, there should be no race. How did the hung
thread see quit not true before waiting but the signaling thread set or see
it as true before signaling?  Smells like quit is being set outside the
mutex…

(Holding the mutex while signaling is fine of course, but feels like it
should have been held even earlier)

Philip

On Tuesday, July 21, 2026, Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:

> 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);
>
>