[nuttx] branch master updated: task/task_cancelpt: Kill the child if it is not in a cancel point

[xi...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 2cdba4a0a2 task/task_cancelpt: Kill the child if it is not in a cancel point
2cdba4a0a2 is described below

commit 2cdba4a0a280d03d93a598da28407cce5c80f3d9
Author: Ville Juven <vi...@unikie.com>
AuthorDate: Thu Feb 16 10:13:27 2023 +0200

    task/task_cancelpt: Kill the child if it is not in a cancel point
    
    Do not allow a deferred cancellation if the group is exiting, it is too
    dangerous to allow the threads to execute any user space code after the
    exit has started.
    
    If the cancelled thread is not inside a cancellation point, just kill it
    immediately via asynchronous cancellation. This will create far less
    problems than allowing it to continue running user code.
---
 include/nuttx/sched.h            |  1 +
 sched/group/group_killchildren.c |  8 ++++++++
 sched/task/task_cancelpt.c       | 21 +++++++++++++++------
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h
index 703ea64f5c..8eae2680bd 100644
--- a/include/nuttx/sched.h
+++ b/include/nuttx/sched.h
@@ -110,6 +110,7 @@
 #define GROUP_FLAG_NOCLDWAIT       (1 << 0)                      /* Bit 0: Do not retain child exit status */
 #define GROUP_FLAG_PRIVILEGED      (1 << 1)                      /* Bit 1: Group is privileged */
 #define GROUP_FLAG_DELETED         (1 << 2)                      /* Bit 2: Group has been deleted but not yet freed */
+#define GROUP_FLAG_EXITING         (1 << 3)                      /* Bit 3: Group exit is in progress */
                                                                  /* Bits 3-7: Available */
 
 /* Values for struct child_status_s ch_flags */
diff --git a/sched/group/group_killchildren.c b/sched/group/group_killchildren.c
index 4f35c6a38b..fa5a98d65c 100644
--- a/sched/group/group_killchildren.c
+++ b/sched/group/group_killchildren.c
@@ -132,6 +132,8 @@ int group_kill_children(FAR struct tcb_s *tcb)
 {
   int ret;
 
+  DEBUGASSERT(tcb->group);
+
 #ifdef CONFIG_SMP
   /* NOTE: sched_lock() is not enough for SMP
    * because tcb->group will be accessed from the child tasks
@@ -145,8 +147,14 @@ int group_kill_children(FAR struct tcb_s *tcb)
 
   sched_lock();
 #endif
+
+  /* Tell the children that this group has started exiting */
+
+  tcb->group->tg_flags |= GROUP_FLAG_EXITING;
+
   ret = group_foreachchild(tcb->group, group_kill_children_handler,
                            (FAR void *)((uintptr_t)tcb->pid));
+
 #ifdef CONFIG_SMP
   leave_critical_section(flags);
 #else
diff --git a/sched/task/task_cancelpt.c b/sched/task/task_cancelpt.c
index 0c8609f474..2a1ab912f8 100644
--- a/sched/task/task_cancelpt.c
+++ b/sched/task/task_cancelpt.c
@@ -323,6 +323,7 @@ bool check_cancellation_point(void)
 bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 {
   irqstate_t flags;
+  bool ret = false;
 
   /* We need perform the following operations from within a critical section
    * because it can compete with interrupt level activity.
@@ -362,9 +363,11 @@ bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 
   if ((tcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
     {
-      /* Then we cannot cancel the task asynchronously.
-       * Mark the cancellation as pending.
-       */
+      /* Then we cannot cancel the task asynchronously. */
+
+      ret = true;
+
+      /* Mark the cancellation as pending. */
 
       tcb->flags |= TCB_FLAG_CANCEL_PENDING;
 
@@ -405,11 +408,17 @@ bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 #endif
         }
 
-      leave_critical_section(flags);
-      return true;
+#ifdef HAVE_GROUP_MEMBERS
+      else if (tcb->group && (tcb->group->tg_flags & GROUP_FLAG_EXITING))
+        {
+          /* Exit in progress, do asynchronous cancel instead */
+
+          ret = false;
+        }
+#endif
     }
 #endif
 
   leave_critical_section(flags);
-  return false;
+  return ret;
 }