[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2946: sched: task: Call nxtask_flushstreams() without critical section

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

masayuki2009 commented on a change in pull request #2946:
URL: https://github.com/apache/incubator-nuttx/pull/2946#discussion_r585251195



##########
File path: sched/task/task_exithook.c
##########
@@ -645,7 +645,20 @@ void nxtask_exithook(FAR struct tcb_s *tcb, int status, bool nonblocking)
 
   if (!nonblocking)
     {
+      /* NOTE: nxtask_flushstreams() calls lib_flushall()
+       * which handles a semaphore in userspace.
+       * It should be done without a critical section.
+       */
+
+#ifdef CONFIG_SMP
+      leave_critical_section(flags);
+#endif
+
       nxtask_flushstreams(tcb);

Review comment:
       @xiaoxiang781216 
   
   Do you mean that we should move the code block before nxtask_exitwakeup() like
   
   ```
   diff --git a/sched/task/task_exithook.c b/sched/task/task_exithook.c
   index 8378c0c8fc..0afec6a007 100644
   --- a/sched/task/task_exithook.c
   +++ b/sched/task/task_exithook.c
   @@ -612,6 +612,22 @@ void nxtask_exithook(FAR struct tcb_s *tcb, int status, bool nonblocking)
        }
    #endif
    
   +  /* If this is the last thread in the group, then flush all streams (File
   +   * descriptors will be closed when the TCB is deallocated).
   +   *
   +   * NOTES:
   +   * 1. We cannot flush the buffered I/O if nonblocking is requested.
   +   *    that might cause this logic to block.
   +   * 2. This function will only be called with non-blocking == true
   +   *    only when called through _exit(). _exit() behavior does not
   +   *    require that the streams be flushed
   +   */
   +
   +  if (!nonblocking)
   +    {
   +      nxtask_flushstreams(tcb);
   +    }
   +
      /* If the task was terminated by another task, it may be in an unknown
       * state.  Make some feeble effort to recover the state.
       */
   @@ -632,22 +648,6 @@ void nxtask_exithook(FAR struct tcb_s *tcb, int status, bool nonblocking)
    
      nxtask_exitwakeup(tcb, status);
    
   -  /* If this is the last thread in the group, then flush all streams (File
   -   * descriptors will be closed when the TCB is deallocated).
   -   *
   -   * NOTES:
   -   * 1. We cannot flush the buffered I/O if nonblocking is requested.
   -   *    that might cause this logic to block.
   -   * 2. This function will only be called with non-blocking == true
   -   *    only when called through _exit(). _exit() behavior does not
   -   *    require that the streams be flushed
   -   */
   -
   -  if (!nonblocking)
   -    {
   -      nxtask_flushstreams(tcb);
   -    }
   -
      /* Leave the task group.  Perhaps discarding any un-reaped child
       * status (no zombies here!)
   ```




----------------------------------------------------------------
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