Fix a potential race condition

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

3 files changed, 43 insertions, 25 deletions

diff --git a/nuttx/sched/work_cancel.c b/nuttx/sched/work_cancel.c
index d3a79f0b6..92393dec3 100755
--- a/nuttx/sched/work_cancel.c
+++ b/nuttx/sched/work_cancel.c
@@ -79,7 +79,9 @@
  * Name: work_cancel
  *
  * Description:
- *   Cancel previously queued work.
+ *   Cancel previously queued work.  This removes work from the work queue.
+ *   After work has been canceled, it may be re-queue by calling work_queue()
+ *   again.
  *
  * Input parameters:
  *   work   - The previously queue work structure to cancel
diff --git a/nuttx/sched/work_queue.c b/nuttx/sched/work_queue.c
index 46afdc1d8..144c32178 100755
--- a/nuttx/sched/work_queue.c
+++ b/nuttx/sched/work_queue.c
@@ -83,6 +83,13 @@
  *   Queue work to be performed at a later time.  All queued work will be
  *   performed on the worker thread of of execution (not the caller's).
  *
+ *   The work structure is allocated by caller, but completely managed by
+ *   the work queue logic.  The caller should never modify the contents of
+ *   the work queue structure; the caller should not call work_queue()
+ *   again until either (1) the previous work has been performed and removed
+ *   from the queue, or (2) work_cancel() has been called to cancel the work
+ *   and remove it from the work queue.
+ *
  * Input parameters:
  *   work   - The work structure to queue
  *   worker - The worker callback to be invoked.  The callback will invoked
diff --git a/nuttx/sched/work_thread.c b/nuttx/sched/work_thread.c
index 35a7f55ca..d1f415d77 100755
--- a/nuttx/sched/work_thread.c
+++ b/nuttx/sched/work_thread.c
@@ -101,6 +101,8 @@ struct dq_queue_s g_work;
 int work_thread(int argc, char *argv[])
 {
   volatile FAR struct work_s *work;
+  worker_t  worker;
+  FAR void *arg;
   irqstate_t flags;
 
   /* Loop forever */
@@ -130,37 +132,44 @@ int work_thread(int argc, char *argv[])
       work  = (FAR struct work_s *)g_work.head;
       while (work)
         {
-           /* Is this work ready?  It is ready if there is no delay or if
-            * the delay has elapsed.
-            */
+          /* Is this work ready?  It is ready if there is no delay or if
+           * the delay has elapsed.
+           */
 
-           if (work->delay == 0 || g_system_timer - work->qtime > work->delay)
-             {
-               /* Remove the ready-to-execute work from the list */
+          if (work->delay == 0 || g_system_timer - work->qtime > work->delay)
+            {
+              /* Remove the ready-to-execute work from the list */
 
-               (void)dq_rem((struct dq_entry_s *)work, &g_work);
+              (void)dq_rem((struct dq_entry_s *)work, &g_work);
 
-               /* Do the work.  Re-enable interrupts while the work is being
-                * performed... we don't have any idea how long that will take!
-                */
+              /* Extract the work description from the entry (in case the work
+               * instance by the re-used after it has been de-queued).
+               */
 
-               irqrestore(flags);
-               work->worker(work->arg);
+              worker = work->worker;
+              arg    = work->arg;
 
-               /* Now, unfortunately, since we re-enabled interrupts we don't
-                * the state of the work list and we will have to start back at
-                * the head of the list.
-                */
+              /* Do the work.  Re-enable interrupts while the work is being
+               * performed... we don't have any idea how long that will take!
+               */
 
-               flags = irqsave();
-               work  = (FAR struct work_s *)g_work.head;
-             }
-           else
-             {
-               /* This one is not ready, try the next in the list. */
+              irqrestore(flags);
+              worker(arg);
 
-               work = (FAR struct work_s *)work->dq.flink;
-             }
+              /* Now, unfortunately, since we re-enabled interrupts we don't
+               * know the state of the work list and we will have to start
+               * back at the head of the list.
+               */
+
+              flags = irqsave();
+              work  = (FAR struct work_s *)g_work.head;
+            }
+          else
+            {
+              /* This one is not ready, try the next in the list. */
+
+              work = (FAR struct work_s *)work->dq.flink;
+            }
         }
       irqrestore(flags);
     }