Re: Calling system(3) when SIGCHLD is SIG_IGN'd

[Rhialto]

On Fri 12 Jan 2024 at 20:28:12 -0600, Matthew D. Fuller wrote:
> On Sat, Aug 12, 2023 at 03:22:56PM +0200 I heard the voice of
> Rhialto, and lo! it spake thus:
> > There seems to be only a single fork() in ctwm anyway, to call m4 to
> > parse the config file. The SIGCHLD changing could be limited to that
> > area, then set back to default.
> > 
> > Or, alternatively, a proper signal handler for SIGCHLD could be set up.
> 
> Presumably, we'd need to do the latter to handle the "inheriting
> unexpected children" case properly anyway.  I wonder what happened
> before the SIG_IGN change.  I guess we just accumulated zombies?

I expect so - I didn't really check.

> >From the looks of that PR, it doesn't seem like any kernel-side
> changes have fallen out of it, and cvsweb doesn't show any recent
> changes to system(3), so I presume this is still needed?

I didn't see any followup either. Their variant of the change goes like
this (but my version fits better with the current handler naming scheme
I think):

Index: signals.c
===================================================================
RCS file: /cvsroot/xsrc/external/mit/ctwm/dist/signals.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- signals.c   5 Jul 2023 07:36:07 -0000       1.1
+++ signals.c   20 Oct 2023 10:18:55 -0000      1.2
@@ -8,6 +8,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <errno.h>
+#include <sys/wait.h>
 
 #include "ctwm_shutdown.h"
 #include "signals.h"
@@ -26,6 +28,15 @@
 // needs to trigger an action.
 bool SignalFlag = false;
 
+void ChildExit(int signum)
+{
+       int Errno = errno;
+       /* reap dead children, ignore status */
+       while (waitpid(-1, NULL, WNOHANG) > 0)
+               continue;
+       /* restore errno for interrupted sys calls */
+       errno = Errno;
+}
 
 /**
  * Setup signal handlers (run during startup)
@@ -46,9 +57,12 @@
        // die...
        signal(SIGALRM, SIG_IGN);
 
-       // This should be set by default, but just in case; explicitly don't
-       // leave zombies.
-       signal(SIGCHLD, SIG_IGN);
+       /* Setting SIGCHLD to SIG_IGN detaches children from the parent
+        * immediately, so it need not be waited for.
+        * In fact, you cannot wait for it, so a function like system()
+        * breaks.
+        */
+       signal(SIGCHLD, ChildExit);
 
        return;
 }

> Matthew Fuller     (MF4839)   |  fulle...@over-yonder.net
-Olaf.
-- 
___ Olaf 'Rhialto' Seibert                            <rhialto/at/falu.nl>
\X/ There is no AI. There is just someone else's work.           --I. Rose

signature.asc
Description: PGP signature