From 012b9c3dfef0ac5c48331f05a05cf14b2c992708 Mon Sep 17 00:00:00 2001
From: patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3>
Date: Fri, 10 Sep 2010 02:34:19 +0000
Subject: [PATCH] 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
---
 ChangeLog                |  5 +++++
 Documentation/NuttX.html | 27 ++++++++++++++++++++++-----
 sched/sem_holder.c       |  5 +++--
 sched/sem_wait.c         | 21 +++++++++++++++------
 sched/sem_waitirq.c      | 19 ++++++++++++++++++-
 5 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2499469a339..96f0e79bf50 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1274,3 +1274,8 @@
 	  reading from offsets into this file that correspond to virtual
 	  fault addresses.  up_fillpage.c implements logic to perform page
 	  files using the CONFIG_PAGING_BINPATH file.
+	* configs/mbed - Add configuration to support the mbed.org LPC1768
+	  board (Contributed by Dave Marples).
+	* sched/sem_wait.c and sem_waitirq.c - Eliminate a race condition
+	  that can occur when a semaphore wait is interrupt by a signal.
+	  (see email thread: http://tech.groups.yahoo.com/group/nuttx/message/530)
diff --git a/Documentation/NuttX.html b/Documentation/NuttX.html
index 427f08687bb..403d0d69776 100644
--- a/Documentation/NuttX.html
+++ b/Documentation/NuttX.html
@@ -8,7 +8,7 @@
   <tr align="center" bgcolor="#e4e4e4">
     <td>
       <h1><big><font color="#3c34ec"><i>NuttX RTOS</i></font></big></h1>
-      <p>Last Updated: September 8, 2010</p>
+      <p>Last Updated: September 9, 2010</p>
     </td>
   </tr>
 </table>
@@ -1262,10 +1262,19 @@
   <td>
     <p>
       <b>NXP LPC1768</b>.
-      This port uses the Nucleus 2G board from <a href="http://www.2g-eng.com/">2G Engineering</a>
-      featuring the NXP LPC1768 MCU.
-      This port uses a GNU arm-elf or arm-eabi toolchain* under either Linux or Cygwin (with native Windows GNU
-      tools or Cygwin-based GNU tools).
+      Configurations are available for two boards:
+      <ul>
+        <li>
+          The Nucleus 2G board from <a href="http://www.2g-eng.com/">2G Engineering</a>, and
+        </li>
+        <li>
+          The mbed board from <a href="http://mbed.org">mbed.org</a> (Contributed by Dave Marples).
+        </li>
+      </ul>
+    </p>
+    <p>
+      Both boards feature the NXP LPC1768 MCU and a GNU arm-elf or arm-eabi toolchain* under
+      either Linux or Cygwin (with native Windows GNU tools or Cygwin-based GNU tools).
     </p>
     <ul>
       <p>
@@ -1282,6 +1291,9 @@
         However, due to some technical reasons, neither the SPI nor the USB device drivers are fully verified.
         (Although it has been reported to me that the SPI microSD is functional on other platforms).
       </p>
+      <p>
+        Support for the mbed board was contributed by Dave Marples and released in NuttX-5.11.
+      </p>
       <p>
         <b>Development Environments:</b>
         1) Linux with native Linux GNU toolchain, 2) Cygwin with Cygwin GNU toolchain, or 3) Cygwin
@@ -1957,6 +1969,11 @@ nuttx-5.11 2010-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 	  reading from offsets into this file that correspond to virtual
 	  fault addresses.  up_fillpage.c implements logic to perform page
 	  files using the CONFIG_PAGING_BINPATH file.
+	* configs/mbed - Add configuration to support the mbed.org LPC1768
+	  board (Contributed by Dave Marples).
+	* sched/sem_wait.c and sem_waitirq.c - Eliminate a race condition
+	  that can occur when a semaphore wait is interrupt by a signal.
+	  (see email thread: http://tech.groups.yahoo.com/group/nuttx/message/530)
 
 pascal-2.1 2010-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 
diff --git a/sched/sem_holder.c b/sched/sem_holder.c
index c2e7d34473f..6db6867c83a 100644
--- a/sched/sem_holder.c
+++ b/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/sched/sem_wait.c b/sched/sem_wait.c
index 747d7cd48bb..12d8c98ca6e 100644
--- a/sched/sem_wait.c
+++ b/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/sched/sem_waitirq.c b/sched/sem_waitirq.c
index 67366ad412f..c37bc57db96 100644
--- a/sched/sem_waitirq.c
+++ b/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;