5

I am writing a Unix program where a father process forks a child and later it has to send a SIGUSR1 signal to be handled by handler_sigusr1, and a SIGKILL signal to finish the program. My problem is that when I run the program, the child program should receive SIGUSR1 and then SIGKILL, but it just receives SIGKILL. Is there something wrong with my code? Thanks for your help!

void handler_sigusr1()
{
    printf("Signal SIGUSR1\n");
}

int main(int argc, char **argv)
{
    struct sigaction action;
    sigset_t new_mask, oldmask;
    int status;

    action.sa_flags = 0;

    sigaction(SIGTERM, &action, NULL);
    sigemptyset(&new_mask);
    sigaddset(&new_mask, SIGTERM);

    sigaction(SIGUSR1, &action, NULL);
    action.sa_handler = handler_sigusr1;
    sigaddset(&new_mask, SIGUSR1);

    child1 = fork();
    if(child1 == 0)
    {
        sigprocmask(SIG_BLOCK, &new_mask, &oldmask);
        sigsuspend(&new_mask);
    }
    if(child1 != 0)
    {
        kill(child1, SIGUSR1);
        kill(child1, SIGKILL);
    }
    wait(&status);
}
Daphne
  • 53
  • 1
  • 6
  • 2
    Note: don't use printf() and friends in signal handlers; they are not signal safe. – wildplasser Dec 05 '16 at 23:15
  • 2
    You are setting the `sa_handler` *after* calling `sigaction`. Of course that doesn't work. – kaylum Dec 05 '16 at 23:23
  • 1
    Note that you should ensure that the main fields in `struct sigaction action` are set before calling `sigaction()` at all. What do you expect `sigaction(SIGTERM, &action, NULL);` to do? You've not specified the `sa_handler` or set the `sa_mask` member of the structure, so you get quasi-random values specified. And why are you handling SIGTERM at all? You set `new_mask` carefully, but it isn't used in the next `sigaction()` call, so the interleaving of operations is peculiar. Your `sigprocmask()` call blocks all except SIGTERM and SIGUSR1 (and SIGKILL and SIGSTOP, which can't be blocked) — OK. – Jonathan Leffler Dec 05 '16 at 23:33
  • The child should be blocked until it receives SIGUSR1 and then it should be blocked again until it receives SIGKILL to finish the program. I don't know how can I do that... – Daphne Dec 05 '16 at 23:45

3 Answers3

7

There are 4 issues:

Issue 1

The code setting up the signal handler is wrong.

These 2 lines are in the wrong order:

    sigaction(SIGUSR1, &action, NULL);
    action.sa_handler = handler_sigusr1;

They should be

    action.sa_handler = handler_sigusr1;
    sigaction(SIGUSR1, &action, NULL);

Issue 2

Your parent process sends SIGKILL to your child right after it sends SIGUSR1. SIGKILL will terminate your child process - and it cannot be blocked or ignored.

Unless you're lucky, your child's sigusr1 handler will not be able to run or finish before the parent sends SIGKILL - which terminates the child process immediately.

You can greatly increase the chance SIGUSR1 gets delivered and handled by inserting a delay like so:

kill(child1, SIGUSR1);
sleep(1);
kill(child1, SIGKILL);

Issue 3

You have SIGUSR1 blocked, so it will not be delivered to your process.

Remove the line

 sigaddset(&new_mask, SIGUSR1);

Note that when the signal mask is being set with sigprocmask() or sigsuspend() , the signals set in the mask are the signals that are blocked.

Issue 4

You cannot safely use printf() in a signal handler.

printf() is not signal async safe, and cannot be used inside a signal handler. If you're unlucky it might not behave as it should behave when used in a signal handler. Use write() instead to write directly to stdout, e.g.

 write(1, "Signal SIGUSR1\n", 15);
Community
  • 1
  • 1
nos
  • 223,662
  • 58
  • 417
  • 506
4

First pass

Here's an amended version of your code which seems to work. Note that no process can ever block SIGKILL or SIGSTOP. I removed the SIGTERM code since is not being used. The signal handler calls write() rather than printf() since write() is async-signal-safe. It hand-encodes the signal number into the message since functions such as sprintf() are not async-signal-safe (neither is strlen(), officially) — see How to avoid using printf() in a signal handler for more information.

#include <signal.h>
#include <stdio.h>
#include <sys/wait.h>
#include <unistd.h>

static void handler_sigusr1(int signum)
{
    char msg[] = "Signal ?? SIGUSR1\n";
    msg[7] = (signum / 10) + '0';
    msg[8] = (signum % 10) + '0';
    write(2, msg, sizeof(msg) - 1);
}

int main(void)
{
    struct sigaction action;

    action.sa_flags = 0;
    action.sa_handler = handler_sigusr1;
    sigemptyset(&action.sa_mask);

    sigaction(SIGUSR1, &action, NULL);

    int child1 = fork();
    if (child1 == 0)
    {
        sigset_t new_mask, oldmask;
        sigemptyset(&new_mask);
        sigaddset(&new_mask, SIGUSR1);
        sigprocmask(SIG_BLOCK, &new_mask, &oldmask);
        sigsuspend(&new_mask);
        printf("Child (%d) unsuspended\n", (int)getpid());
    }

    if (child1 != 0)
    {
        kill(child1, SIGUSR1);
        sleep(1);
        kill(child1, SIGKILL);
        int status;
        int corpse = wait(&status);
        printf("Child (%d) - corpse = %d (0x%.4X)\n", child1, corpse, status);
    }
    return 0;
}

Example run:

Signal 30 SIGUSR1
Child (41875) - corpse = 41875 (0x0009)

The first message is from the signal handler in the child, obviously. It indicates that SIGUSR1 was received. The second message is from the parent. The hex number indicates that the child died from signal 9 (aka SIGKILL).

Second pass

What is that code in the child process doing? Answer: confusing me (and probably you too). I added an extra printf() before sigsuspend(), and ran the code and it shows:

Signal 30 SIGUSR1
Child (42688) suspending
Child (42688) - corpse = 42688 (0x0009)

The sigprocmask() call blocks SIGUSR1, but the parent on my machine is too quick and gets the signal delivered before that takes effect. The sigsuspend() then goes to sleep with SIGUSR1 blocked; the process dies because of the SIGKILL.

This variant of the code doesn't use sigprocmask() at all. It has a very different effect:

#include <signal.h>
#include <stdio.h>
#include <sys/wait.h>
#include <unistd.h>

static void signal_handler(int signum)
{
    char msg[] = "Signal ??\n";
    msg[7] = (signum / 10) + '0';
    msg[8] = (signum % 10) + '0';
    write(2, msg, sizeof(msg) - 1);
}

int main(void)
{
    struct sigaction action;

    action.sa_flags = 0;
    action.sa_handler = signal_handler;
    sigemptyset(&action.sa_mask);

    sigaction(SIGUSR1, &action, NULL);
    sigaction(SIGTERM, &action, NULL);

    int child1 = fork();
    if (child1 == 0)
    {
        sigset_t new_mask;
        sigemptyset(&new_mask);
        printf("Child (%d) suspending\n", (int)getpid());
        sigsuspend(&new_mask);
        printf("Child (%d) unsuspended\n", (int)getpid());
    }
    else
    {
        kill(child1, SIGUSR1);
        sleep(1);
        kill(child1, SIGTERM);
        sleep(1);
        kill(child1, SIGUSR1);
        sleep(1);
        kill(child1, SIGKILL);
        int status;
        int corpse = wait(&status);
        printf("Child (%d) - corpse = %d (0x%.4X)\n", child1, corpse, status);
    }

    return 0;
}

Example output:

Signal 30
Child (42969) suspending
Signal 15
Child (42969) unsuspended
Child (42969) - corpse = 42969 (0x0000)

This time, the child dies normally, after reporting that it was unsuspended, because SIGTERM and SIGUSR1 were not blocked.

You can ring the variations, adding SIGTERM and/or SIGUSR1 to the signal mask passed to sigsuspend(), maybe in response to different command line options.

I note that the man page for sigsuspend() on macOS Sierra says:

The signal mask set is usually empty to indicate that all signals are to be unblocked for the duration of the call.

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
0

Simple - the child process doesn't have time to handle the SIGUSR1 signal before it gets killed.

user253751
  • 57,427
  • 7
  • 48
  • 90