This is an automated email from the ASF dual-hosted git repository.
acassis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git
The following commit(s) were added to refs/heads/master by this push:
new e0174bf71 nshlib: Reap the background commands that exited.
e0174bf71 is described below
commit e0174bf7148af5f19d8e7868f0c5c207aa2dc874
Author: Marco Casaroli <[email protected]>
AuthorDate: Mon Oct 19 19:12:58 2026 +0200
nshlib: Reap the background commands that exited.
With CONFIG_SCHED_CHILD_STATUS, the kernel keeps the exit status of
each child until the parent waits for it. NSH waits only for a
foreground command. A background command ("cmd &") and each pipeline
stage before the last one are also children of NSH, but NSH never waits
for them. Each of them keeps a status entry in the kernel heap until
NSH exits, and the console NSH does not exit.
nsh_builtin() tried to prevent this for builtin applications. It set
SA_NOCLDWAIT for SIGCHLD before it started a background command, and
restored the old action after. This does not work:
- SA_NOCLDWAIT applies to the whole process. sigaction() with
SA_NOCLDWAIT also discards the status of all children that NSH did
not reap yet, and NuttX does not clear the flag when the old action
comes back. After the first background builtin, NSH keeps no child
status at all, also not for its foreground commands.
- The code was only compiled with CONFIG_ENABLE_ALL_SIGNALS.
- nsh_fileapp() had no such code, so every background program of a
kernel build kept its status.
Remove that code. Instead, nsh_parse_command() reaps all children that
exited, with waitpid(-1, WNOHANG), before it runs a command. NSH runs
one command at a time and waits for a foreground command before it
starts the next one. Thus a child that exited at this point is one
that no other code waits for. The "wait" command still gets the status
of a background command that runs, because it calls waitpid() for that
pid.
Two other solutions were not used. A SIGCHLD handler that calls
waitpid(-1) needs CONFIG_ENABLE_ALL_SIGNALS, and it can take the
status of a foreground command before NSH waits for it. A list of the
background pids needs memory for each job, and it does not reap a
foreground command when its waitpid() fails, for example with EINTR.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Marco Casaroli <[email protected]>
---
nshlib/nsh_builtin.c | 42 ------------------------------------------
nshlib/nsh_parse.c | 12 ++++++++++++
2 files changed, 12 insertions(+), 42 deletions(-)
diff --git a/nshlib/nsh_builtin.c b/nshlib/nsh_builtin.c
index 6d1203057..cb0ac3af3 100644
--- a/nshlib/nsh_builtin.c
+++ b/nshlib/nsh_builtin.c
@@ -34,7 +34,6 @@
#include <stdbool.h>
#include <errno.h>
#include <sched.h>
-#include <signal.h>
#include <string.h>
#include <nuttx/lib/builtin.h>
@@ -74,14 +73,6 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char
*cmd,
FAR char **argv,
FAR const struct nsh_param_s *param)
{
-#if !defined(CONFIG_NSH_DISABLEBG) && defined(CONFIG_SCHED_CHILD_STATUS) && \
- defined(CONFIG_ENABLE_ALL_SIGNALS)
- struct sigaction act;
- struct sigaction old;
-#endif /* !CONFIG_NSH_DISABLEBG && CONFIG_SCHED_CHILD_STATUS &&
- * CONFIG_ENABLE_ALL_SIGNALS
- */
-
int ret = OK;
/* Lock the scheduler in an attempt to prevent the application from
@@ -90,23 +81,6 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char
*cmd,
sched_lock();
-#if !defined(CONFIG_NSH_DISABLEBG) && \
- defined(CONFIG_SCHED_CHILD_STATUS) && defined(CONFIG_ENABLE_ALL_SIGNALS)
- /* Ignore the child status if run the application on background. */
-
- if (vtbl->np.np_bg == true)
- {
- act.sa_handler = SIG_DFL;
- act.sa_flags = SA_NOCLDWAIT;
- sigemptyset(&act.sa_mask);
-
- sigaction(SIGCHLD, &act, &old);
- }
-
-#endif /* !CONFIG_NSH_DISABLEBG && !CONFIG_SCHED_CHILD_STATUS &&
- * CONFIG_ENABLE_ALL_SIGNALS
- */
-
/* Try to find and execute the command within the list of builtin
* applications.
*/
@@ -230,22 +204,6 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const
char *cmd,
#if !defined(CONFIG_SCHED_WAITPID) || !defined(CONFIG_NSH_DISABLEBG)
{
-#if !defined(CONFIG_NSH_DISABLEBG) && defined(CONFIG_SCHED_CHILD_STATUS) && \
- defined(CONFIG_ENABLE_ALL_SIGNALS)
-
- /* Restore the old actions */
-
-# ifndef CONFIG_SCHED_WAITPID
- if (vtbl->np.np_bg == true)
-# endif
- {
- sigaction(SIGCHLD, &old, NULL);
- }
-
-# endif /* !CONFIG_NSH_DISABLEBG && CONFIG_SCHED_CHILD_STATUS && \
- * CONFIG_ENABLE_ALL_SIGNALS
- */
-
struct sched_param sched;
sched_getparam(ret, &sched);
diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c
index 51811c667..7bbc52ff5 100644
--- a/nshlib/nsh_parse.c
+++ b/nshlib/nsh_parse.c
@@ -34,6 +34,7 @@
#include <nuttx/debug.h>
#include <sched.h>
#include <unistd.h>
+#include <sys/wait.h>
#ifdef CONFIG_NSH_CMDPARMS
# include <sys/stat.h>
@@ -2524,6 +2525,17 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s
*vtbl, FAR char *cmdline)
sched_note_beginex(NOTE_TAG_APP, tracebuf);
#endif
+#if defined(CONFIG_SCHED_CHILD_STATUS) && defined(CONFIG_SCHED_WAITPID)
+ /* Reap the children that exited and that nobody waits for: background
+ * commands and pipeline stages. The kernel keeps their exit status
+ * until the parent waits for it.
+ */
+
+ while (waitpid(-1, NULL, WNOHANG) > 0)
+ {
+ }
+#endif
+
/* Initialize parser state */
memset(argv, 0, MAX_ARGV_ENTRIES*sizeof(FAR char *));
