Re: [hackers] [sinit] Ensure children are reaped periodically || sin

From: Dimitris Papastamos <sin_AT_2f30.org>
Date: Mon, 19 Mar 2018 17:13:09 +0000

On Sun, Mar 18, 2018 at 01:09:08PM -0700, Michael Forney wrote:
> On 2018-03-18, git_AT_suckless.org <git_AT_suckless.org> wrote:
> > commit 170d599d58efee6c9be675a85c6e435d68e8a2de
> > Author: sin <sin_AT_2f30.org>
> > AuthorDate: Sun Mar 18 13:35:38 2018 +0000
> > Commit: sin <sin_AT_2f30.org>
> > CommitDate: Sun Mar 18 13:43:25 2018 +0000
> >
> > Ensure children are reaped periodically
> >
> > There is a pathological case where a parent receives SIGCHLD after its
> > child dies but does not reap it. After the parent dies, the child is
> > reparented to init but SIGCHLD is not redelivered.
> >
> > To fix this, periodically check if there are zombies pending to be
> > reaped.
>
> I'm assuming this patch is a response to the bug liwakura reported on
> #musl? I also did some investigation of this, and wasn't able to
> reproduce the issue on linux.

i knew about this bug for a while and discussed with emg about possible
solutions a year ago or so. somebody reminded me of this again because
of the discussion on #musl.

> Were you able to trigger this case somehow (maybe on another OS)?

no i didn't test the patch but the issue is real.

> > diff --git a/sinit.c b/sinit.c
> > index 93f9925..7166710 100644
> > --- a/sinit.c
> > +++ b/sinit.c
> > _AT_@ -7,7 +7,8 @@
> > #include <stdlib.h>
> > #include <unistd.h>
> >
> > -#define LEN(x) (sizeof (x) / sizeof *(x))
> > +#define LEN(x) (sizeof (x) / sizeof *(x))
> > +#define TIMEO 30
> >
> > static void sigpoweroff(void);
> > static void sigreap(void);
> > _AT_@ -20,6 +21,7 @@ static struct {
> > } sigmap[] = {
> > { SIGUSR1, sigpoweroff },
> > { SIGCHLD, sigreap },
> > + { SIGALRM, sigreap },
> > { SIGINT, sigreboot },
> > };
> >
> > _AT_@ -40,6 +42,7 @@ main(void)
> > sigprocmask(SIG_BLOCK, &set, NULL);
> > spawn(rcinitcmd);
> > while (1) {
> > + alarm(TIMEO);
>
> I wonder if there is another way to do this without a timeout. Perhaps
> a dedicated reaper thread?
>
> > sigwait(&set, &sig);
> > for (i = 0; i < LEN(sigmap); i++) {
> > if (sigmap[i].sig == sig) {
> > _AT_@ -63,6 +66,7 @@ sigreap(void)
> > {
> > while (waitpid(-1, NULL, WNOHANG) > 0)
> > ;
> > + alarm(TIMEO);
> > }
> >
> > static void
> >
> >
>
Received on Mon Mar 19 2018 - 18:13:09 CET