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

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


The following commit(s) were added to refs/heads/releases/12.2 by this push:
     new ec145d5823 sched/pthread: fix race condition on pthread_cond_wait()
ec145d5823 is described below

commit ec145d5823f47eb662a88648a0cef2a40a2e5a39
Author: chao an <anc...@xiaomi.com>
AuthorDate: Tue Jun 27 22:32:04 2023 +0800

    sched/pthread: fix race condition on pthread_cond_wait()
    
    pthread_cond_wait() should be an atomic operation in the mutex lock/unlock.
    Since the sched_lock() has been wrongly deleted in the previous commit,
    the context switch will occurred after the mutex was unlocked:
    
    --------------------------------------------------------------------
      Task1(Priority 100)             |      Task2(Priority 101)
                                      |
      pthread_mutex_lock(mutex);      |
      |                               |
      pthread_cond_wait(cond, mutex)  |
      |  |                            |
      |  |                            |
      |  ->enter_critical_section()   |
      |  ->pthread_mutex_give(mutex)  | ----> pthread_mutex_lock(mutex);    // 
contex switch to high priority task
      |                               |       pthread_cond_signal(cond);    // 
signal before wait
      |                               | <---- pthread_mutex_unlock(mutex);  // 
switch back to original task
      |  ->pthread_sem_take(cond->sem)|                                     // 
try to wait the signal, Deadlock.
      |  ->leave_critical_section()   |
      |
      |  ->pthread_mutex_take(mutex)  |
      |                               |
      pthread_mutex_lock(mutex);      |
    ---------------------------------------------------------------------
    
    This PR will bring back sched_lock()/sched_unlock() to avoid context switch 
to ensure atomicity
    
    Signed-off-by: chao an <anc...@xiaomi.com>
---
 sched/pthread/pthread_condwait.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sched/pthread/pthread_condwait.c b/sched/pthread/pthread_condwait.c
index c94b8edaa5..412814d339 100644
--- a/sched/pthread/pthread_condwait.c
+++ b/sched/pthread/pthread_condwait.c
@@ -94,6 +94,7 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR 
pthread_mutex_t *mutex)
       sinfo("Give up mutex / take cond\n");
 
       flags = enter_critical_section();
+      sched_lock();
       mutex->pid = INVALID_PROCESS_ID;
 #ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
       mflags     = mutex->flags;
@@ -116,6 +117,7 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR 
pthread_mutex_t *mutex)
           ret = status;
         }
 
+      sched_unlock();
       leave_critical_section(flags);
 
       /* Reacquire the mutex.

Reply via email to