From b6693065e73206b0c318fd7bf2882e135698382d Mon Sep 17 00:00:00 2001 From: hujun5 Date: Thu, 21 Sep 2023 17:36:27 +0800 Subject: [PATCH] pthread_once: g_lock may lead deadlock For programs with the dependencies logic in pthread_once callback , using global locks may cause deadlock: task A pthread_once() | |-> nxrmutex_lock(&g_lock); -> init_routine(); // callback to wait task B task B pthread_once() | ->nxrmutex_lock(&g_lock); // Deadlock ->init_routine(); // hold resource to wake up task A Signed-off-by: hujun5 --- include/pthread.h | 13 ++++++++++--- libs/libc/pthread/pthread_once.c | 27 ++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/include/pthread.h b/include/pthread.h index e7e195983e..b9fc93ae0d 100644 --- a/include/pthread.h +++ b/include/pthread.h @@ -136,7 +136,7 @@ /* Used to initialize a pthread_once_t */ -#define PTHREAD_ONCE_INIT (false) +#define PTHREAD_ONCE_INIT {false, PTHREAD_MUTEX_INITIALIZER} /* This is returned by pthread_barrier_wait. It must not match any errno * in errno.h @@ -371,8 +371,14 @@ typedef struct pthread_barrier_s pthread_barrier_t; # define __PTHREAD_BARRIER_T_DEFINED 1 #endif +struct pthread_once_s +{ + bool done; + pthread_mutex_t mutex; +}; + #ifndef __PTHREAD_ONCE_T_DEFINED -typedef bool pthread_once_t; +typedef struct pthread_once_s pthread_once_t; # define __PTHREAD_ONCE_T_DEFINED 1 #endif @@ -839,7 +845,8 @@ typedef FAR struct pthread_spinlock_s pthread_spinlock_t; #endif /* CONFIG_PTHREAD_SPINLOCKS */ #ifndef __PTHREAD_ONCE_T_DEFINED -typedef bool pthread_once_t; +struct pthread_once_s; +typedef struct pthread_once_s pthread_once_t; # define __PTHREAD_ONCE_T_DEFINED 1 #endif diff --git a/libs/libc/pthread/pthread_once.c b/libs/libc/pthread/pthread_once.c index 2862ddba2f..8d3a38f178 100644 --- a/libs/libc/pthread/pthread_once.c +++ b/libs/libc/pthread/pthread_once.c @@ -61,8 +61,6 @@ * ****************************************************************************/ -static rmutex_t g_lock = NXRMUTEX_INITIALIZER; - int pthread_once(FAR pthread_once_t *once_control, CODE void (*init_routine)(void)) { @@ -73,25 +71,20 @@ int pthread_once(FAR pthread_once_t *once_control, return EINVAL; } - /* Prohibit pre-emption while we test and set the once_control. */ - - nxrmutex_lock(&g_lock); - - if (!*once_control) + if (!once_control->done) { - *once_control = true; + pthread_mutex_lock(&once_control->mutex); - /* Call the init_routine with pre-emption enabled. */ + if (!once_control->done) + { + /* Call the init_routine with pre-emption enabled. */ - init_routine(); - nxrmutex_unlock(&g_lock); - return OK; + init_routine(); + once_control->done = true; + } + + pthread_mutex_unlock(&once_control->mutex); } - /* The init_routine has already been called. - * Restore pre-emption and return. - */ - - nxrmutex_unlock(&g_lock); return OK; }