Fix race condition when semaphore wait is interrupted by a signl

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@2935 42af7a65-404d-4744-a932-0658087f49c3

3 files changed, 36 insertions, 9 deletions

diff --git a/nuttx/sched/sem_holder.c b/nuttx/sched/sem_holder.c
index c2e7d3447..6db6867c8 100644
--- a/nuttx/sched/sem_holder.c
+++ b/nuttx/sched/sem_holder.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * sched/sem_holder.c
  *
- *   Copyright (C) 2009 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2009-2010 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -818,7 +818,8 @@ void sem_restorebaseprio(FAR _TCB *stcb, FAR sem_t *sem)
  * Description:
  *   Called from sem_post() after a thread that was waiting for a semaphore
  *   count was awakened because of a signal and the semaphore wait has been
- *   canceled.
+ *   canceled.  This function restores the correct thread priority of each
+ *   holder of the semaphore.
  *
  * Parameters:
  *   sem - A reference to the semaphore no longer being waited for
diff --git a/nuttx/sched/sem_wait.c b/nuttx/sched/sem_wait.c
index 747d7cd48..12d8c98ca 100644
--- a/nuttx/sched/sem_wait.c
+++ b/nuttx/sched/sem_wait.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * sched/sem_wait.c
  *
- *   Copyright (C) 2007-2009 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2010 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -178,6 +178,20 @@ int sem_wait(FAR sem_t *sem)
            * assigned to this thread of execution, or (2) the semaphore wait
            * has been interrupted by a signal.  We can detect the latter case
            * be examining the errno value.
+           *
+           * In the event that the semaphore wait was interrupt was interrupted
+           * by a signal, certain semaphore clean-up operations have already been
+           * performed (see sem_waitirq.c).  Specifically:
+           *
+           * - sem_canceled() was called to restore the priority of all threads
+           *   that hold a reference to the semaphore,
+           * - The semaphore count was decremented, and
+           * - tcb->waitsem was nullifed.
+           *
+           * It is necesaary to do these things in sem_waitirq.c because a long
+           * time may elapse between the time that the signal was issued and
+           * this thread is awakened and this leaves a door open to several
+           * race conditions.
            */
 
           if (errno != EINTR)
@@ -187,11 +201,6 @@ int sem_wait(FAR sem_t *sem)
               sem_addholder(sem);
               ret = OK;
             }
-          else
-            {
-              sem_canceled(sem);	            
-              sem->semcount++;
-            }
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
           sched_unlock();
diff --git a/nuttx/sched/sem_waitirq.c b/nuttx/sched/sem_waitirq.c
index 67366ad41..c37bc57db 100644
--- a/nuttx/sched/sem_waitirq.c
+++ b/nuttx/sched/sem_waitirq.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * sched/sem_waitirq.c
  *
- *   Copyright (C) 2007-2009 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2010 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -107,6 +107,23 @@ void sem_waitirq(FAR _TCB *wtcb)
 
   if (wtcb->task_state == TSTATE_WAIT_SEM)
     {
+      sem_t *sem = wtcb->waitsem;
+      DEBUGASSERT(sem != NULL && sem->semcount < 0);
+
+      /* Restore the correct priority of all threads that hold references
+       * to this semaphore.
+       */
+
+      sem_canceled(sem);
+
+      /* And increment the count on the semaphore.  This releases the
+       * count that was taken by sem_post().  This count decremented
+       * the semaphore count to negative and caused the thread to be
+       * blocked in the first place.
+       */ 
+
+      sem->semcount++;
+
       /* Indicate that the semaphore wait is over. */
 
       wtcb->waitsem = NULL;