Signal handler doesn't handle signal

Question

I have this homework where I'm supposed to write a C program that generates a child. The parent opens a file and loops forever by reading each line, printing the line number and the line content and then rewinding the file.

The child process sends a SIGUSR1 signal to the parent at random intervals. To create these intervals I chose to make the child sleep.

When receiving the first signal the parent skips the print statement. When receiving another SIGUSR1 signal it starts printing again. And so it goes on until a random number of signals have been sent. Then it ignores five signals. After ignoring five signals the parent process prints the number of signals received and exits after receiving one more signal.

My problem is that the program crashes when the signals are sent from the child process. The parent process has signal handlers but they are never used. What mistake am I making?

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include <signal.h>

void checkArguments(int nbrOfArg);
static void sig_exit(int signo);
static void sig_handler(int signo);

int sigcount, n;

int main(int argc, char *argv[]){
    int interval, i, linecount = 1;
    char buffer[1024];
    FILE *f;
    sigcount = 0;
    signal(SIGUSR1, sig_handler);
    srand(time(NULL)); // generate random numbers   
    n = rand() % 20 + 10;
    if(fork() != 0){ //parent process
        printf("%d\n", getpid()); //debugging
        checkArguments(argc);
        f = fopen(argv[1], "r"); //open file
        if(f == NULL){
            printf("Error opening file\n");
            exit(EXIT_FAILURE);
        }
        if(sigcount < n){ //n is random number of signals sent that starts/stops printing
            signal(SIGUSR1, sig_handler); //set signal handler
            do {
                while(!feof(f)){// infinite printing loop
                    fgets(buffer, 1024, f);
                    if(sigcount % 2 == 0){ //stop printing at odd number of signals received
                        printf("%d %s", linecount, buffer);
                    }   
                    linecount++;
                }
                rewind(f);
                linecount = 1;  
            } while(linecount == 1); //infinite loop
        } else if(sigcount < (n + 6)) {
            signal(SIGUSR1, SIG_IGN);   // ignore signal
        } else {
            printf("%d signals received", sigcount);
            signal(SIGUSR1, sig_exit); // terminate parent process
        }   
    } else {
        for(i = 0; i < n; i++){
            interval = rand() % 10 + 1; //send signals at random interval
            sigcount++; //number of signals sent
            printf("Sending signal %d from %d to %d\n", sigcount, getpid(), getppid()); //debugging
            kill(getppid(), SIGUSR1); //send signal
            printf("Child sleeping for %d seconds\n", interval); //debugging
            sleep(interval); //let child sleep after sending signal
        }
    }
}

/** Checks so command line argument is valid **/
void checkArguments(int nbrOfArg){
    int k;
    if(nbrOfArg != 2){
        printf("Wrong number of arguments");
        exit(-1);
    }
}

void sig_handler(int signo){
    if(signo == SIGUSR1){
        printf("Receivied SIGUSR1 signal\n");   
    } else printf("Error: received undefined signal\n");

}

void sig_exit(int signo){
    if(signo == SIGUSR1){
        printf("Received SIGUSR1 signal\n");
        exit(SIGUSR1);  
    } else printf("Error: received undefined signal\n");
}

Answer 1

In addition to the points made in the comments you should look at where you are setting up your signal handler, it's after you call fork() and start the child process so what you have is a race condition between the parent and the child:

         fork()
      /         \
 Parent          Child
 CheckArguments  Send signal     
 Open file
 Create Handler

So the child starts and sends a signal before the parent has registered its handler. You can check if this is the issue by adding a sleep in the child before it sends the first signal.

Also the behavior of signal varies between versions of Unix, when I tried your code on my machine the first signal was caught but none after that. This is because on my machine the signal handler is uninstalled after it has run, you need to re-enable it as part of the signal handler.

void sig_handler(int signo){
    if(signo == SIGUSR1)
    {
        printf("Receivied SIGUSR1 signal\n");
    } 
    else {
        printf("Error: received undefined signal\n");
    }
    signal(SIGUSR1, sig_handler);
}

I'd also consider changing to use sigaction( rather than signal(), it gives you much better control over signal handling.

Answer 2

Different systems behave differently using signal(). On a Mac running macOS (10.14.1 Mojave, but it applies to other versions too), the original code using signal() works OK — there are a variety of gotchas, but the signal handling works. In a VM running Ubuntu 18.04 LTS (hosted on the same Mac), the code using signal() doesn't work well because (as noted in the comments), the signal handling is reset to the default when the signal is caught, before the signal handler is entered. That sets up a race condition. Both these divergent behaviours are compliant with standard C — macOS provides 'reliable signals' and Linux doesn't.

However, all is not lost; both Linux and macOS have sigaction() which allows great control — and can be used to emulate either set of signal() behaviour. See also What is the difference between sigaction() and signal()?

There are some other problems that need to be dealt with. You should validate the arguments and (check that you can) open the file before forking; you should report the error. The child should note its original parent's PID so that if the parent dies, it can attempt to send it a signal and be notified that it failed. When the original parent dies, the parent PID switches to PID 1, the init process, which basically ends up ignoring the signal.

I fixed the problem with feof() — there's no reason to use feof() in the control condition of a loop (see while (!feof(file)) is always wrong!). Test the basic I/O function instead. (The C library on Ubuntu tags the fgets() function so that the return value must be used. Heed the compiler warnings.)

The code below slows down the main printing loop so it processes 4 lines per second, rather than running at full tilt. Both macOS and Linux have nanosleep(); Linux didn't make usleep() available, though it has a simpler (but less powerful) interface.

The code below separates sigcount used by the parent from i used by the child to count signals received and sent. I've also assumed C99 or later support and moved variable declarations nearer to where they're used.

As written, the code never enters the state where SIGUSR1 is ignored, let alone the state where it triggers an error. The child (now) sends enough signals (n *= 2; sends twice as many signals the parent 'expects'), but the parent is still stuck in the original code for if (sigcount < n). Note that you can't count signals received if you're ignoring those signals. That part of the code needs serious reworking. You should exit the file-reading loop when you've received the appropriate number of signals, and then simply count the next five signals (not ignore them), and then set the sig_exit handler. This is not implemented in the code below.

I didn't attempt to fix the problem with calling printf() in the signal handlers. It didn't seem to be causing trouble in this code (nor would I expect it to), but in general it is dangerous. See How to avoid using printf() in signal handlers? for more details.

The #define _XOPEN_SOURCE 700 allows the POSIX functions etc to be defined, even when the compilation options call for -std=c11 (which disables most extensions). The program source was in sig41.c and the program was therefore sig41. The code compiles cleanly with (GCC 8.2.0):

$ gcc -O3 -g -std=c11 -Wall -Wextra -Werror -Wmissing-prototypes -Wstrict-prototypes \
>     sig41.c -o sig41
$

Along with some other minor changes (reporting errors on stderr, for example), this code works on both Ubuntu and macOS:

#define _XOPEN_SOURCE 700
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>

static void sig_exit(int signo);
static void sig_handler(int signo);

static int sigcount, n;

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        fprintf(stderr, "Usage: %s filename\n", argv[0]);
        exit(EXIT_FAILURE);
    }

    FILE *f = fopen(argv[1], "r");
    if (f == NULL)
    {
        fprintf(stderr, "Error opening file %s for reading\n", argv[1]);
        exit(EXIT_FAILURE);
    }

    struct sigaction sa = { 0 };
    sa.sa_handler = sig_handler;
    sa.sa_flags = 0;
    sigemptyset(&sa.sa_mask);
    sigaction(SIGUSR1, &sa, 0);

    srand(time(NULL));
    n = rand() % 20 + 10;
    int pid = fork();
    if (pid < 0)
    {
        fprintf(stderr, "failed to fork!\n");
        exit(EXIT_FAILURE);
    }
    else if (pid != 0)
    {
        printf("%d\n", getpid());
        if (sigcount < n)
        {
            int linecount = 1;
            while (linecount == 1)
            {
                char buffer[1024];
                while (fgets(buffer, 1024, f))
                {
                    if (sigcount % 2 == 0)
                    {
                        printf("%d %s", linecount, buffer);
                    }
                    linecount++;
                    // nap time = quarter second
                    struct timespec nap = { .tv_sec = 0, .tv_nsec = 250000000 };
                    nanosleep(&nap, NULL);
                }
                rewind(f);
                linecount = 1;
            }
        }
        else if (sigcount < (n + 6))
        {
            printf("Going into SIG_IGN mode\n");
            sa.sa_handler = SIG_IGN;
            sigaction(SIGUSR1, &sa, 0);
        }
        else
        {
            printf("%d of %d signals received - sig_exit mode\n", sigcount, n);
            sa.sa_handler = sig_exit;
            sigaction(SIGUSR1, &sa, 0);
        }
    }
    else
    {
        fclose(f);
        int pid = getpid();
        int ppid = getppid();
        n *= 2;                     // Child needs to send more signals
        for (int i = 0; i < n; i++)
        {
            int interval = rand() % 10 + 1;
            printf("Sending signal %d of %d from %d to %d\n", i + 1, n, pid, ppid);
            if (kill(ppid, SIGUSR1) != 0)
            {
                fprintf(stderr, "Child failed to signal parent - exiting\n");
                exit(1);
            }
            printf("Child sleeping for %d seconds\n", interval);
            sleep(interval);
        }
    }
}

static void sig_handler(int signo)
{
    sigcount++;
    if (signo == SIGUSR1)
        printf("Received SIGUSR1 signal %d of %d\n", sigcount, n);
    else
        printf("Error: received undefined signal\n");
}

static void sig_exit(int signo)
{
    if (signo == SIGUSR1)
    {
        fprintf(stderr, "Received SIGUSR1 signal\n");
        exit(SIGUSR1);
    }
    else
        printf("Error: received undefined signal\n");
}

It's a bit hard to do a good job showing this working. I ended up interrupting the program to stop it.

$ ./sig41 sig41.c
3247
1 #define _XOPEN_SOURCE 700
Sending signal 1 of 30 from 3248 to 3247
Child sleeping for 7 seconds
Received SIGUSR1 signal 1 of 15
Sending signal 2 of 30 from 3248 to 3247
Child sleeping for 7 seconds
Received SIGUSR1 signal 2 of 15
30     sa.sa_flags = 0;
31     sigemptyset(&sa.sa_mask);
…
56                     }
57                     linecount++;
Sending signal 3 of 30 from 3248 to 3247
Child sleeping for 1 seconds
Received SIGUSR1 signal 3 of 15
Sending signal 4 of 30 from 3248 to 3247
Child sleeping for 4 seconds
Received SIGUSR1 signal 4 of 15
62                 rewind(f);
63                 linecount = 1;
…
76             sigaction(SIGUSR1, &sa, 0);
77         }
Sending signal 5 of 30 from 3248 to 3247
Child sleeping for 2 seconds
Received SIGUSR1 signal 5 of 15
Sending signal 6 of 30 from 3248 to 3247
Child sleeping for 3 seconds
Received SIGUSR1 signal 6 of 15
86         {
87             int interval = rand() % 10 + 1;
…
96         }
97     }
Sending signal 7 of 30 from 3248 to 3247
Child sleeping for 7 seconds
Received SIGUSR1 signal 7 of 15
Sending signal 8 of 30 from 3248 to 3247
Child sleeping for 10 seconds
Received SIGUSR1 signal 8 of 15
8 static void sig_exit(int signo);
9 static void sig_handler(int signo);
…
46         {
47             int linecount = 1;
Sending signal 9 of 30 from 3248 to 3247
Child sleeping for 5 seconds
Received SIGUSR1 signal 9 of 15
Sending signal 10 of 30 from 3248 to 3247
Child sleeping for 8 seconds
Received SIGUSR1 signal 10 of 15
68             printf("Going into SIG_IGN mode\n");
69             sa.sa_handler = SIG_IGN;
…
98 }
99 
Sending signal 11 of 30 from 3248 to 3247
Child sleeping for 9 seconds
Received SIGUSR1 signal 11 of 15
Sending signal 12 of 30 from 3248 to 3247
Child sleeping for 4 seconds
Received SIGUSR1 signal 12 of 15
18         exit(EXIT_FAILURE);
19     }
…
32     sigaction(SIGUSR1, &sa, 0);
33 
Sending signal 13 of 30 from 3248 to 3247
Child sleeping for 6 seconds
Received SIGUSR1 signal 13 of 15
Sending signal 14 of 30 from 3248 to 3247
Child sleeping for 6 seconds
Received SIGUSR1 signal 14 of 15
58                     // nap time = quarter second
59                     struct timespec nap = { .tv_sec = 0, .tv_nsec = 250000000 };
…
80     {
81         fclose(f);
Sending signal 15 of 30 from 3248 to 3247
Child sleeping for 7 seconds
Received SIGUSR1 signal 15 of 15
Sending signal 16 of 30 from 3248 to 3247
Child sleeping for 8 seconds
Received SIGUSR1 signal 16 of 15
110 {
111     if (signo == SIGUSR1)
…
22     if (f == NULL)
23     {
Sending signal 17 of 30 from 3248 to 3247
Child sleeping for 1 seconds
Received SIGUSR1 signal 17 of 15
Sending signal 18 of 30 from 3248 to 3247
Child sleeping for 6 seconds
Received SIGUSR1 signal 18 of 15
28     struct sigaction sa = { 0 };
29     sa.sa_handler = sig_handler;
…
^C
$

If you work through the output, you can see the output pausing for a given number of lines — if the child sleeps for 1 second, 4 output lines are omitted.