[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1187: sched: Unify the main thread and pthread behaivour

[GitBox <gi...@apache.org>]

yamt commented on a change in pull request #1187:
URL: https://github.com/apache/incubator-nuttx/pull/1187#discussion_r437857953



##########
File path: sched/task/task.h
##########
@@ -48,6 +48,13 @@
 
 #include <nuttx/sched.h>
 
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define EXIT_NORMAL                0
+#define EXIT_BYPASS                1

Review comment:
       can you document what this bypasses?

##########
File path: sched/task/task_cancelpt.c
##########
@@ -339,51 +344,89 @@ 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."
+       *
+       * REVISIT:  Does this rule apply to equally to both SIGKILL and
+       * SIGINT?

Review comment:
       i don't understand this REVISIT comment. how signals are related here?

##########
File path: sched/pthread/pthread_cancel.c
##########
@@ -85,6 +76,15 @@ int pthread_cancel(pthread_t thread)
       pthread_exit(PTHREAD_CANCELED);
     }
 

Review comment:
       does this mean to make pthread_cancel a cancellation point?

##########
File path: sched/task/task_delete.c
##########
@@ -143,6 +144,15 @@ int task_delete(pid_t pid)
       goto errout;
     }
 
+#ifdef HAVE_GROUP_MEMBERS
+  /* Kill all of the children of the task.  This will not kill the currently
+   * running task/pthread (this_task).  It will kill the main thread of the
+   * task group if this_task is a pthread.
+   */
+
+  group_kill_children(rtcb);

Review comment:
       why is this necessary?
   why is this rtcb, not dtcb?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org