From caf5fdf21750857fa1999997c1fed0c928270141 Mon Sep 17 00:00:00 2001
From: patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3>
Date: Thu, 24 Jan 2013 23:18:32 +0000
Subject: [PATCH] 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 | 15 ++++-
net/net_poll.c | 4 +-
sched/Kconfig | 7 ++
sched/os_internal.h | 1 +
sched/sched_waitid.c | 136 +++++++++++++++++++++++++++++----------
sched/sched_waitpid.c | 132 +++++++++++++++++++++++--------------
sched/sig_action.c | 22 ++++---
sched/task_childstatus.c | 36 +++++++++++
sched/task_reparent.c | 24 ++++++-
sched/task_setup.c | 23 ++++---
10 files changed, 294 insertions(+), 106 deletions(-)
diff --git a/TODO b/TODO
index ed91515ba5..d6bd18d129 100644
--- a/TODO
+++ b/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/)
^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/net/net_poll.c b/net/net_poll.c
index ea6f54a6be..3021ac35ec 100644
--- a/net/net_poll.c
+++ b/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;
- goto errout;
+ return -ENOSYS;
}
#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
diff --git a/sched/Kconfig b/sched/Kconfig
index 7ea3016009..7745c2c25e 100644
--- a/sched/Kconfig
+++ b/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
diff --git a/sched/os_internal.h b/sched/os_internal.h
index 7d5095bad4..dc87cb9a40 100644
--- a/sched/os_internal.h
+++ b/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);
diff --git a/sched/sched_waitid.c b/sched/sched_waitid.c
index 37ee26ce0b..e73bc223c7 100644
--- a/sched/sched_waitid.c
+++ b/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)
- {
- /* This specific pid is not a child */
+ /* Get the TCB corresponding to this PID and make sure it is our 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);
- if (rtcb->children == NULL)
+ DEBUGASSERT(!retains || rtcb->children);
+ if (idtype == P_ALL)
{
- /* This should not happen. I am just wasting your FLASH. */
+ /* We are waiting for any child to exit */
- err = ECHILD;
- goto errout_with_errno;
+ if (retains && (child = task_exitchild(rtcb)) != NULL)
+ {
+ /* 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
- * of the child task.
- */
+ /* We are waiting for a specific PID. Does this task retain child status? */
+
+ 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;
- info->si_code = CLD_EXITED;
- info->si_value.sival_ptr = NULL;
- info->si_pid = (pid_t)id;
- info->si_status = child->ch_status;
+ exitted_child(rtcb, child, info);
+ break;
+ }
+ }
+ 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);
- task_freechild(child);
+ err = ECHILD;
+ goto errout_with_errno;
}
}
#else
diff --git a/sched/sched_waitpid.c b/sched/sched_waitpid.c
index fe3f7167dc..ecdc60a2c2 100644
--- a/sched/sched_waitpid.c
+++ b/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);
- if (!tcb)
+ ctcb = sched_gettcb(pid);
+ 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;
- mystat = true;
+ ctcb->stat_loc = stat_loc;
+ 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
- * TCB is actually a child of this task.
+ /* Verify that this task actually has children and that the requested PID
+ * is actually a child of this task.
*/
#ifdef CONFIG_SCHED_CHILD_STATUS
- if (rtcb->children == NULL)
- {
- /* There are no children */
+ /* Does this task retain child status? */
+ 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(rtcb->children);
- if (rtcb->children == NULL)
-#else
- if (rtcb->nchildren == 0)
-#endif
+ DEBUGASSERT(!retains || rtcb->children);
+ if (retains && (child = task_exitchild(rtcb)) != NULL)
{
- /* There were one or more children when we started so they
- * must have exit'ed. There are just no bread crumbs left
- * behind to tell us the PID(s) of the existed children.
- * Reporting ECHLD is about all we can do in this case.
+ /* A child has exitted. Apparently we missed the signal.
+ * Return the saved exit status.
*/
- err = ECHILD;
- goto errout_with_errno;
+ /* The child has exited. Return the saved exit status */
+
+ *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
- /* We are waiting for a specific PID. Get the current status
- * of the child task.
- */
+ /* 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
- * signal number 0 to determine if that task is still alive.
+ }
+ else
+ {
+ /* 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
- * 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 that bogus status.
+ /* 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.
*/
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 */
diff --git a/sched/sig_action.c b/sched/sig_action.c
index 7086679939..7d84b62918 100644
--- a/sched/sig_action.c
+++ b/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 */
+
+ sq_rem((FAR sq_entry_t*)sigact, &rtcb->sigactionq);
- /* And deallocate it */
+ /* And deallocate it */
- sig_releaseaction(sigact);
+ 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;
}
/****************************************************************************
diff --git a/sched/task_childstatus.c b/sched/task_childstatus.c
index 0f6d36c296..09aa48135b 100644
--- a/sched/task_childstatus.c
+++ b/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
*
diff --git a/sched/task_reparent.c b/sched/task_reparent.c
index 28d371bf10..3a7ece37d0 100644
--- a/sched/task_reparent.c
+++ b/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);
diff --git a/sched/task_setup.c b/sched/task_setup.c
index 8f54b2e938..66c948cfd3 100644
--- a/sched/task_setup.c
+++ b/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 */
+#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)
+ if (ttype == TCB_FLAG_TTYPE_TASK &&
+ (rtcb->flags && TCB_FLAG_NOCLDWAIT) == 0)
{
-#ifdef CONFIG_SCHED_CHILD_STATUS
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++;
+ 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);
--
GitLab