C - SIGINT handler not working with multiple threads, each of which has a popen process

Question

I have a small C program which does the following:

Open multiple thread(s), each of which spawns an ssh process using popen().
For each thread, processes the output from the FILE return from popen(). (Think opening an ssh command and tailing).

I'm trying to make it so that if the user presses Ctrl+C (Once), all the open ssh processes (spawned by threads) will be killed and the process will exit accordingly.

I've tried a solution similar to the top answer here: POSIX pthread programming

This example works, however, if I change the thread function to do what mine is doing (calling popen()), it requires me hitting Ctrl+C several times (presumably # of threads that are open). I'm guessing this has to do with the fact that the ssh process opened by popen() is not ignoring the SIGINT signal, allowing it to pass through to my defined handler. Is this correct?

Here's my current code:

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

void handler(int signo) {
    printf("I'm in the handler with SIGNAL %d!", signo);
    exit(0);
}

void *thread(void *argument) {
    // Hardcoding to "myuser@myhost.com" here for sake of example, but you
    // could image the host is different per thread
    FILE *file = popen("ssh myuser@myhost.com \"tail -f /path/to/some/log.log\"", "r");
    if (file == null) {
        printf("Error opening file\n");
        exit(1);
    }

    char line[2048];
    while (fgets(line, sizeof(line), fp) != NULL) {
        printf("%s\n", line);
    }
    return NULL;
}

int main(void) {
    // Block the SIGINT signal. The threads will inherit the signal mask.
    // This will avoid them catching SIGINT instead of this thread.
    sigset_t sigset, oldset;
    sigemptyset(&sigset);
    sigaddset(&sigset, SIGINT);
    pthread_sigmask(SIG_BLOCK, &sigset, &oldset);

    // Spawn the two threads.
    pthread_t thread1, thread2;
    pthread_create(&thread1, NULL, thread, &(unsigned int){1});
    pthread_create(&thread2, NULL, thread, &(unsigned int){2});

    // Install the signal handler for SIGINT.
    struct sigaction s;
    s.sa_handler = handler;
    sigemptyset(&s.sa_mask);
    s.sa_flags = 0;
    sigaction(SIGINT, &s, NULL);

    // Restore the old signal mask only for this thread.
    pthread_sigmask(SIG_SETMASK, &oldset, NULL);

    // Wait for SIGINT to arrive.
    pause();

    // Cancel both threads.
    pthread_cancel(thread1);
    pthread_cancel(thread2);

    // Join both threads.
    pthread_join(thread1, NULL);
    pthread_join(thread2, NULL);

    // Done.
    puts("Terminated.");
    return EXIT_SUCCESS;
}

Is my assumption on what's happening correct? If not, any ideas what I'm doing wrong?

Neither `printf()` nor `exit()` are async-signal-safe and therefore can not be safely used in a signal handler. The C standard [even states](https://port70.net/~nsz/c/c11/n1570.html#note188): "... a signal handler cannot, in general, call standard library functions." POSIX allows for calling async-signal-safe functions. — Andrew Henle, May 17 '20 at 16:14
To start with there is already a problem in `thread()` because popen returns `FILE*` not `File*`, secondly you use a variable called `fp` in your call to fgets but you never define a variable with name `fp`. I assume you meant `file` which you defined at the top of the function — txk2048, May 17 '20 at 16:14
@Toothless204 Yes, apologies. Was copying the code by hand into a StackOverflow question and mistyped. Fixed the code to be `FILE` as it is in my project. — user3330599, May 17 '20 at 16:21
Mixing threads, `fork()` (via `popen()`) and signals in a single program will produce an incredible and unfixable mess. — EOF, May 17 '20 at 16:23

score 0 · Answer 1 · answered May 17 '20 at 16:53

When you start your ssh programs, you are starting them with SIGINT masked, so those programs at the end of the pipe run ignoring SIGINT. You can test this by changing the popen command to something like "sleep 30; ls", start your program, hit ctrl-C, then type "ps s". You will likely see a few lines of the form "sh -c sleep 30; ls" and "sleep 30" with their signal dispositions. If you wait until they die naturally, then run the program again, but this time hit it with a "Ctrl-\" (SIGTERM), you will note that it cleans up immediately.

You can change the popen line to:

sigset_t nset;
pthread_sigmask(SIG_SETMASK, &oldset, &nset);
FILE *file = popen("ssh myuser@myhost.com \"tail -f /path/to/some/log.log\"", "r");
pthread_sigmask(SIG_SETMASK, &nset, NULL);

and move the definition of oldset to be common. Mostly, you will get the effect you want, but there is a race condition between these two calls to pthread_sigmask(), where a SIGINT could invoke handler in this context rather than the intended one.

To fixe the race condition you need to do is write your own popen()-like function which fork(2)s then sets the signal disposition in the child to the way you would like it to be, before exec(2)-ing the shell.

It is unfortunate, but signals and threads really don't mix well.

C - SIGINT handler not working with multiple threads, each of which has a popen process

1 Answers1