From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: rpki-client: cleanup mutexes and conditional variables
To: Philip Guenther <guenther@gmail.com>
Cc: job@openbsd.org, tech@openbsd.org
Date: Tue, 24 Jun 2025 11:02:22 +0200

> From: Philip Guenther <guenther@gmail.com>
> Date: Mon, 23 Jun 2025 22:39:48 -0700
> Content-Type: text/plain; charset="UTF-8"
> 
> 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.

But still a bit of a weird thing to do.

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