Index | Thread | Search

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

Download raw body.

Thread
  • Philip Guenther:

    rpki-client: cleanup mutexes and conditional variables

    • Mark Kettenis:

      rpki-client: cleanup mutexes and conditional variables

  • > 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
    > >
    > 
    > 
    
    
  • Philip Guenther:

    rpki-client: cleanup mutexes and conditional variables