Re: [hackers] Re: [dwm][PATCH v2] Use sigaction(SA_NOCLDWAIT) for SIGCHLD handling

From: NRK <nrk_AT_disroot.org>
Date: Wed, 25 Jan 2023 12:06:25 +0600

On Tue, Jan 24, 2023 at 09:01:08PM +0100, Hiltjo Posthuma wrote:
> Although of course checking errno on a success condition is a bit wonky in this test case.

It was just an illustration that the malloc succeeded :)
A more real-world check would be something like this (in fact, I'm quite
sure this is precisely the cause of the xlib XIO error):

        if (poll(...) < 0) {
                if (errno == EINTR)
                        continue;
                else
                        error(...);
        }

> What about a simplified version of a patch below, it should do a similar thing:
>
> /* clean up any zombies immediately */
> - sigchld(0);
> + if (signal(SIGCHLD, SIG_IGN) == SIG_ERR)
> + die("can't ignore SIGCHLD:");

One issue here is that this will not clean up any existing zombies
(inherited from .xinitrc etc).

That's what that "clean up any zombies immediately" comment is referring
to. I think that comment ought to be a bit more descriptive and mention
where these zombies are coming from since it's not immediately obvious.

One other thing worth mentioning is that this behavior wasn't well
defined on earlier POSIX versions so some historical BSDs may have
different behavior. From (https://man7.org/linux/man-pages/man2/sigaction.2.html#NOTES):

| POSIX.1-1990 disallowed setting the action for SIGCHLD to
| SIG_IGN. POSIX.1-2001 and later allow this possibility, so that
| ignoring SIGCHLD can be used to prevent the creation of zombies.
| Nevertheless, the historical BSD and System V behaviors for ignoring
| SIGCHLD differ

The POSIX manpage for signal(3p) also says that "new applications should
use sigaction() rather than signal()" - probably due to these
incompatibility reasons.

But in any case, if you don't want to use `sigaction`, I think reaping
the existing zombies after the `signal` call _should_ work:

        - /* clean up any zombies immediately */
        - sigchld(0);
        + if (signal(SIGCHLD, SIG_IGN) == SIG_ERR)
        + die("can't ignore SIGCHLD:");
        + /* clean up any zombies (inherited from .xinitrc etc) immediately */
        + while (waitpid(-1, NULL, WNOHANG) > 0);

I haven't tested (or even compiled) the above patch. And it seems that
signals (much like threads) are really easy to mess up, so feel free to
correct if anything is wrong.

- NRK
Received on Wed Jan 25 2023 - 07:06:25 CET

This archive was generated by hypermail 2.3.0 : Wed Jan 25 2023 - 07:12:32 CET