You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2023/09/26 02:13:06 UTC

[nuttx] branch master updated: pthread_once: g_lock may lead deadlock

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new b6693065e7 pthread_once: g_lock may lead deadlock
b6693065e7 is described below

commit b6693065e73206b0c318fd7bf2882e135698382d
Author: hujun5 <hu...@xiaomi.com>
AuthorDate: Thu Sep 21 17:36:27 2023 +0800

    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 <hu...@xiaomi.com>
---
 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;
+        }
 
-  /* The init_routine has already been called.
-   * Restore pre-emption and return.
-   */
+      pthread_mutex_unlock(&once_control->mutex);
+    }
 
-  nxrmutex_unlock(&g_lock);
   return OK;
 }