Search code examples
cunixsignalsposixhandler

Killing child with SIGTERM


I have 2 programs: 1) Father 2) Child. When Father receives SIGINT (CTRL-C) signal his handler sends a SIGTERM to his child. The problem is that often (not always, don't know why) it shows this error in loop after SIGINT:

Invalid Argument

Goal of the father is to create a child and then just being alive to be ready to handle SIGINT.

Father

#include "library.h"

static void handler();

int main(int argc, char* argv[]){
    int value, que_id;
    char str_que_id[10], **child_arg;
    pid_t child_pid;
    sigaction int_sa;

    //Create message queue
    do{
        que_id = msgget(IPC_PRIVATE, ALL_PERM | IPC_CREAT);
    }while(que_id == -1);
    snprintf(str_que_id, sizeof(str_que_id), "%d", que_id);

    //Set arguments for child
    child_arg = malloc(sizeof(char*) * 3);
    child[0] = "child";
    child[1] = str_que_id;
    child[2] = NULL;

    //Set handler for SIGINT
    int_sa.sa_handler = &handler;
    int_sa.sa_flags = SA_RESTART;
    sigemptyset(&int_sa.sa_mask);
    sigaddset(&int_sa.sa_mask, SIGALRM);
    sigaction(SIGINT, &int_sa, NULL);

    //Fork new child
    if(value = fork() == 0){
        child_pid = getpid();
        do{
            errno = 0;
            execve("./child", child_arg, NULL);
        }while(errno);
    }

    //Keep alive father
    while(1);

    return 0;
}

static void handler(){
    if(kill(child_pid, SIGTERM) != -1)
        waitpid(child_pid, NULL, WNOHANG);
    while(msgctl(que_id, IPC_RMID, NULL) == -1);
    free(child_arg);
    exit(getpid());
}

Goal of the child (only for now in my project) is just to wait a new message incoming from the message queue. Since there won't be any message, it will always be blocked.

Child

#include "library.h"

typedef struct _Msgbuf {
    long mtype;
    char[10] message;
} Msgbuf;

int main(int argc, char * argv[]){
    int que_id;

    //Recovery of message queue id
    que_id = atoi(argv[1]);

    //Set handler for SIGTERM
    signal(SIGTERM, handler);

    //Dynamic allocation of message
    received = calloc(1, sizeof(Msgbuf));

    while(1){
        do{
            errno = 0;
            //This will block child because there won't be any message incoming
            msgrcv(que_id, received, sizeof(Msgbuf) - sizeof(long), getpid(), 0);
            if(errno)
                perror(NULL);
        }while(errno && errno != EINTR);
    }
}

static void handler(){
    free(received);
    exit(getpid());
}

I know from the man pages on msgrcv():

The calling process catches a signal. In this case the system call fails with errno set to EINTR. (msgrcv() is never automatically restarted after being interrupted by a signal handler, regardless of the setting of the SA_RESTART flag when establishing a signal handler.)

So why does it go to loop printing that error? It should exit in the handler instead it seems that after the handler comes back and (since the free(received) ) it doesn't find the buffer of the message setting errno to EINVAL .


Solution

  • There are a number of problems with your program. It invokes undefined behaviour by calling exit, free, and msgctl from within the signal handlers. The table in the Signal Actions section of The Open Group Base Specifications lists the functions that are safe to call from within a signal handler. In most cases, you simply want to toggle a "running" flag from within the handler and have your main loop run until it is told to exit. Something like the following simple example:

    #include <signal.h>
    #include <stddef.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <unistd.h>
    
    
    /* this will be set when the signal is received */
    static sig_atomic_t running = 1;
    
    
    void
    sig_handler(int signo, siginfo_t *si, void *context)
    {
        running = 0;
    }
    
    
    int
    main(int argc, char *argv[])
    {
        int rc;
        struct sigaction sa;
    
        sigemptyset(&sa.sa_mask);
        sa.sa_flags = SA_SIGINFO;
        sa.sa_sigaction = &sig_handler;
        rc = sigaction(SIGINT, &sa, NULL);
        if (rc < 0) {
            perror("sigaction");
            exit(EXIT_FAILURE);
        }
    
        printf("Waiting for SIGINT\n");
        while (running) {
            printf("... sleeping for 10 seconds\n");
            sleep(10);
        }
        printf("Signal received\n");
    
        return 0;
    }
    

    I put together a more complex session on repl.it as well.

    The other problem is that you assume that errno retains a zero value across function calls. This is likely the case but the only thing that you should assume about errno is that it will be assigned a value when a library function returns a failure code -- e.g., read returns -1 and sets errno to something that indicates the error. The conventional way to call a C runtime library function is to check the return value and consult errno when appropriate:

    int bytes_read;
    unsigned char buf[128];
    
    bytes_read = read(some_fd, &buf[0], sizeof(buf));
    if (bytes_read < 0) {
        printf("read failed: %s (%d)\n", strerror(errno), errno);
    }
    

    Your application is probably looping because the parent is misbehaving and not waiting on the child or something similar (see above about undefined behavior). If the message queue is removed before the child exits, then the msgrcv call is going to fail and set errno to EINVAL. You should check if msgrcv is failing before you check errno. The child should also be terminating the loop when it encounters a msgrcv failure with errno equal to EINVAL since that is a terminal condition -- the anonymous message queue can never be recreated after it ceases to exist.