Hello Jann,
On 1/8/16 5:39 PM, Jann Horn wrote:
> On Fri, Jan 08, 2016 at 05:21:55PM +0100, Michael Kerrisk (man-pages) wrote:
>> Hi Jann,
>>
>> Your mail is a little cryptic. It would be best to start with
>> a brief summary of your point--something like the text of your
>> patch at the end of the mail.
>
> Ok, will do that next time. I wanted to avoid duplicating the content.
>
>
>> On 01/06/2016 07:23 PM, Jann Horn wrote:
>>> Proof:
>>> In kernel/sys.c:
>>>
>>> case PR_SET_PDEATHSIG:
>>> if (!valid_signal(arg2)) {
>>> error = -EINVAL;
>>> break;
>>> }
>>> me->pdeath_signal = arg2;
>>> break;
>>
>> I don't understand how the code above relates to the point you
>> want to make. (Or maybe you mean: "look, there's no check here
>> to see that if the parent is already dead"; but it would help
>> to state that explicitly).
>
> Yes, that's what I meant.
>
>
>>> Testcase:
>>>
>>> #include <sys/prctl.h>
>>> #include <err.h>
>>> #include <unistd.h>
>>> #include <signal.h>
>>> #include <stdio.h>
>>>
>>> void ponk(int s) {
>>> puts("ponk!");
>>> }
>>>
>>> int main(void) {
>>> if (fork() == 0) {
>>> if (fork() == 0) {
>>> sleep(1);
>>> signal(SIGUSR1, ponk);
>>> prctl(PR_SET_PDEATHSIG, SIGUSR1, 0, 0, 0);
>>> sleep(1);
>>> return 0;
>>> }
>>> return 0;
>>> }
>>>
>>> sleep(3);
>>> return 0;
>>> }
>>> ---
>>> man2/prctl.2 | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/man2/prctl.2 b/man2/prctl.2
>>> index 5cea3bb..3dce8e9 100644
>>> --- a/man2/prctl.2
>>> +++ b/man2/prctl.2
>>> @@ -670,6 +670,9 @@ In other words, the signal will be sent when that thread terminates
>>> (via, for example,
>>> .BR pthread_exit (3)),
>>> rather than after all of the threads in the parent process terminate.
>>> +
>>> +If the parent has already died by the time the parent death signal
>>> +is set, the new parent death signal will not be sent.
>>
>> In a way, this seems almost obvious. But perhaps it is better to make the
>> point explicitly, as you suggest. But, because there may have been a
>> previous PR_SET_PDEATHSIG, I'd prefer something like this:
>>
>> [[
>> If the caller's parent has already died by the time of this
>> PR_SET_PDEATHSIG operation, the operation shall have no effect.
>> ]]
>>
>> What do you think?
>
> I don't think "no effect" would be strictly correct because weird stuff
> happens on subreaper death - I'm not sure whether this is intended or a
> bug though:
>
> $ cat deathsig2.c
> #include <sys/prctl.h>
> #include <err.h>
> #include <unistd.h>
> #include <signal.h>
> #include <stdio.h>
>
> void ponk(int s) {
> puts("ponk!");
> }
>
> int main(void) {
> if (fork() == 0) {
> prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0);
> puts("enabled subreaper");
> if (fork() == 0) {
> if (fork() == 0) {
> sleep(1);
> puts("setting deathsig...");
> signal(SIGUSR1, ponk);
> prctl(PR_SET_PDEATHSIG, SIGUSR1, 0, 0, 0);
> sleep(2);
> return 0;
> }
> puts("parent will die now, causing reparent to subreaper");
> return 0;
> }
> sleep(2);
> puts("subreaper will die now");
> return 0;
> }
> sleep(4);
> return 0;
> }
> $ gcc -o deathsig2 deathsig2.c
> $ cat deathsig3.c
> #include <sys/prctl.h>
> #include <err.h>
> #include <unistd.h>
> #include <signal.h>
> #include <stdio.h>
>
> void ponk(int s) {
> puts("ponk!");
> }
>
> int main(void) {
> if (fork() == 0) {
> prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0);
> puts("enabled subreaper");
> if (fork() == 0) {
> if (fork() == 0) {
> puts("setting deathsig...");
> signal(SIGUSR1, ponk);
> prctl(PR_SET_PDEATHSIG, SIGUSR1, 0, 0, 0);
> sleep(3);
> return 0;
> }
> sleep(1);
> puts("parent will die now, causing reparent to subreaper");
> return 0;
> }
> sleep(2);
> puts("subreaper will die now");
> return 0;
> }
> sleep(4);
> return 0;
> }
> $ gcc -o deathsig3 deathsig3.c
> $ ./deathsig2
> enabled subreaper
> parent will die now, causing reparent to subreaper
> setting deathsig...
> subreaper will die now
> ponk!
> $ ./deathsig3
> enabled subreaper
> setting deathsig...
> parent will die now, causing reparent to subreaper
> ponk!
> subreaper will die now
> $
>
> I didn't manage to find the reason for that in the code.
So, I think that the reason was that you were looking at the
wrong code.
I could not reproduce what you were seeing with my own
test program. Then I realized there's a bug in your code.
You child calls sleep(3). The first SIGUSR1 causes the
sleep() to terminate, at which point the child terminates
before it sees the second SIGUSR1 from the subreaper
termination. If you add a second sleep() call in the
child, then you see "ponk!" twice.
> Sorry, I probably should have tried to figure out the details of
> this before sending a manpages patch.
Here's the story as I understand it:
The parent-death signal is sent upon subsequent termina‐
tion of the parent thread and also upon termination of
each subreaper process (see the description of
PR_SET_CHILD_SUBREAPER above) to which the caller is
subsequently reparented. If the parent thread and all
ancestor subreapers have already terminated by the time
of the PR_SET_PDEATHSIG operation, then no parent-death
signal is sent to the caller.
I've applied that patch to the manual page. See below for more details.
Thanks for the report!
Cheers,
Michael
PS You use signal() in your test above. It's a very bad API to
use, because it's behavior varies according to compilation options.
In particular, with suitable compilation options or feature test
macro definitions, one gets the old System V signal() behavior,
which includes the effect of the SA_RESETHAND flag. In that case,
the second SIGUSR1 signal, generated when the subreaper terminated,
would have simply have killed the child. This also threw me while
I was testing your code, since I have some of those compilation
options on my default.
PPS My test code (which demos following the code)
$ cat pdeath_signal.c
#include <sys/prctl.h>
#include <signal.h>
#include <sys/types.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#define errExit(msg) do { perror(msg); exit(EXIT_FAILURE); \
} while (0)
static void
handler(int sig, siginfo_t *si, void *ucontext)
{
printf("*********** Child (%ld) got signal; si_pid = %d; si_uid = %d\n",
(long) getpid(), si->si_pid, si->si_uid);
printf(" Parent PID is now %ld\n", (long) getppid());
}
int
main(int argc, char *argv[])
{
struct sigaction sa;
int childPreSleep, childPostSleep, parentSleep;
if (argc < 2) {
fprintf(stderr, "Usage: %s child-pre-sleep "
"[child-post-sleep [parent-sleep [subreaper-sleep...]]]\n",
argv[0]);
exit(EXIT_FAILURE);
}
childPreSleep = atoi(argv[1]);
if (argc > 2)
childPostSleep = atoi(argv[2]);
if (argc > 3)
parentSleep = atoi(argv[3]);
/* Optionally create a series of subreapers */
if (argc > 4) {
for (int sr = 4; sr < argc; sr++) {
if (prctl(PR_SET_CHILD_SUBREAPER, 1) == -1)
errExit("prctl");
printf("%ld marked itself as a subreaper\n", (long) getpid());
switch (fork()) {
case -1:
errExit("fork");
case 0:
break;
default:
printf("%ld subreaper about to sleep %s seconds\n",
(long) getpid(), argv[sr]);
sleep(atoi(argv[sr]));
printf("%ld subreaper about to terminate\n", (long) getpid());
exit(EXIT_SUCCESS);
}
}
}
switch (fork()) {
case -1:
errExit("fork");
case 0:
sa.sa_flags = SA_SIGINFO;
sigemptyset(&sa.sa_mask);
sa.sa_sigaction = handler;
if (sigaction(SIGUSR1, &sa, NULL) == -1)
errExit("sigaction");
if (childPreSleep > 0) {
printf("Child about to sleep %d seconds before setting "
"PR_SET_PDEATHSIG\n", childPreSleep);
sleep(childPreSleep);
}
printf("Child about to set PR_SET_PDEATHSIG\n");
if (prctl(PR_SET_PDEATHSIG, SIGUSR1) == -1)
errExit("prctl");
printf("Child about to sleep\n");
for (int j = 0; j < childPostSleep; j++)
sleep(1);
printf("Child about to exit\n");
exit(EXIT_SUCCESS);
default:
printf("Parent (%ld) about to sleep for %d seconds\n",
(long) getpid(), parentSleep);
sleep(parentSleep);
printf("Parent (%ld) terminating\n", (long) getpid());
exit(EXIT_SUCCESS);
}
}
Some demonstrations:
First, show that the pdeath signal is sent when the parent
terminates:
$ ./pdeath_signal 0 10 4
Parent (18595) about to sleep for 4 seconds
Child about to set PR_SET_PDEATHSIG
Child about to sleep
Parent (18595) terminating
*********** Child (18596) got signal; si_pid = 18595; si_uid = 1000
Parent PID is now 1403
$ Child about to exit
But the signal is not sent if the parent terminates before the
child uses PR_SET_PDEATHSIG:
$ ./pdeath_signal 2 10 0
Parent (18707) about to sleep for 0 seconds
Parent (18707) terminating
Child about to sleep 2 seconds before setting PR_SET_PDEATHSIG
$ Child about to set PR_SET_PDEATHSIG
Child about to sleep
Child about to exit
Demonstrate that the pdeath signal is sent on termination of each
ancestor subreaper process:
$ ./pdeath_signal 2 10 3 7 6 5
18786 marked itself as a subreaper
18786 subreaper about to sleep 7 seconds
18787 marked itself as a subreaper
18787 subreaper about to sleep 6 seconds
18788 marked itself as a subreaper
18788 subreaper about to sleep 5 seconds
Parent (18789) about to sleep for 3 seconds
Child about to sleep 2 seconds before setting PR_SET_PDEATHSIG
Child about to set PR_SET_PDEATHSIG
Child about to sleep
Parent (18789) terminating
*********** Child (18790) got signal; si_pid = 18789; si_uid = 1000
Parent PID is now 18788
18788 subreaper about to terminate
*********** Child (18790) got signal; si_pid = 18788; si_uid = 1000
Parent PID is now 18787
18787 subreaper about to terminate
*********** Child (18790) got signal; si_pid = 18787; si_uid = 1000
Parent PID is now 18786
18786 subreaper about to terminate
*********** Child (18790) got signal; si_pid = 18786; si_uid = 1000
Parent PID is now 1403
$ Child about to exit
But in the case where some subreapers terminate before they
have a chance to adopt the child, the terminations of those
subreapers do not result in a signal for the child:
$ ./pdeath_signal 2 10 3 5 6 7
18836 marked itself as a subreaper
18836 subreaper about to sleep 5 seconds
18837 marked itself as a subreaper
18837 subreaper about to sleep 6 seconds
18838 marked itself as a subreaper
18838 subreaper about to sleep 7 seconds
Parent (18839) about to sleep for 3 seconds
Child about to sleep 2 seconds before setting PR_SET_PDEATHSIG
Child about to set PR_SET_PDEATHSIG
Child about to sleep
Parent (18839) terminating
*********** Child (18840) got signal; si_pid = 18839; si_uid = 1000
Parent PID is now 18838
18836 subreaper about to terminate
$ 18837 subreaper about to terminate
18838 subreaper about to terminate
*********** Child (18840) got signal; si_pid = 18838; si_uid = 1000
Parent PID is now 1403
Child about to exit
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
