Found this while tracing a memory leak in filter-dkimsign, thanks to
libcrypto. The mutex in pthread_once_t is never destroyed, so the
memory allocated inside the mutex is never released.

The diff below was inspired by Ed Schouten and switches form mutex to
futex to prevent any memory allocation. I've run with it for about a
week without issues and tb@ has given it some beating on sparc64.
However I'm no expert in this area and scrutiny from people with more
experience in this area and testing in general would be appreciated.

This implementation has one shortcoming I can see, namely[0]:
The pthread_once() function is not a cancellation point. However, if
init_routine is a cancellation point and is canceled, the effect on
once_control shall be as if pthread_once() was never called.
It doesn't handle this situation by waking up the sleeping threads.
However, the current code doesn't handle this requirement either:
#include <stdio.h>
#include <pthread.h>

pthread_once_t once = PTHREAD_ONCE_INIT;

void
init(void)
{
        printf("init\n");
        pthread_exit(NULL);
}

void *
routine(void *arg)
{
        pthread_once(&once, init);
        printf("%s\n", __func__);
        return NULL;
}

int
main(int argc, char *argv[])
{
        pthread_t thread;
        pthread_create(&thread, NULL, routine, NULL);
        pthread_once(&once, init);
        printf("%s\n", __func__);
        return 0;
}

Since our current code shows similar behaviour without real world
problems and all the solutions that I can come up with are racey I think 
this diff can stand on its own and some other brave soul can fix this
requirement at a later time. :-)

OK?

martijn@

[0] https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_once.html

Index: include/pthread.h
===================================================================
RCS file: /cvs/src/include/pthread.h,v
retrieving revision 1.4
diff -u -p -r1.4 pthread.h
--- include/pthread.h   5 Mar 2018 01:15:26 -0000       1.4
+++ include/pthread.h   2 May 2021 11:24:17 -0000
@@ -136,20 +136,13 @@ typedef void      *(*pthread_startroutine_t)(
  * Once definitions.
  */
 struct pthread_once {
-       int             state;
-       pthread_mutex_t mutex;
+       volatile unsigned int   state;
 };
 
 /*
- * Flags for once initialization.
- */
-#define PTHREAD_NEEDS_INIT  0
-#define PTHREAD_DONE_INIT   1
-
-/*
  * Static once initialization values. 
  */
-#define PTHREAD_ONCE_INIT   { PTHREAD_NEEDS_INIT, PTHREAD_MUTEX_INITIALIZER }
+#define PTHREAD_ONCE_INIT   { 0 }
 
 /*
  * Static initialization values. 
Index: lib/libc/thread/rthread_once.c
===================================================================
RCS file: /cvs/src/lib/libc/thread/rthread_once.c,v
retrieving revision 1.3
diff -u -p -r1.3 rthread_once.c
--- lib/libc/thread/rthread_once.c      4 Nov 2017 22:53:57 -0000       1.3
+++ lib/libc/thread/rthread_once.c      2 May 2021 11:24:17 -0000
@@ -18,15 +18,25 @@
 
 #include <pthread.h>
 
+#include "synch.h"
+
 int
 pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
 {
-       pthread_mutex_lock(&once_control->mutex);
-       if (once_control->state == PTHREAD_NEEDS_INIT) {
+       switch (atomic_cas_uint(&(once_control->state), 0, 1)) {
+       case 0:
                init_routine();
-               once_control->state = PTHREAD_DONE_INIT;
+               atomic_inc_int(&once_control->state);
+               _wake(&once_control->state, INT_MAX);
+               break;
+       case 1:
+               do {
+                       _twait(&once_control->state, 1, 0, NULL);
+               } while (once_control->state != 2);
+               break;
+       default:
+               break;
        }
-       pthread_mutex_unlock(&once_control->mutex);
 
-       return (0);
+       return 0;
 }


Reply via email to