On 2025-06-01 13:35:07 +0800, Nadav Tasher wrote:
Since we're setting the signal handler to SIG_IGN, both SA_RESTART and
SA_NOCLDWAIT are meaningless - they only matter when setting the handler
to SIG_DFL or to a custom signal handler.
This could be simplified to signal(SIGCHLD, SIG_IGN).
My personal opinion is that SIGCHLD should be used until you actually
implement error handling, as it is more efficient.
When you do implement error handling, there would be no other option
then to use waitpid, and that would be acceptable.
If you do decide to use SIGCHLD, I'd appreciate it if you added a
comment next to the existing waitpid, just to make sure nobody tries
to pass wstatus to it.
Sure ! Here's what you've suggested. Thx for your time.
From 5b31a17e0feb23b8fa89df521973f422604e84d0 Mon Sep 17 00:00:00 2001
From: Valentin Lab <[email protected]>
Date: Mon, 2 Jun 2025 16:56:26 +0800
Subject: [PATCH] crond: reap orphaned grandchildren to prevent zombie
buildup
If a cron job launches a background task, e.g. `sh -c "sleep 5 &"`,
the shell exits immediately and the `sleep` process is re-parented to
PID 1. When BusyBox `crond` itself happens to be PID 1 (common in a
minimal container), those orphans become direct children of `crond`.
Because `crond` only calls waitpid() for the PIDs it explicitly tracks,
these processes remain forever in Z state and the container slowly
fills with zombies.
Setting the SIGCHLD disposition to SIG_IGN will ensure the kernel
reaps automatically all child processes.
Size impact: +0 bytes on x86-64 (gcc 13.3.0 -Os, static)
Signed-off-by: Valentin Lab <[email protected]>
---
miscutils/crond.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/miscutils/crond.c b/miscutils/crond.c
index b3762d327..933816456 100644
--- a/miscutils/crond.c
+++ b/miscutils/crond.c
@@ -989,6 +989,15 @@ static int check_completions(void)
if (line->cl_pid <= 0)
continue;
+ /* SIGCHLD disposition is set to SIG_IGN in
+ * `crond_main()`, so the kernel reaps children
+ * automatically and discards their exit status. If we
+ * ever need the exit status, drop SIG_IGN and add a
+ * generic reaper (e.g. `while (waitpid(-1, NULL,
WNOHANG)
+ * > 0);`) at the end of this function to prevent
zombies
+ * when crond happens to be PID 1 (all orphans are
+ * re-parented to it).
+ */
r = waitpid(line->cl_pid, NULL, WNOHANG);
if (r < 0 || r == line->cl_pid) {
process_finished_job(file->cf_username, line);
@@ -1025,6 +1034,9 @@ int crond_main(int argc UNUSED_PARAM, char **argv)
unsigned sleep_time;
unsigned opts;
+ /* Reap all children process automatically. */
+ signal(SIGCHLD, SIG_IGN);
+
INIT_G();
opts = getopt32(argv, "^"
--
2.43.0
From 5b31a17e0feb23b8fa89df521973f422604e84d0 Mon Sep 17 00:00:00 2001
From: Valentin Lab <[email protected]>
Date: Mon, 2 Jun 2025 16:56:26 +0800
Subject: [PATCH] crond: reap orphaned grandchildren to prevent zombie buildup
If a cron job launches a background task, e.g. `sh -c "sleep 5 &"`,
the shell exits immediately and the `sleep` process is re-parented to
PID 1. When BusyBox `crond` itself happens to be PID 1 (common in a
minimal container), those orphans become direct children of `crond`.
Because `crond` only calls waitpid() for the PIDs it explicitly tracks,
these processes remain forever in Z state and the container slowly
fills with zombies.
Setting the SIGCHLD disposition to SIG_IGN will ensure the kernel
reaps automatically all child processes.
Size impact: +0 bytes on x86-64 (gcc 13.3.0 -Os, static)
Signed-off-by: Valentin Lab <[email protected]>
---
miscutils/crond.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/miscutils/crond.c b/miscutils/crond.c
index b3762d327..933816456 100644
--- a/miscutils/crond.c
+++ b/miscutils/crond.c
@@ -989,6 +989,15 @@ static int check_completions(void)
if (line->cl_pid <= 0)
continue;
+ /* SIGCHLD disposition is set to SIG_IGN in
+ * `crond_main()`, so the kernel reaps children
+ * automatically and discards their exit status. If we
+ * ever need the exit status, drop SIG_IGN and add a
+ * generic reaper (e.g. `while (waitpid(-1, NULL, WNOHANG)
+ * > 0);`) at the end of this function to prevent zombies
+ * when crond happens to be PID 1 (all orphans are
+ * re-parented to it).
+ */
r = waitpid(line->cl_pid, NULL, WNOHANG);
if (r < 0 || r == line->cl_pid) {
process_finished_job(file->cf_username, line);
@@ -1025,6 +1034,9 @@ int crond_main(int argc UNUSED_PARAM, char **argv)
unsigned sleep_time;
unsigned opts;
+ /* Reap all children process automatically. */
+ signal(SIGCHLD, SIG_IGN);
+
INIT_G();
opts = getopt32(argv, "^"
--
2.43.0
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox