On Sun, Jun 01, 2025 at 08:57:39AM +0800, Valentin Lab wrote:
> On 2025-05-31 21:57:54 +0800, Nadav Tasher wrote:
> > Great catch!
> >
> > However, since the process status is never used (NULL provided to waitpid),
> > why not just set the signal handler for SIGCHLD to SIG_IGN?
> >
> > This would significantly reduce the number of waitpid invocations.
>
> Setting SIGCHLD to SIG_IGN would indeed solve the leak and costs zero
> runtime overhead.
>
> I stuck to a waitpid sweep because it’s fully portable and keeps the option
> of inspecting the exit status in the future (an option I was considering to
> implement to report/use errorlevel), but I’m happy to switch if you prefer
> the SIG_IGN approach.
>
> Let me know and I’ll resend the patch using:
>
> struct sigaction sa = { .sa_handler = SIG_IGN, .sa_flags =
> SA_RESTART|SA_NOCLDWAIT };
> sigaction(SIGCHLD, &sa, NULL);
>
> (SA_NOCLDWAIT for maximum portability.)
>
>
> -
> Valentin Lab
>
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.
Nadav
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
