On Sat, Jan 28, 2023 at 03:04:50PM +0600, NRK wrote:
> Hi Hiltjo,
>
> On Sat, Jan 28, 2023 at 12:11:44AM +0100, Hiltjo Posthuma wrote:
> > We do not need to waitpid() on child processes. It is handled already
> > by using sigaction().
>
> Here's a test case. I see `cat` turning into a zombie on my system
> without the `waitpid`:
>
> [/tmp]~> cat test.c
> #include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/wait.h>
> #include <unistd.h>
>
> int main(void)
> {
> struct sigaction sa;
>
> puts("waiting for cat to become a zombie");
> sleep(1);
>
> sigemptyset(&sa.sa_mask);
> sa.sa_flags = SA_NOCLDSTOP | SA_NOCLDWAIT | SA_RESTART;
> sa.sa_handler = SIG_IGN;
> sigaction(SIGCHLD, &sa, NULL);
>
> puts("hello there");
> (void)getchar();
> }
> [/tmp]~> cat test.sh
> #!/bin/sh
>
> cat &
> exec ./test
> [/tmp]~> cc -o test test.c
> [/tmp]~> ./test.sh
> waiting for cat to become a zombie
> hello there
>
> Just putting `while (waitpid(-1, NULL, WNOHANG) > 0);` after the
> `sigaction` call (without worrying about EINTR) should still be better
> than not calling `waitpid` at all IMO.
>
> - NRK
>
Hi,
OK thanks, below is the final patch I intend to commit:
>From ca522f8f0e4c9ccb0d5203536abb118db8b36d7f Mon Sep 17 00:00:00 2001
From: Chris Down <[email protected]>
Date: Sat, 28 Jan 2023 12:39:28 +0100
Subject: [PATCH] Use sigaction(SA_NOCLDWAIT) for SIGCHLD handling
signal() semantics are pretty unclearly specified. For example, depending on OS
kernel and libc, the handler may be returned to SIG_DFL (hence the inner call
to read the signal handler). Moving to sigaction() means the behaviour is
consistently defined.
Using SA_NOCLDWAIT also allows us to avoid calling the non-reentrant function
die() in the handler.
waitpid() and sigaction() can also fail with EINTR, which may mean our zombie
handling fails, so block signals while setting things up to be careful.
Some addditional notes for archival purposes:
* NRK pointed out errno of waitpid could also theoretically get clobbered.
* The original patch was iterated on and modified by NRK and Hiltjo:
* SIG_DFL was changed to SIG_IGN, this is required, atleast on older systems
such as tested on Slackware 11.
* signals are not blocked using sigprocmask, because in theory it would
briefly for example also ignore a SIGTERM signal. It is OK if waitpid() is
(in
theory interrupted).
POSIX reference:
"Consequences of Process Termination":
https://pubs.opengroup.org/onlinepubs/9699919799/functions/_Exit.html#tag_16_01_03_01
---
dwm.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/dwm.c b/dwm.c
index 03baf42..88c3947 100644
--- a/dwm.c
+++ b/dwm.c
@@ -205,7 +205,6 @@ static void setmfact(const Arg *arg);
static void setup(void);
static void seturgent(Client *c, int urg);
static void showhide(Client *c);
-static void sigchld(int unused);
static void spawn(const Arg *arg);
static void tag(const Arg *arg);
static void tagmon(const Arg *arg);
@@ -1543,9 +1542,16 @@ setup(void)
int i;
XSetWindowAttributes wa;
Atom utf8string;
+ struct sigaction sa;
- /* clean up any zombies immediately */
- sigchld(0);
+ /* do not transform children into zombies when they terminate. */
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = SA_NOCLDSTOP | SA_NOCLDWAIT | SA_RESTART;
+ sa.sa_handler = SIG_IGN;
+ sigaction(SIGCHLD, &sa, NULL);
+
+ /* clean up any zombies (inherited from .xinitrc etc) immediately */
+ while (waitpid(-1, NULL, WNOHANG) > 0);
/* init screen */
screen = DefaultScreen(dpy);
@@ -1638,14 +1644,6 @@ showhide(Client *c)
}
}
-void
-sigchld(int unused)
-{
- if (signal(SIGCHLD, sigchld) == SIG_ERR)
- die("can't install SIGCHLD handler:");
- while (0 < waitpid(-1, NULL, WNOHANG));
-}
-
void
spawn(const Arg *arg)
{
--
2.39.1
--
Kind regards,
Hiltjo
