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

--q6tO0GvJqH1VTrB7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
> >=20
> > Or, alternatively, a proper signal handler for SIGCHLD could be set up.
>=20
> 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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>
=20
 #include "ctwm_shutdown.h"
 #include "signals.h"
@@ -26,6 +28,15 @@
 // needs to trigger an action.
 bool SignalFlag =3D false;
=20
+void ChildExit(int signum)
+{
+	int Errno =3D errno;
+	/* reap dead children, ignore status */
+	while (waitpid(-1, NULL, WNOHANG) > 0)
+		continue;
+	/* restore errno for interrupted sys calls */
+	errno =3D Errno;
+}
=20
 /**
  * Setup signal handlers (run during startup)
@@ -46,9 +57,12 @@
 	// die...
 	signal(SIGALRM, SIG_IGN);
=20
-	// 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);
=20
 	return;
 }

> Matthew Fuller     (MF4839)   |  [email protected]
-Olaf.
--=20
___ Olaf 'Rhialto' Seibert                            <rhialto/at/falu.nl>
\X/ There is no AI. There is just someone else's work.           --I. Rose

--q6tO0GvJqH1VTrB7
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEFVAhiiWjqgwBVdQAmYnGRWHD+9MFAmWjA0cACgkQmYnGRWHD
+9Pragf+JEgANzILga4YMaPl+JrexASZMtVHmIrr9OMxUTZu8Oasl3HRSyka4wvT
4YHz3r7Su/wRfs1sXkww7fg/ICui+iK6Z1REWQbsa5Av/skJqlyM5k/MFThqwMKg
yFPEsn7Wo6cl3SrrOV9LTe3YM6yUytBI+0kWXTJ/UPcm0W5UGure0/vnUa9d3kzu
6NbNQlaRayCdQC7YytRCtOkKbAW6Q1UDrnF/yc5JXZUsuM5VvQPs8MGzq3mRdm6A
6RWdKou/xXLIIfApRbtlwmh/F2+1OcXnliDsSqd9NvxobWB+r4M4oh6voaG+5c/R
ATaMjePc+63gzer8KVOsIxo4ACK5Pg==
=3+9V
-----END PGP SIGNATURE-----

--q6tO0GvJqH1VTrB7--