From 698597a838fe89cb4c2e2a73f71be3c5bdce0802 Mon Sep 17 00:00:00 2001
From: Gregory Nutt <gnutt@nuttx.org>
Date: Sat, 10 Dec 2016 14:39:19 -0600
Subject: [PATCH] task_delete() now obeys all cancellation point semantics.

---
 Documentation/NuttxUserGuide.html |  22 ++++-
 sched/pthread/pthread_cancel.c    |  14 +--
 sched/task/task_delete.c          | 146 ++++++++++++++++++++----------
 sched/task/task_restart.c         |   2 +-
 4 files changed, 126 insertions(+), 58 deletions(-)

diff --git a/Documentation/NuttxUserGuide.html b/Documentation/NuttxUserGuide.html
index ada2c6d9e1a..c4bc4c839d9 100644
--- a/Documentation/NuttxUserGuide.html
+++ b/Documentation/NuttxUserGuide.html
@@ -13,7 +13,7 @@
       <h1><big><font color="#3c34ec"><i>NuttX Operating System<p>User's Manual</i></font></big></h1>
       <p><small>by</small></p>
       <p>Gregory Nutt<p>
-      <p>Last Updated: December 9, 2016</p>
+      <p>Last Updated: December 10, 2016</p>
     </td>
   </tr>
 </table>
@@ -469,8 +469,21 @@ int task_delete(pid_t pid);
 
 <p>
   <b>Description:</b>
-  This function causes a specified task to cease to exist -- its stack and TCB will be deallocated.
-  This function is the companion to task_create().
+  This function causes a specified task to cease to exist.
+  Its  stack and TCB will be deallocated. 
+  This function is the companion to <code>task_create()</code>.
+  This is the version of the function exposed to the user;
+  it is simply a wrapper around the internal, <code>task_terminate()</code> function.
+</p>
+<p>
+  The logic in this function only deletes non-running tasks.
+  If the  <code>pid</code> parameter refers to to the currently runing task, then processing is redirected to <code>exit()</code>. 
+  This can only happen if a task calls <code>task_delete()</code> in order to delete itself.
+</p>
+<p>
+  This function obeys the semantics of pthread cancellation:
+  task deletion is deferred if cancellation is disabled or if deferred cancellation is supported (with cancellation points enabled).
+</p>
 <p>
 <b>Input Parameters:</b>
 <ul>
@@ -485,7 +498,8 @@ int task_delete(pid_t pid);
 <ul>
   <li>
     <code>OK</code>, or <code>ERROR</code> if the task cannot be deleted.
-    This function can fail if the provided pid does not correspond to a task (<a href="#ErrnoAccess"><code>errno</code></a> is not set).
+    The <a href="#ErrnoAccess"><code>errno</code></a> is set to indicate the nature of the failure.
+    This function can fail, for example, if the provided pid does not correspond to a currently executing task.
   </li>
 </ul>
 <p>
diff --git a/sched/pthread/pthread_cancel.c b/sched/pthread/pthread_cancel.c
index b8f89ead30d..6b974629a01 100644
--- a/sched/pthread/pthread_cancel.c
+++ b/sched/pthread/pthread_cancel.c
@@ -58,7 +58,7 @@ int pthread_cancel(pthread_t thread)
 
   /* First, make sure that the handle references a valid thread */
 
-  if (!thread)
+  if (thread == 0)
     {
       /* pid == 0 is the IDLE task.  Callers cannot cancel the
        * IDLE task.
@@ -77,11 +77,13 @@ int pthread_cancel(pthread_t thread)
       return ESRCH;
     }
 
+  /* Only pthreads should use this interface */
+
   DEBUGASSERT((tcb->cmn.flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD);
 
   /* Check to see if this thread has the non-cancelable bit set in its
    * flags. Suppress context changes for a bit so that the flags are stable.
-   * (the flags should not change in interrupt handling.
+   * (the flags should not change in interrupt handling).
    */
 
   sched_lock();
@@ -110,7 +112,7 @@ int pthread_cancel(pthread_t thread)
 
   if ((tcb->cmn.flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
     {
-      /* Then we cannot cancel the thread asynchronoulsy.  Mark the cancellation
+      /* Then we cannot cancel the thread asynchronously.  Mark the cancellation
        * as pending.
        */
 
@@ -162,9 +164,7 @@ int pthread_cancel(pthread_t thread)
 
   (void)pthread_completejoin((pid_t)thread, PTHREAD_CANCELED);
 
-  /* Then let pthread_delete do the real work */
+  /* Then let task_terminate do the real work */
 
-  task_delete((pid_t)thread);
-  return OK;
+  return task_terminate((pid_t)thread, false);
 }
-
diff --git a/sched/task/task_delete.c b/sched/task/task_delete.c
index 50746f23870..755584fd0c1 100644
--- a/sched/task/task_delete.c
+++ b/sched/task/task_delete.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * sched/task/task_delete.c
  *
- *   Copyright (C) 2007-2009, 2011-2013 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2009, 2011-2013, 2016 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -40,6 +40,7 @@
 #include <nuttx/config.h>
 
 #include <stdlib.h>
+#include <errno.h>
 
 #include <nuttx/sched.h>
 
@@ -64,37 +65,111 @@
  *   redirected to exit().  This can only happen if a task calls task_delete()
  *   in order to delete itself.
  *
- *   In fact, this function (and task_terminate) are the final functions
- *   called all task termination sequences.  task_delete may be called
- *   from:
- *
- *   - task_restart(),
- *   - pthread_cancel(),
- *   - and directly from user code.
- *
- *   Other exit paths (exit(), _eixt(), and pthread_exit()) will go through
- *   task_terminate()
+ *   This function obeys the semantics of pthread cancellation:  task
+ *   deletion is deferred if cancellation is disabled or if deferred
+ *   cancellation is supported (with cancellation points enabled).
  *
  * Inputs:
  *   pid - The task ID of the task to delete.  A pid of zero
  *         signifies the calling task.
  *
  * Return Value:
- *   OK on success; or ERROR on failure
- *
- *   This function can fail if the provided pid does not correspond to a
- *   task (errno is not set)
+ *   OK on success; or ERROR on failure with the errno variable set
+ *   appropriately.
  *
  ****************************************************************************/
 
 int task_delete(pid_t pid)
 {
+  FAR struct tcb_s *dtcb;
   FAR struct tcb_s *rtcb;
+  int ret;
+
+  /* Check if the task to delete is the calling task:  PID=0 means to delete
+   * the calling task.  In this case, task_delete() is much like exit()
+   * except that it obeys the cancellation semantics.
+   */
+
+  rtcb = this_task();
+  if (pid == 0)
+    {
+      pid = rtcb->pid;
+    }
+
+  /* Get the TCB of the task to be deleted */
+
+  dtcb = (FAR struct tcb_s *)sched_gettcb(pid);
+  if (dtcb == NULL)
+    {
+      /* The pid does not correspond to any known thread.  The task
+       * has probably already exited.
+       */
+
+      set_errno(ESRCH);
+      return ERROR;
+    }
+
+  /* Only tasks and kernel threads should use this interface */
+
+  DEBUGASSERT((dtcb->flags & TCB_FLAG_TTYPE_MASK) != TCB_FLAG_TTYPE_PTHREAD);
+
+  /* Check to see if this task has the non-cancelable bit set in its
+   * flags. Suppress context changes for a bit so that the flags are stable.
+   * (the flags should not change in interrupt handling).
+   */
+
+  sched_lock();
+  if ((dtcb->flags & TCB_FLAG_NONCANCELABLE) != 0)
+    {
+      /* Then we cannot cancel the thread now.  Here is how this is
+       * supposed to work:
+       *
+       * "When cancelability is disabled, all cancels are held pending
+       *  in the target thread until the thread changes the cancelability.
+       *  When cancelability is deferred, all cancels are held pending in
+       *  the target thread until the thread changes the cancelability, calls
+       *  a function which is a cancellation point or calls pthread_testcancel(),
+       *  thus creating a cancellation point. When cancelability is asynchronous,
+       *  all cancels are acted upon immediately, interrupting the thread with its
+       *  processing."
+       */
+
+      dtcb->flags |= TCB_FLAG_CANCEL_PENDING;
+      sched_unlock();
+      return OK;
+    }
+
+#ifdef CONFIG_CANCELLATION_POINTS
+  /* Check if this task supports deferred cancellation */
+
+  if ((dtcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
+    {
+      /* Then we cannot cancel the task asynchronously.  Mark the cancellation
+       * as pending.
+       */
+
+      dtcb->flags |= TCB_FLAG_CANCEL_PENDING;
+
+      /* If the task is waiting at a cancellation point, then notify of the
+       * cancellation thereby waking the task up with an ECANCELED error.
+       *
+       * REVISIT: is locking the scheduler sufficent in SMP mode?
+       */
+
+      if (dtcb->cpcount > 0)
+        {
+          notify_cancellation(dtcb);
+        }
+
+      sched_unlock();
+      return OK;
+    }
+#endif
 
   /* Check if the task to delete is the calling task */
 
-  rtcb = this_task();
-  if (pid == 0 || pid == rtcb->pid)
+  sched_unlock();
+  if (pid == rtcb->pid)
     {
       /* If it is, then what we really wanted to do was exit. Note that we
        * don't bother to unlock the TCB since it will be going away.
@@ -103,37 +178,16 @@ int task_delete(pid_t pid)
       exit(EXIT_SUCCESS);
     }
 
-#ifdef CONFIG_CANCELLATION_POINTS
-  /* Check if this task supports deferred cancellation */
-
-  sched_lock();
-  if ((rtcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
-    {
-      /* Then we cannot cancel the task asynchronoulsy.  Mark the cancellation
-       * as pending.
-       */
-
-      rtcb->flags |= TCB_FLAG_CANCEL_PENDING;
-
-      /* If the task is waiting at a cancellation point, then notify of the
-       * cancellation thereby waking the task up with an ECANCELED error.
-       *
-       * REVISIT: is locking the scheduler sufficent in SMP mode?
-       */
-
-      if (rtcb->cpcount > 0)
-        {
-          notify_cancellation(rtcb);
-        }
-
-      sched_unlock();
-      return OK;
-    }
-#endif
-
   /* Otherwise, perform the asynchronous cancellation, letting
    * task_terminate() do all of the heavy lifting.
    */
 
-  return task_terminate(pid, false);
+  ret = task_terminate(pid, false);
+  if (ret < 0)
+    {
+      set_errno(-ret);
+      return ERROR;
+    }
+
+  return OK;
 }
diff --git a/sched/task/task_restart.c b/sched/task/task_restart.c
index 21aec95b474..e499a6c5988 100644
--- a/sched/task/task_restart.c
+++ b/sched/task/task_restart.c
@@ -214,7 +214,7 @@ int task_restart(pid_t pid)
   ret = task_activate((FAR struct tcb_s *)tcb);
   if (ret != OK)
     {
-      (void)task_delete(pid);
+      (void)task_terminate(pid, true);
       errcode = -ret;
       goto errout_with_lock;
     }