On Sat, Mar 11, 2023 at 06:20:07AM +0600, NRK wrote:
> issues with the current signal handler:
>
> 1. die() isn't async-signal-safe
> 2. there's a race-window during reinstating the signal handler allowing
> zombies to be produced (should be rare in practice).
> 3. if waitpid fails, it will clobber the errno which can lead to
> undesired codepath being taken on the resuming code (this one can
> actually occur in practice).
>
> to reproduce the 3rd issue:
>
> ~> ./tabbed &
> [1] 20644
> 0x1800003
> ~> while :; do kill -s SIGCHLD 20644; done >&2 2>/dev/null
> XIO: fatal IO error 10 (No child processes) on X server ":0"
> after 47 requests (47 known processed) with 0 events remaining.
> [1] + exit 1 ./tabbed
>
> set the signal handler to SIG_IGN to reap zombies automatically
> (according to POSIX.1-2001).
>
> NOTE: this patch follows dwm's commit 712d663. according to the manpage,
> none of the sa_flags being set are meaningful since we're using SIG_IGN.
> they used to be meaningful on earlier version of the patch where it was
> using SIG_DFL, i've kept them here just in case.
> ---
> Makefile | 2 +-
> tabbed.c | 21 +++++++++------------
> 2 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f8f5ba4..54ba350 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -11,7 +11,7 @@ DOCPREFIX = ${PREFIX}/share/doc/${NAME}
> # use system flags.
> TABBED_CFLAGS = -I/usr/X11R6/include -I/usr/include/freetype2 ${CFLAGS}
> TABBED_LDFLAGS = -L/usr/X11R6/lib -lX11 -lfontconfig -lXft ${LDFLAGS}
> -TABBED_CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE
> +TABBED_CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE
> -D_XOPEN_SOURCE=700L
>
> # OpenBSD (uncomment)
> #TABBED_CFLAGS = -I/usr/X11R6/include -I/usr/X11R6/include/freetype2
> ${CFLAGS}
> diff --git a/tabbed.c b/tabbed.c
> index eafe28a..477a041 100644
> --- a/tabbed.c
> +++ b/tabbed.c
> @@ -125,7 +125,6 @@ static void run(void);
> static void sendxembed(int c, long msg, long detail, long d1, long d2);
> static void setcmd(int argc, char *argv[], int);
> static void setup(void);
> -static void sigchld(int unused);
> static void spawn(const Arg *arg);
> static int textnw(const char *text, unsigned int len);
> static void toggle(const Arg *arg);
> @@ -976,9 +975,16 @@ setup(void)
> XWMHints *wmh;
> XClassHint class_hint;
> XSizeHints *size_hint;
> + 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 that might have been inherited */
> + while (waitpid(-1, NULL, WNOHANG) > 0);
>
> /* init screen */
> screen = DefaultScreen(dpy);
> @@ -1078,15 +1084,6 @@ setup(void)
> focus(-1);
> }
>
> -void
> -sigchld(int unused)
> -{
> - if (signal(SIGCHLD, sigchld) == SIG_ERR)
> - die("%s: cannot install SIGCHLD handler", argv0);
> -
> - while (0 < waitpid(-1, NULL, WNOHANG));
> -}
> -
> void
> spawn(const Arg *arg)
> {
> --
> 2.39.1
>
>
Hi NRK,
Thanks for the patch!
--
Kind regards,
Hiltjo
