You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ag...@apache.org on 2020/06/15 20:21:29 UTC

[incubator-nuttx] 02/02: sched: Consolidate the cancellation notification logic

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

aguettouche pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 4d634b9006760555c48ce70375190dc57fb594e2
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Sun Jun 7 00:41:47 2020 +0800

    sched: Consolidate the cancellation notification logic
    
    to avoid the code duplication
    
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
    Change-Id: Ie2ba55c382eb3eb7c8d9f04bba1b9e294aaf6196
---
 sched/pthread/pthread_cancel.c | 50 +--------------------
 sched/signal/sig_default.c     | 53 +----------------------
 sched/task/Make.defs           |  6 +--
 sched/task/task.h              |  4 +-
 sched/task/task_cancelpt.c     | 98 +++++++++++++++++++++++++++++-------------
 sched/task/task_delete.c       | 50 +--------------------
 6 files changed, 77 insertions(+), 184 deletions(-)

diff --git a/sched/pthread/pthread_cancel.c b/sched/pthread/pthread_cancel.c
index 02b8087..dcc58f6 100644
--- a/sched/pthread/pthread_cancel.c
+++ b/sched/pthread/pthread_cancel.c
@@ -67,61 +67,15 @@ int pthread_cancel(pthread_t thread)
   DEBUGASSERT((tcb->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).
-   */
-
-  sched_lock();
-  if ((tcb->flags & TCB_FLAG_NONCANCELABLE) != 0)
-    {
-      /* Then we cannot cancel the thread now.  Here is how this is
-       * supposed to work:
-       *
-       * "When cancellability is disabled, all cancels are held pending
-       *  in the target thread until the thread changes the cancellability.
-       *  When cancellability is deferred, all cancels are held pending in
-       *  the target thread until the thread changes the cancellability,
-       *  calls a function which is a cancellation point or calls
-       *  pthread_testcancel(), thus creating a cancellation point. When
-       *  cancellability is asynchronous, all cancels are acted upon
-       *  immediately, interrupting the thread with its processing."
-       */
-
-      tcb->flags |= TCB_FLAG_CANCEL_PENDING;
-      sched_unlock();
-      return OK;
-    }
-
-#ifdef CONFIG_CANCELLATION_POINTS
-  /* Check if this thread supports deferred cancellation */
+  /* Notify the target if the non-cancelable or deferred cancellation set */
 
-  if ((tcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
+  if (nxnotify_cancellation(tcb))
     {
-      /* Then we cannot cancel the thread asynchronously.  Mark the
-       * cancellation as pending.
-       */
-
-      tcb->flags |= TCB_FLAG_CANCEL_PENDING;
-
-      /* If the thread is waiting at a cancellation point, then notify of the
-       * cancellation thereby waking the task up with an ECANCELED error.
-       */
-
-      if (tcb->cpcount > 0)
-        {
-          nxnotify_cancellation(tcb);
-        }
-
-      sched_unlock();
       return OK;
     }
-#endif
 
   /* Otherwise, perform the asyncrhonous cancellation */
 
-  sched_unlock();
-
   /* Check to see if the ID refers to ourselves.. this would be the
    * same as pthread_exit(PTHREAD_CANCELED).
    */
diff --git a/sched/signal/sig_default.c b/sched/signal/sig_default.c
index 28bad31..70a2238 100644
--- a/sched/signal/sig_default.c
+++ b/sched/signal/sig_default.c
@@ -207,61 +207,12 @@ static void nxsig_abnormal_termination(int signo)
 {
   FAR struct tcb_s *rtcb = (FAR struct tcb_s *)this_task();
 
-  /* 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 ((rtcb->flags & TCB_FLAG_NONCANCELABLE) != 0)
-    {
-      /* Then we cannot cancel the thread now.  Here is how this is
-       * supposed to work:
-       *
-       * "When cancellability is disabled, all cancels are held pending
-       *  in the target thread until the thread changes the cancellability.
-       *  When cancellability is deferred, all cancels are held pending in
-       *  the target thread until the thread changes the cancellability,
-       *  calls a function which is a cancellation point or calls
-       *  pthread_testcancel(), thus creating a cancellation point.  When
-       *  cancellability is asynchronous, all cancels are acted upon
-       *  immediately, interrupting the thread with its processing."
-       *
-       * REVISIT:  Does this rule apply to equally to both SIGKILL and
-       * SIGINT?
-       */
-
-      rtcb->flags |= TCB_FLAG_CANCEL_PENDING;
-      sched_unlock();
-      return;
-    }
-
-#ifdef CONFIG_CANCELLATION_POINTS
-  /* Check if this task supports deferred cancellation */
+  /* Notify the target if the non-cancelable or deferred cancellation set */
 
-  if ((rtcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
+  if (nxnotify_cancellation(rtcb))
     {
-      /* Then we cannot cancel the task asynchronously.
-       * 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.
-       */
-
-      if (rtcb->cpcount > 0)
-        {
-          nxnotify_cancellation(rtcb);
-        }
-
-      sched_unlock();
       return;
     }
-#endif
-
-  sched_unlock();
 
   /* Careful:  In the multi-threaded task, the signal may be handled on a
    * child pthread.
diff --git a/sched/task/Make.defs b/sched/task/Make.defs
index a5c2295..1fa00c7 100644
--- a/sched/task/Make.defs
+++ b/sched/task/Make.defs
@@ -37,7 +37,7 @@ CSRCS += task_create.c task_init.c task_setup.c task_activate.c
 CSRCS += task_start.c task_delete.c task_exit.c task_exithook.c
 CSRCS += task_getgroup.c task_getpid.c task_prctl.c task_recover.c
 CSRCS += task_restart.c task_spawnparms.c task_setcancelstate.c
-CSRCS += task_terminate.c exit.c
+CSRCS += task_cancelpt.c task_terminate.c exit.c
 
 ifeq ($(CONFIG_ARCH_HAVE_VFORK),y)
 ifeq ($(CONFIG_SCHED_WAITPID),y)
@@ -71,10 +71,6 @@ ifeq ($(CONFIG_SCHED_ONEXIT),y)
 CSRCS += task_onexit.c
 endif
 
-ifeq ($(CONFIG_CANCELLATION_POINTS),y)
-CSRCS += task_cancelpt.c
-endif
-
 # Include task build support
 
 DEPPATH += --dep-path task
diff --git a/sched/task/task.h b/sched/task/task.h
index 80279a7..3de2d42 100644
--- a/sched/task/task.h
+++ b/sched/task/task.h
@@ -71,8 +71,6 @@ void nxtask_recover(FAR struct tcb_s *tcb);
 
 /* Cancellation points */
 
-#ifdef CONFIG_CANCELLATION_POINTS
-void nxnotify_cancellation(FAR struct tcb_s *tcb);
-#endif
+bool nxnotify_cancellation(FAR struct tcb_s *tcb);
 
 #endif /* __SCHED_TASK_TASK_H */
diff --git a/sched/task/task_cancelpt.c b/sched/task/task_cancelpt.c
index 988e9bf..cad59df 100644
--- a/sched/task/task_cancelpt.c
+++ b/sched/task/task_cancelpt.c
@@ -316,6 +316,8 @@ bool check_cancellation_point(void)
   return ret;
 }
 
+#endif /* CONFIG_CANCELLATION_POINTS */
+
 /****************************************************************************
  * Name: nxnotify_cancellation
  *
@@ -327,9 +329,12 @@ bool check_cancellation_point(void)
  *   leave_cancellation_point() would then follow, causing the thread to
  *   exit.
  *
+ * Returned Value:
+ *   Indicate whether the notification delivery to the target
+ *
  ****************************************************************************/
 
-void nxnotify_cancellation(FAR struct tcb_s *tcb)
+bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 {
   irqstate_t flags;
 
@@ -339,51 +344,86 @@ void nxnotify_cancellation(FAR struct tcb_s *tcb)
 
   flags = enter_critical_section();
 
-  /* Make sure that the cancellation pending indication is set. */
-
-  tcb->flags |= TCB_FLAG_CANCEL_PENDING;
-
   /* We only notify the cancellation if (1) the thread has not disabled
    * cancellation, (2) the thread uses the deferred cancellation mode,
    * (3) the thread is waiting within a cancellation point.
    */
 
-  if (((tcb->flags & TCB_FLAG_NONCANCELABLE) == 0 &&
-       (tcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0) ||
-      tcb->cpcount > 0)
+  /* Check to see if this task has the non-cancelable bit set. */
+
+  if ((tcb->flags & TCB_FLAG_NONCANCELABLE) != 0)
     {
-      /* If the thread is blocked waiting for a semaphore, then the thread
-       * must be unblocked to handle the cancellation.
+      /* Then we cannot cancel the thread now.  Here is how this is
+       * supposed to work:
+       *
+       * "When cancellability is disabled, all cancels are held pending
+       *  in the target thread until the thread changes the cancellability.
+       *  When cancellability is deferred, all cancels are held pending in
+       *  the target thread until the thread changes the cancellability,
+       *  calls a function which is a cancellation point or calls
+       *  pthread_testcancel(), thus creating a cancellation point.  When
+       *  cancellability is asynchronous, all cancels are acted upon
+       *  immediately, interrupting the thread with its processing."
        */
 
-      if (tcb->task_state == TSTATE_WAIT_SEM)
-        {
-          nxsem_wait_irq(tcb, ECANCELED);
-        }
+      tcb->flags |= TCB_FLAG_CANCEL_PENDING;
+      leave_critical_section(flags);
+      return true;
+    }
+
+#ifdef CONFIG_CANCELLATION_POINTS
+  /* Check if this task supports deferred cancellation */
 
-      /* If the thread is blocked waiting on a signal, then the
-       * thread must be unblocked to handle the cancellation.
+  if ((tcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
+    {
+      /* Then we cannot cancel the task asynchronously.
+       * Mark the cancellation as pending.
        */
 
-      else if (tcb->task_state == TSTATE_WAIT_SIG)
-        {
-          nxsig_wait_irq(tcb, ECANCELED);
-        }
+      tcb->flags |= TCB_FLAG_CANCEL_PENDING;
 
-#ifndef CONFIG_DISABLE_MQUEUE
-      /* If the thread is blocked waiting on a message queue, then the
-       * thread must be unblocked to handle the cancellation.
+      /* If the task is waiting at a cancellation point, then notify of the
+       * cancellation thereby waking the task up with an ECANCELED error.
        */
 
-      else if (tcb->task_state == TSTATE_WAIT_MQNOTEMPTY ||
-               tcb->task_state == TSTATE_WAIT_MQNOTFULL)
+      if (tcb->cpcount > 0)
         {
-          nxmq_wait_irq(tcb, ECANCELED);
-        }
+          /* If the thread is blocked waiting for a semaphore, then the
+           * thread must be unblocked to handle the cancellation.
+           */
+
+          if (tcb->task_state == TSTATE_WAIT_SEM)
+            {
+              nxsem_wait_irq(tcb, ECANCELED);
+            }
+
+          /* If the thread is blocked waiting on a signal, then the
+           * thread must be unblocked to handle the cancellation.
+           */
+
+          else if (tcb->task_state == TSTATE_WAIT_SIG)
+            {
+              nxsig_wait_irq(tcb, ECANCELED);
+            }
+
+#ifndef CONFIG_DISABLE_MQUEUE
+          /* If the thread is blocked waiting on a message queue, then
+           * the thread must be unblocked to handle the cancellation.
+           */
+
+          else if (tcb->task_state == TSTATE_WAIT_MQNOTEMPTY ||
+                  tcb->task_state == TSTATE_WAIT_MQNOTFULL)
+            {
+              nxmq_wait_irq(tcb, ECANCELED);
+            }
 #endif
+        }
+
+      leave_critical_section(flags);
+      return true;
     }
+#endif
 
   leave_critical_section(flags);
+  return false;
 }
-
-#endif /* CONFIG_CANCELLATION_POINTS */
diff --git a/sched/task/task_delete.c b/sched/task/task_delete.c
index 8a6fe5b..eef94c1 100644
--- a/sched/task/task_delete.c
+++ b/sched/task/task_delete.c
@@ -143,59 +143,13 @@ int task_delete(pid_t pid)
       exit(EXIT_SUCCESS);
     }
 
-  /* 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).
-   */
+  /* Notify the target if the non-cancelable or deferred cancellation set */
 
-  sched_lock();
-  if ((dtcb->flags & TCB_FLAG_NONCANCELABLE) != 0)
+  if (nxnotify_cancellation(dtcb))
     {
-      /* Then we cannot cancel the thread now.  Here is how this is
-       * supposed to work:
-       *
-       * "When cancellability is disabled, all cancels are held pending
-       *  in the target thread until the thread changes the cancellability.
-       *  When cancellability is deferred, all cancels are held pending in
-       *  the target thread until the thread changes the cancellability,
-       *  calls a function which is a cancellation point or calls
-       *  pthread_testcancel(), thus creating a cancellation point. When
-       *  cancellability 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.
-       */
-
-      if (dtcb->cpcount > 0)
-        {
-          nxnotify_cancellation(dtcb);
-        }
-
-      sched_unlock();
-      return OK;
-    }
-#endif
-
-  sched_unlock();
-
   /* Otherwise, perform the asynchronous cancellation, letting
    * nxtask_terminate() do all of the heavy lifting.
    */