Fix some missing logic and inconsistencies in child status logic; Fix a bug introduced into sigaction()

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@5560 42af7a65-404d-4744-a932-0658087f49c3

TODO

@@ -7,7 +7,7 @@ standards, things that could be improved, and ideas for enhancements.
 nuttx/
 
  (11)  Task/Scheduler (sched/)
-  (1)  Memory Managment (mm/)
+  (2)  Memory Managment (mm/)
   (3)  Signals (sched/, arch/)
   (2)  pthreads (sched/)
   (2)  C++ Support
@@ -278,6 +278,19 @@ o Memory Managment (mm/)
   Priority:    Medium/Low, a good feature to prevent memory leaks but would
                have negative impact on memory usage and code size.
 
+  Title:       CONTAINER ALLOCATOR
+  Description: There are several places where the logic requires allocation of
+               a tiny structure that just contains pointers to other things or
+               small amounts of data that needs to be bundled together.  There
+               are examples net/net_poll.c and numerous other places.
+
+               I am wondering if it would not be good create a pool of generic
+               containers (say void *[4]).  There re-use these when we need
+               small containers.  The code in sched/task_childstatus.c might
+               be generalized for this purpose.
+  Status:      Open
+  Priority:    Very low (I am not even sure that this is a good idea yet).
+
 o Signals (sched/, arch/)
   ^^^^^^^^^^^^^^^^^^^^^^^
 

net/net_poll.c

@@ -367,8 +367,7 @@ int psock_poll(FAR struct socket *psock, FAR struct pollfd *fds, bool setup)
 
   if (psock->s_type != SOCK_STREAM)
     {
-      ret = -ENOSYS;
+      return -ENOSYS;
-      goto errout;
     }
 #endif
 
@@ -387,7 +386,6 @@ int psock_poll(FAR struct socket *psock, FAR struct pollfd *fds, bool setup)
       ret = net_pollteardown(psock, fds);
     }
 
-errout:
   return ret;
 }
 #endif

sched/Kconfig

@@ -124,6 +124,13 @@ config PREALLOC_CHILDSTATUS
 			sa.sa_flags = SA_NOCLDWAIT;
 			int ret = sigaction(SIGCHLD, &sa, NULL);
 
+config DEBUG_CHILDSTATUS
+	bool "Enable Child Status Debug Output"
+	default n
+	depends on SCHED_CHILD_STATUS && DEBUG
+	---help---
+		Very detailed... I am sure that you do not want this.
+
 config JULIAN_TIME
 	bool "Enables Julian time conversions"
 	default n

sched/os_internal.h

@@ -274,6 +274,7 @@ void weak_function task_initialize(void);
 FAR struct child_status_s *task_allocchild(void);
 void task_freechild(FAR struct child_status_s *status);
 void task_addchild(FAR _TCB *tcb, FAR struct child_status_s *child);
+FAR struct child_status_s *task_exitchild(FAR _TCB *tcb);
 FAR struct child_status_s *task_findchild(FAR _TCB *tcb, pid_t pid);
 FAR struct child_status_s *task_removechild(FAR _TCB *tcb, pid_t pid);
 void task_removechildren(FAR _TCB *tcb);

sched/sched_waitid.c

@@ -53,6 +53,36 @@
  * Private Functions
  *****************************************************************************/
 
+/*****************************************************************************
+ * Name: exitted_child
+ *
+ * Description:
+ *   Handle the case where a child exitted properlay was we (apparently) lost
+ *   the detch of child signal.
+ *
+ *****************************************************************************/
+
+#ifdef CONFIG_SCHED_CHILD_STATUS
+static void exitted_child(FAR _TCB *rtcb, FAR struct child_status_s *child,
+                          FAR siginfo_t *info)
+{
+  /* The child has exited. Return the saved exit status (and some fudged
+   * information.
+   */
+
+  info->si_signo           = SIGCHLD;
+  info->si_code            = CLD_EXITED;
+  info->si_value.sival_ptr = NULL;
+  info->si_pid             = child->ch_pid;
+  info->si_status          = child->ch_status;
+
+  /* Discard the child entry */
+
+  (void)task_removechild(rtcb, child->ch_pid);
+  task_freechild(child);
+}
+#endif
+
 /*****************************************************************************
  * Public Functions
  *****************************************************************************/
@@ -120,9 +150,14 @@
  *
  *****************************************************************************/
 
-int waitid(idtype_t idtype, id_t id, siginfo_t *info, int options)
+int waitid(idtype_t idtype, id_t id, FAR siginfo_t *info, int options)
 {
   FAR _TCB *rtcb = (FAR _TCB *)g_readytorun.head;
+  FAR _TCB *ctcb;
+#ifdef CONFIG_SCHED_CHILD_STATUS
+  FAR struct child_status_s *child;
+  bool retains;
+#endif
   sigset_t sigset;
   int err;
   int ret;
@@ -160,7 +195,11 @@ int waitid(idtype_t idtype, id_t id, siginfo_t *info, int options)
    */
 
 #ifdef CONFIG_SCHED_CHILD_STATUS
-  if (rtcb->children == NULL)
+  /* Does this task retain child status? */
+
+  retains = ((rtcb->flags && TCB_FLAG_NOCLDWAIT) == 0);
+
+  if (rtcb->children == NULL && retains)
     {
       /* There are no children */
 
@@ -169,13 +208,29 @@ int waitid(idtype_t idtype, id_t id, siginfo_t *info, int options)
     }
   else if (idtype == P_PID)
     {
-      if (task_findchild(rtcb, (pid_t)id) == NULL)
+      /* Get the TCB corresponding to this PID and make sure it is our child. */
-        {
-          /* This specific pid is not a child */
 
+      ctcb = sched_gettcb((pid_t)id);
+      if (!ctcb || ctcb->parent != rtcb->pid)
+        {
           err = ECHILD;
           goto errout_with_errno;
         }
+
+      /* Does this task retain child status? */
+
+       if (retains)
+        {
+           /* Check if this specific pid has allocated child status? */
+
+          if (task_findchild(rtcb, (pid_t)id) == NULL)
+            {
+              /* This specific pid is not a child */
+
+              err = ECHILD;
+              goto errout_with_errno;
+            }
+        }
     }
 #else
   if (rtcb->nchildren == 0)
@@ -189,7 +244,7 @@ int waitid(idtype_t idtype, id_t id, siginfo_t *info, int options)
     {
      /* Get the TCB corresponding to this PID and make sure it is our child. */
 
-      FAR _TCB *ctcb = sched_gettcb((pid_t)id);
+      ctcb = sched_gettcb((pid_t)id);
       if (!ctcb || ctcb->parent != rtcb->pid)
         {
           err = ECHILD;
@@ -209,48 +264,61 @@ int waitid(idtype_t idtype, id_t id, siginfo_t *info, int options)
        * instead).
        */
 
-      DEBUGASSERT(rtcb->children);
+      DEBUGASSERT(!retains || rtcb->children);
-      if (rtcb->children == NULL)
+      if (idtype == P_ALL)
         {
-          /* This should not happen.  I am just wasting your FLASH. */
+          /* We are waiting for any child to exit */
 
-          err = ECHILD;
+          if (retains && (child = task_exitchild(rtcb)) != NULL)
-          goto errout_with_errno;
+            {
+              /* A child has exitted.  Apparently we missed the signal.
+               * Return the exit status and break out of the loop.
+               */
+
+              exitted_child(rtcb, child, info);
+              break;
+            }
         }
-      else if (idtype == P_PID)
-        {
-          FAR struct child_status_s *child;
 
-          /* We are waiting for a specific PID.  Get the current status
+      /* We are waiting for a specific PID.  Does this task retain child status? */
-           * of the child task.
+
-           */
+      else if (retains)
+        {
+          /* Yes ... Get the current status of the child task. */
 
           child = task_findchild(rtcb, (pid_t)id);
           DEBUGASSERT(child);
-          if (!child)
+      
-            {
-              /* Yikes!  The child status entry just disappeared! */
-
-              err = ECHILD;
-              goto errout_with_errno;
-            }
-
           /* Did the child exit? */
 
           if ((child->ch_flags & CHILD_FLAG_EXITED) != 0)
             {
-              /* The child has exited. Return the saved exit status */
+              /* The child has exited. Return the exit status and break out
+               * of the loop.
+               */
 
-              info->si_signo           = SIGCHLD;
+              exitted_child(rtcb, child, info);
-              info->si_code            = CLD_EXITED;
+              break;
-              info->si_value.sival_ptr = NULL;
+            }
-              info->si_pid             = (pid_t)id;
+        }
-              info->si_status          = child->ch_status;
+      else
+        {
+          /* We can use kill() with signal number 0 to determine if that
+           * task is still alive.
+           */
 
-              /* Discard the child entry and break out of the loop */
+          ret = kill((pid_t)id, 0);
+          if (ret < 0)
+            {
+              /* It is no longer running.  We know that the child task
+               * was running okay when we started, so we must have lost
+               * the signal.  In this case, we know that the task exit'ed,
+               * but we do not know its exit status.  It would be better
+               * to reported ECHILD than bogus status.
+               */
 
-              (void)task_removechild(rtcb, (pid_t)id);
+              err = ECHILD;
-              task_freechild(child);
+              goto errout_with_errno;
             }
         }
 #else

sched/sched_waitpid.c

@@ -185,7 +185,7 @@
 #ifndef CONFIG_SCHED_HAVE_PARENT
 pid_t waitpid(pid_t pid, int *stat_loc, int options)
 {
-  _TCB *tcb;
+  _TCB *ctcb;
   bool mystat;
   int err;
   int ret;
@@ -208,8 +208,8 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
 
   /* Get the TCB corresponding to this PID */
 
-  tcb = sched_gettcb(pid);
+  ctcb = sched_gettcb(pid);
-  if (!tcb)
+  if (!ctcb)
     {
       err = ECHILD;
       goto errout_with_errno;
@@ -221,15 +221,15 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
    * others?
    */
 
-  if (stat_loc != NULL && tcb->stat_loc == NULL)
+  if (stat_loc != NULL && ctcb->stat_loc == NULL)
     {
-      tcb->stat_loc = stat_loc;
+      ctcb->stat_loc = stat_loc;
-      mystat        = true;
+      mystat         = true;
     }
 
   /* Then wait for the task to exit */
  
-  ret = sem_wait(&tcb->exitsem);
+  ret = sem_wait(&ctcb->exitsem);
   if (ret < 0)
     {
       /* Unlock pre-emption and return the ERROR (sem_wait has already set
@@ -239,7 +239,7 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
 
       if (mystat)
         {
-          tcb->stat_loc = NULL;
+          ctcb->stat_loc = NULL;
         }
 
       goto errout;
@@ -274,8 +274,10 @@ errout:
 pid_t waitpid(pid_t pid, int *stat_loc, int options)
 {
   FAR _TCB *rtcb = (FAR _TCB *)g_readytorun.head;
+  FAR _TCB *ctcb;
 #ifdef CONFIG_SCHED_CHILD_STATUS
   FAR struct child_status_s *child;
+  bool retains;
 #endif
   FAR struct siginfo info;
   sigset_t sigset;
@@ -303,27 +305,43 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
 
   sched_lock();
 
-  /* Verify that this task actually has children and that the the request
+  /* Verify that this task actually has children and that the requested PID
-   * TCB is actually a child of this task.
+   * is actually a child of this task.
    */
 
 #ifdef CONFIG_SCHED_CHILD_STATUS
-  if (rtcb->children == NULL)
+  /* Does this task retain child status? */
-    {
-      /* There are no children */
 
+  retains = ((rtcb->flags && TCB_FLAG_NOCLDWAIT) == 0);
+
+  if (rtcb->children == NULL && retains)
+    {
       err = ECHILD;
       goto errout_with_errno;
     }
   else if (pid != (pid_t)-1)
     {
-      /* This specific pid is not a child */
+      /* Get the TCB corresponding to this PID and make sure it is our child. */
 
-      if (task_findchild(rtcb, pid) == NULL)
+      ctcb = sched_gettcb(pid);
+      if (!ctcb || ctcb->parent != rtcb->pid)
         {
           err = ECHILD;
           goto errout_with_errno;
         }
+
+      /* Does this task retain child status? */
+
+       if (retains)
+        {
+           /* Check if this specific pid has allocated child status? */
+
+           if (task_findchild(rtcb, pid) == NULL)
+            {
+              err = ECHILD;
+              goto errout_with_errno;
+            }
+        }
     }
 #else
   if (rtcb->nchildren == 0)
@@ -337,7 +355,7 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
     {
      /* Get the TCB corresponding to this PID and make sure it is our child. */
 
-      FAR _TCB *ctcb = sched_gettcb(pid);
+      ctcb = sched_gettcb(pid);
       if (!ctcb || ctcb->parent != rtcb->pid)
         {
           err = ECHILD;
@@ -350,6 +368,7 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
 
   for (;;)
     {
+#ifdef CONFIG_SCHED_CHILD_STATUS
       /* Check if the task has already died. Signals are not queued in
        * NuttX.  So a possibility is that the child has died and we
        * missed the death of child signal (we got some other signal
@@ -362,39 +381,33 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
            * chilren.
            */
 
-#ifdef CONFIG_SCHED_CHILD_STATUS
+          DEBUGASSERT(!retains || rtcb->children);
-          DEBUGASSERT(rtcb->children);
+          if (retains && (child = task_exitchild(rtcb)) != NULL)
-          if (rtcb->children == NULL)
-#else
-          if (rtcb->nchildren == 0)
-#endif
             {
-              /* There were one or more children when we started so they
+              /* A child has exitted.  Apparently we missed the signal.
-               * must have exit'ed.  There are just no bread crumbs left
+               * Return the saved exit status.
-               * behind to tell us the PID(s) of the existed children.
-               * Reporting ECHLD is about all we can do in this case.
                */
 
-              err = ECHILD;
+              /* The child has exited. Return the saved exit status */
-              goto errout_with_errno;
+
+              *stat_loc = child->ch_status;
+
+              /* Discard the child entry and break out of the loop */
+
+              (void)task_removechild(rtcb, child->ch_pid);
+              task_freechild(child);
+              break;
             }
         }
-      else
+
+      /* We are waiting for a specific PID. Does this task retain child status? */
+
+      else if (retains)
         {
-#ifdef CONFIG_SCHED_CHILD_STATUS
+          /* Get the current status of the child task. */
-          /* We are waiting for a specific PID.  Get the current status
-           * of the child task.
-           */
 
           child = task_findchild(rtcb, pid);
           DEBUGASSERT(child);
-          if (!child)
-            {
-              /* Yikes!  The child status entry just disappeared! */
-
-              err = ECHILD;
-              goto errout_with_errno;
-            }
 
           /* Did the child exit? */
 
@@ -408,27 +421,48 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options)
 
               (void)task_removechild(rtcb, pid);
               task_freechild(child);
+              break;
             }
-#else
+        }
-          /* We are waiting for a specific PID.  We can use kill() with
+      else
-           * signal number 0 to determine if that task is still alive.
+        {
+          /* We can use kill() with signal number 0 to determine if that
+           * task is still alive.
            */
 
           ret = kill(pid, 0);
           if (ret < 0)
             {
-              /* It is no longer running.  We know that the child task was
+              /* It is no longer running.  We know that the child task
-               * running okay when we started, so we must have lost the
+               * was running okay when we started, so we must have lost
-               * signal.  In this case, we know that the task exit'ed, but
+               * the signal.  In this case, we know that the task exit'ed,
-               * we do not know its exit status.  It would be better to
+               * but we do not know its exit status.  It would be better
-               * reported ECHILD that bogus status.
+               * to reported ECHILD than bogus status.
                */
 
               err = ECHILD;
               goto errout_with_errno;
             }
-#endif
         }
+#else
+      /* Check if the task has already died. Signals are not queued in
+       * NuttX.  So a possibility is that the child has died and we
+       * missed the death of child signal (we got some other signal
+       * instead).
+       */
+
+      if (rtcb->nchildren == 0 ||
+          (pid != (pid_t)-1 && (ret = kill((pid_t)id, 0)) < 0))
+        {
+          /* We know that the child task was running okay we stared,
+           * so we must have lost the signal.  What can we do?
+           * Let's claim we were interrupted by a signal.
+           */
+
+          err = EINTR;
+          goto errout_with_errno;
+        }
+#endif
 
       /* Wait for any death-of-child signal */
 

sched/sig_action.c

@@ -169,7 +169,6 @@ int sigaction(int signo, FAR const struct sigaction *act, FAR struct sigaction *
 {
   FAR _TCB      *rtcb = (FAR _TCB*)g_readytorun.head;
   FAR sigactq_t *sigact;
-  int            ret;
 
   /* Since sigactions can only be installed from the running thread of
    * execution, no special precautions should be necessary.
@@ -251,24 +250,31 @@ int sigaction(int signo, FAR const struct sigaction *act, FAR struct sigaction *
 
   if (act->sa_u._sa_handler == SIG_IGN)
     {
-      /* If there is a old sigaction, remove it from sigactionq */
+      /* Do we still have a sigaction container from the previous setting? */
 
-      sq_rem((FAR sq_entry_t*)sigact, &rtcb->sigactionq);
+      if (sigact)
+        {
+          /* Yes.. Remove it from sigactionq */
 
-      /* And deallocate it */
+          sq_rem((FAR sq_entry_t*)sigact, &rtcb->sigactionq);
 
-      sig_releaseaction(sigact);
+          /* And deallocate it */
+
+          sig_releaseaction(sigact);
+        }
     }
 
   /* A sigaction has been supplied */
 
   else
     {
-      /* Check if a sigaction was found */
+      /* Do we still have a sigaction container from the previous setting?
+       * If so, then re-use for the new signal action.
+       */
 
       if (!sigact)
         {
-          /* No sigaction was found, but one is needed.  Allocate one. */
+          /* No.. Then we need to allocate one for the new action. */
 
           sigact = sig_allocateaction();
 
@@ -294,7 +300,7 @@ int sigaction(int signo, FAR const struct sigaction *act, FAR struct sigaction *
       COPY_SIGACTION(&sigact->act, act);
     }
 
-  return ret;
+  return OK;
 }
 
 /****************************************************************************

sched/task_childstatus.c

@@ -297,6 +297,42 @@ FAR struct child_status_s *task_findchild(FAR _TCB *tcb, pid_t pid)
   return NULL;
 }
 
+/*****************************************************************************
+ * Name: task_exitchild
+ *
+ * Description:
+ *   Search for any child that has exitted.
+ *
+ * Parameters:
+ *   tcb - The TCB of the parent task to containing the child status.
+ *
+ * Return Value:
+ *   On success, a non-NULL pointer to a child status structure for the
+ *   exited child.  NULL is returned if not child has exited.
+ *
+ * Assumptions:
+ *   Called during SIGCHLD processing in a safe context.  No special precautions
+ *   are required here.
+ *
+ *****************************************************************************/
+
+FAR struct child_status_s *task_exitchild(FAR _TCB *tcb)
+{
+  FAR struct child_status_s *child;
+
+  /* Find the status structure with the matching PID  */
+
+  for (child = tcb->children; child; child = child->flink)
+    {
+      if ((child->ch_flags & CHILD_FLAG_EXITED) != 0)
+        {
+          return child;
+        }
+    }
+
+  return NULL;
+}
+
 /*****************************************************************************
  * Name: task_removechild
  *

sched/task_reparent.c

@@ -138,14 +138,32 @@ int task_reparent(pid_t ppid, pid_t chpid)
   child = task_removechild(otcb, chpid);
   if (child)
     {
-      /* Add the child status entry to the new parent TCB */
+      /* Has the new parent supressed child exit status? */
+
+      if ((ptcb->flags && TCB_FLAG_NOCLDWAIT) == 0)
+        {
+          /* No.. Add the child status entry to the new parent TCB */
+
+          task_addchild(ptcb, child);
+        }
+      else
+        {
+          /* Yes.. Discard the child status entry */
+
+          task_freechild(child);
+        }
+
+      /* Either case is a success */
 
-      task_addchild(ptcb, child);
       ret = OK;
     }
   else
     {
-      ret = -ENOENT;
+      /* This would not be an error if the original parent has
+       * suppressed child exit status.
+       */
+
+      ret = ((otcb->flags && TCB_FLAG_NOCLDWAIT) == 0) ? -ENOENT : OK;
     }
 #else
   DEBUGASSERT(otcb->nchildren > 0);

sched/task_setup.c

@@ -150,7 +150,8 @@ static int task_assignpid(FAR _TCB *tcb)
  * Name: task_saveparent
  *
  * Description:
- *   Save the task ID of the parent task in the child task's TCB.
+ *   Save the task ID of the parent task in the child task's TCB and allocate
+ *   a child status structure to catch the child task's exit status.
  *
  * Parameters:
  *   tcb   - The TCB of the new, child task.
@@ -177,11 +178,15 @@ static inline void task_saveparent(FAR _TCB *tcb, uint8_t ttype)
 
   tcb->parent = rtcb->pid;
 
-  /* Exit status only needs to be retained for the case of tasks, however */
-
-  if (ttype == TCB_FLAG_TTYPE_TASK)
-    {
 #ifdef CONFIG_SCHED_CHILD_STATUS
+  /* Exit status only needs to be retained for the case of tasks, however.
+   * Tasks can also suppress retention of their child status by applying
+   * the SA_NOCLDWAIT flag with sigaction()/
+   */
+
+  if (ttype == TCB_FLAG_TTYPE_TASK &&
+      (rtcb->flags && TCB_FLAG_NOCLDWAIT) == 0)
+    {
       FAR struct child_status_s *child;
 
       /* Make sure that there is not already a structure for this PID in the
@@ -212,11 +217,11 @@ static inline void task_saveparent(FAR _TCB *tcb, uint8_t ttype)
 
           task_addchild(rtcb, child);
         }
-#else
-      DEBUGASSERT(rtcb->nchildren < UINT16_MAX);
-      rtcb->nchildren++;
-#endif
     }
+#else
+  DEBUGASSERT(rtcb->nchildren < UINT16_MAX);
+  rtcb->nchildren++;
+#endif
 }
 #else
 #  define task_saveparent(tcb,ttype)
@@ -318,7 +323,9 @@ int task_schedsetup(FAR _TCB *tcb, int priority, start_t start, main_t main,
       tcb->flags         &= ~TCB_FLAG_TTYPE_MASK;
       tcb->flags         |= ttype;
 
-      /* Save the task ID of the parent task in the TCB */
+      /* Save the task ID of the parent task in the TCB and allocate
+       * a child status structure.
+       */
 
       task_saveparent(tcb, ttype);