From: Philip Guenther <guenther@gmail.com>
Subject: Re: rpki-client: cleanup mutexes and conditional variables
To: Job Snijders <job@openbsd.org>, tech@openbsd.org
Date: Mon, 23 Jun 2025 22:39:48 -0700

On Mon, Jun 23, 2025 at 10:31 PM Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:
>
> On Mon, Jun 23, 2025 at 11:25:30PM +0000, Job Snijders wrote:
> > Free the pthreads mutex and conditional variable memory allocations.
> > Spotted by tb@
> >
> > OK?
>
> All of those use static initalisation. So it is unclear to me if this
> works on all systems. Documentation is very unclear here.

If so, that is a failure of our documentation.  To quote POSIX's
description of pthread_mutex_{destroy,init}():

     It shall be safe to destroy an initialized mutex that is unlocked.
...
     In cases where default mutex attributes are appropriate, the macro
     PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes. The
effect shall be
     equivalent to dynamic initialization by a call to
pthread_mutex_init() with parameter attr
     specified as NULL, except that no error checks are performed.

Ergo, it is safe to call pthread_mutex_destroy() on an (unlocked)
mutex object initialized with PTHREAD_MUTEX_INITIALIZER.


Philip Guenther





>
> > Index: parser.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> > diff -u -p -r1.158 parser.c
> > --- parser.c  23 Jun 2025 22:01:14 -0000      1.158
> > +++ parser.c  23 Jun 2025 23:21:50 -0000
> > @@ -1301,6 +1301,15 @@ proc_parser(int fd, int nthreads)
> >
> >       msgbuf_free(inbufq);
> >       ibufq_free(globalmsgq);
> > +
> > +     if (pthread_mutex_destroy(&globalq_mtx) != 0)
> > +             errx(1, "pthread_mutex_destroy");
> > +     if (pthread_cond_destroy(&globalq_cond) != 0)
> > +             errx(1, "pthread_cond_destroy");
> > +     if (pthread_mutex_destroy(&globalmsgq_mtx) != 0)
> > +             errx(1, "pthread_mutex_destroy");
> > +     if (pthread_cond_destroy(&globalmsgq_cond) != 0)
> > +             errx(1, "pthread_cond_destroy");
> >
> >       if (certid > CERTID_MAX)
> >               errx(1, "processing incomplete: too many certificates");
> >
>
> --
> :wq Claudio
>