How to terminate threads cleanly in C?

Question

I am trying to write a multithreaded application in C for the Raspberry Pi in raspbian environment (UNIX system).

Apart from the main thread three other threads are created and do the following:

1. the first looks at the output of a PIR sensor and if movement is detected it takes a picture. The thread function is task1();
2. the second uses sigwait and alarm() to measure the temperature every given seconds. The thread function is task2()
3. The third thread checks if a new picture is taken and if so it does some other stuff. The synchronization with the first thread is done with a global flag, a mutex and with pthread_cond_wait. The thread function is task3().

All the thread functions have an infinite loop. The execution of the program seems good.

The main thread call the function pause() and then pthread_cancel() to exit cleanly from each thread (lowering the pins). At first I did not use the signal handler and the process quit without calling the exiting thread functions registered with the function pthread_cleanup_push. This because pause() returns only if the handler returns. That is why I added my signal handler which returns.

In this way the pthread_cancel are called correctly and also the exiting thread functions are called correctly (the output is printed) but the process keeps running even with pressing CTRL-C or calling kill from another terminal window.

I think I messed up with the masks so that the signal generated by pthread_cancel (if any) has no effect.

Apart from this I have read that in general it is bad practice using pthread_cancel so my question is:

what is the best way to exit cleanly from each thread (especially in my case)? Shall I use another global flag? With mutex or read-write lock? Should I set it from the main thread or handler?

Any suggestion will be appreciated.

EDIT: If instead of calling pthread_cancel I use a global flag for the infinite loops, how would you set the condition in task3()?

NOTE: the code is incomplete for the sake of brevity. I tried to emphasize the logic. If needed I will add all the code.

#include<wiringPi.h>  
#include<stdlib.h>
#include<stdio.h>
#include<signal.h>
#include<stdint.h>
#include<pthread.h>

g_new_pic_flag=FALSE;
pthread_cond_t g_new_pic_cond = PTHREAD_COND_INITIALIZER;
pthread_mutex_t g_new_pic_m = PTHREAD_MUTEX_INITIALIZER;

/* FUNCTION DECLARATION */

/*We define thread exit functions so that each pin 
is lowered by the thread in which it is used avoiding
race condition between the signal handler of the main thread
and the other threads*/
void exitingThreadTask1(void* arg);
void exitingThreadTask2(void* arg);
void exitingThreadTask3(void* arg);

void* task1(void *arg); //thread function for the motion sensor
void* task2(void *arg); //thread function for the temperature reading
void* task3(void *arg); //thread function to post data on IOT platforms

/*Signal handler to return from pause*/
void sig_handler(int signo);

int main()
{
    int err;
    sigset_t omask, mask;
    pthread_t thread_motionSensor;
    pthread_t thread_tempReading;
    pthread_t thread_platformPost;

    printf("Created threads IDs\n");

    if (wiringPiSetup()<0)
    {
        printf("WiringPi error\n");
        return -1;
    }
    printf("WiringPi is ok\n");

    if (signal(SIGQUIT, sig_handler)==SIG_ERR)
        printf("Error on recording SIGQUITHANDLER\n");
    if (signal(SIGINT, sig_handler)==SIG_ERR)
        printf("Error on recording SIGINTHANDLER\n");
    if (signal(SIGTERM, sig_handler)==SIG_ERR)
        printf("Error on recording SIGTERMHANDLER\n");

    /*Create a new mask to block all signals for the following thread*/
    sigfillset(&mask);
    pthread_sigmask(SIG_SETMASK, &mask, &omask);
    printf("Trying to create threads\n");
    if ((err = pthread_create (&thread_motionSensor, NULL, task1, NULL))!=0)
    {
    printf("Thread 1 not created: error %d\n", err);
        err_exit((const char)err, "pthread_create error");
    }
    printf("Thread 1 created. Trying to create Thread 2\n");
    if((err = pthread_create (&thread_tempReading,   NULL, task2, NULL))!=0)
    {
    printf("Thread 2 not created: error %d\n", err);
        err_exit((const char)err, "pthread_create error");
    }
    printf("Thread 2 created. Trying to create Thread 3\n");
    if ((err = pthread_create (&thread_platformPost, NULL, task3, NULL))!=0)
    {
     printf("Thread 3 not created: error %d %d\n", err);
         err_exit((const char)err, "pthread_create error");
    }
    printf("Thread 3 created\n");
    /*The main thread must block the SIGALRM but catch SIGINT
    SIGQUIT, SIGTERM, SIgkILL*/
    sigemptyset(&omask);
    sigaddset(&omask, SIGINT);
    sigaddset(&omask, SIGQUIT);
    sigaddset(&omask, SIGKILL);
    sigaddset(&omask, SIGTERM);

    pthread_sigmask(SIG_UNBLOCK, &omask, NULL);
    printf("Main thread waiting for signal\n");
    pause();
    printf("Exit signal received: cancelling threads\n");

    pthread_cancel(thread_motionSensor);
    pthread_cancel(thread_tempReading);
    pthread_cancel(thread_platformPost);
    pthread_join(thread_motionSensor, NULL);
    pthread_join(thread_tempReading,  NULL);
    pthread_join(thread_platformPost, NULL);
    printf("Exiting from main thread and process\n");
    exit(0);
}

void* task1(void *arg)
{
    //INITIALIZING
    pthread_cleanup_push(exitingThreadTask1, NULL);
    while(1)
    {
        //do stuff1
    }
    pthread_cleanup_pop(0);
    pthread_exit(0);

}

void* task2(void *arg)
{
    static const unsigned char schedule_time = 5;
    int signo, err;
    /*
    We set a local mask with SIGALARM for the function sigwait
    All signals have already been blocked
    */
    sigset_t alarm_mask;
    sigemptyset(&alarm_mask);
    sigaddset(&alarm_mask, SIGALRM);
    alarm(schedule_time);
    pthread_cleanup_push(exitingThreadTask2, NULL);
    while (1)
    {
        err = sigwait(&alarm_mask, &signo); //signo == SIGALRM check
        if (err!=0)
            err_exit(err, "sigwait failed\n");
        //do stuff
        alarm(schedule_time);
    }
    pthread_cleanup_pop(0);
    pthread_exit(0);
}

void* task3(void *arg)
{
    pthread_cleanup_push(exitingThreadTask3, NULL);
    while(1)
    {
        pthread_mutex_lock(&g_new_pic_m);
        while(g_new_pic_flag==FALSE)
        {
            pthread_cond_wait(&g_new_pic_cond, &g_new_pic_m);
        }
        pthread_mutex_unlock(&g_new_pic_m);
        //do stuff
    }
    pthread_cleanup_pop(0);
    pthread_exit(0);

}

void exitingThreadTask1(void* arg)
{
    printf("Thread of task 1 exiting\n");
    digitalWrite(OUTPIN, LOW);
    digitalWrite(INPIN, LOW);
    printf("Pins lowered\n");
    pthread_exit((void*)0);
}

void exitingThreadTask2(void* arg)
{
    printf("Thread of task 2 exiting\n");
    digitalWrite(DHTPIN, LOW);
    printf("Pin lowered\n");
    pthread_exit((void*)0);
}

void exitingThreadTask3(void* arg)
{
    printf("Thread of task 3 exiting\n");
    pthread_exit((void*)0);
}

void sig_handler(int signo)
{
    printf("Running handler to return from pause\n");
    return;
}

Answer 1

You must not call pthread_exit() in the cleanup functions, because pthread_exit() will also call the cleanup function registered for the thread.

So, in your program, the cleanup function is called recursively and the threads never exit.

About the kill from another terminal, the command kill -9 and the pid of the process should always work because SIGKILL can't be ignored nor caught.

And in the signal handler function, you have to use async-signal-safe functions, printf() isn't async-signal-safe.

Another way to wait for a signal in the main thread is to use sigwait() or sigwaitinfo() instead of pause(), like you did for SIGALARM in a thread. So it won't need to register a handler function, but it will need to block the signals to be caught in all threads.

EDIT: To answer your last comment.

Exiting the threads task2() and task3() with a flag seems to be complex, because the main thread have to send SIGALRM to task2 in order to wake it up, and also signal the condition in order to wake up task3.

I modified your code to try to use a flag, but i may have missed an eventual problem because synchronizing threads may be complex.

In the case of your program, I haven't enough knwoledge to say if it is better to use pthread_cancel() and pthread_testcancel(), or to use flags. However, pthread_cancel() seems to be able to cancel without synchronization problems, threads that are waiting for signals or for a condition.

Using a flag, for task3, there could be the following problem:

1. task3 check the flag that is 0
2. main thread set the flag to 1
3. main thread signal the condition
4. task3 begin to wait for the condition

In this case, thread task3 won't exit, because it wasn't waiting when the condition was signaled. I'am not sure, but this problem is maybe avoided by protecting the flag with the same mutex we use for the condition. Because when the flag will be set and the condition signaled, task3 will be waiting for the condition or doing work out of the critical section.

I don't know if there may be a problem for task2, for example if the signal is lost due to an internal problem, but normally, the signal will be pending.

Here is the code of my test. I placed 1 as argument for the function pthread_cleanup_pop(), to make the threads execute the cleanup functions.

#include<stdlib.h>
#include<stdio.h>
#include<signal.h>
#include<stdint.h>
#include<pthread.h>

#define FALSE 0
volatile sig_atomic_t g_new_pic_flag=FALSE;
pthread_cond_t g_new_pic_cond = PTHREAD_COND_INITIALIZER;
pthread_mutex_t g_new_pic_m = PTHREAD_MUTEX_INITIALIZER;
volatile int g_shutdown_task_3 = 0;

volatile int g_shutdown_task_1_2 = 0;
pthread_mutex_t g_shutdown_mutex = PTHREAD_MUTEX_INITIALIZER;
/* FUNCTION DECLARATION */

/*We define thread exit functions so that each pin 
is lowered by the thread in which it is used avoiding
race condition between the signal handler of the main thread
and the other threads*/
void exitingThreadTask1(void* arg);
void exitingThreadTask2(void* arg);
void exitingThreadTask3(void* arg);

void* task1(void *arg); //thread function for the motion sensor
void* task2(void *arg); //thread function for the temperature reading
void* task3(void *arg); //thread function to post data on IOT platforms

/*Signal handler to return from pause*/
void sig_handler(int signo);

void err_exit(char err, char *msg) {
  printf("\nError: %s\n",msg);
  exit(1);
}

int main()
{
    int err;
    sigset_t omask, mask;
    pthread_t thread_motionSensor;
    pthread_t thread_tempReading;
    pthread_t thread_platformPost;

    printf("Created threads IDs\n");
    /*
    if (wiringPiSetup()<0)
    {
        printf("WiringPi error\n");
        return -1;
    }
    */
    printf("WiringPi is ok\n");

    if (signal(SIGQUIT, sig_handler)==SIG_ERR)
        printf("Error on recording SIGQUITHANDLER\n");
    if (signal(SIGINT, sig_handler)==SIG_ERR)
        printf("Error on recording SIGQUITHANDLER\n");
    if (signal(SIGTERM, sig_handler)==SIG_ERR)
        printf("Error on recording SIGQUITHANDLER\n");

    /*Create a new mask to block all signals for the following thread*/
    sigfillset(&mask);
    pthread_sigmask(SIG_SETMASK, &mask, &omask);
    printf("Trying to create threads\n");
    if ((err = pthread_create (&thread_motionSensor, NULL, task1, NULL))!=0)
    {
    printf("Thread 1 not created: error %d\n", err);
        err_exit((const char)err, "pthread_create error");
    }
    printf("Thread 1 created. Trying to create Thread 2\n");
    if((err = pthread_create (&thread_tempReading,   NULL, task2, NULL))!=0)
    {
    printf("Thread 2 not created: error %d\n", err);
        err_exit((const char)err, "pthread_create error");
    }
    printf("Thread 2 created. Trying to create Thread 3\n");
    if ((err = pthread_create (&thread_platformPost, NULL, task3, NULL))!=0)
    {
     printf("Thread 3 not created: error %d %d\n", err);
         err_exit((const char)err, "pthread_create error");
    }
    printf("Thread 3 created\n");
    /*The main thread must block the SIGALRM but catch SIGINT
    SIGQUIT, SIGTERM, SIgkILL*/
    sigemptyset(&omask);
    sigaddset(&omask, SIGINT);
    sigaddset(&omask, SIGQUIT);
    sigaddset(&omask, SIGKILL);
    sigaddset(&omask, SIGTERM);

    pthread_sigmask(SIG_UNBLOCK, &omask, NULL);
    printf("Main thread waiting for signal\n");
    pause();
    printf("Exit signal received: cancelling threads\n");
    
    
    pthread_mutex_lock(&g_shutdown_mutex);
    g_shutdown_task_1_2 = 1;
    pthread_mutex_unlock(&g_shutdown_mutex);
    pthread_mutex_lock(&g_new_pic_m);
    g_shutdown_task_3 = 1;
    pthread_cond_signal(&g_new_pic_cond);
    pthread_mutex_unlock(&g_new_pic_m);
    
    pthread_kill(thread_tempReading,SIGALRM);
    

    pthread_join(thread_motionSensor, NULL);
    pthread_join(thread_tempReading,  NULL);
    pthread_join(thread_platformPost, NULL);
    printf("Exiting from main thread and process\n");
    exit(0);
}

void* task1(void *arg)
{
    //INITIALIZING
    pthread_cleanup_push(exitingThreadTask1, NULL);
    while(1)
    {
        pthread_mutex_lock(&g_shutdown_mutex);
        if(g_shutdown_task_1_2) {
          pthread_mutex_unlock(&g_shutdown_mutex);
          break;
        }
        pthread_mutex_unlock(&g_shutdown_mutex);
        //do stuff1
        sleep(1);
    }
    pthread_cleanup_pop(1);
    pthread_exit(0);

}

void* task2(void *arg)
{
    static const unsigned char schedule_time = 5;
    int signo, err;
    /*
    We set a local mask with SIGALARM for the function sigwait
    All signals have already been blocked
    */
    sigset_t alarm_mask;
    sigemptyset(&alarm_mask);
    sigaddset(&alarm_mask, SIGALRM);
    alarm(schedule_time);
    pthread_cleanup_push(exitingThreadTask2, NULL);
    while (1)
    {
        pthread_mutex_lock(&g_shutdown_mutex);
        if(g_shutdown_task_1_2) {
          pthread_mutex_unlock(&g_shutdown_mutex);
          break;
        }
        pthread_mutex_unlock(&g_shutdown_mutex);
        
        err = sigwait(&alarm_mask, &signo); //signo == SIGALRM check
        if (err!=0)
            err_exit(err, "sigwait failed\n");
        
        pthread_mutex_lock(&g_shutdown_mutex);
        if(g_shutdown_task_1_2) {
          pthread_mutex_unlock(&g_shutdown_mutex);
          break;
        }
        pthread_mutex_unlock(&g_shutdown_mutex);
        
        //do stuff
        alarm(schedule_time);
    }
    pthread_cleanup_pop(1);
    pthread_exit(0);
}

void* task3(void *arg)
{
    pthread_cleanup_push(exitingThreadTask3, NULL);
    while(1)
    {
        pthread_mutex_lock(&g_new_pic_m);
        if(g_shutdown_task_3) {
          pthread_mutex_unlock(&g_new_pic_m);
          break;
        }
        while(g_new_pic_flag==FALSE)
        {
            if(g_shutdown_task_3) break;
                        
            pthread_cond_wait(&g_new_pic_cond, &g_new_pic_m);
            
            if(g_shutdown_task_3) break;
        }
        if(g_shutdown_task_3) {
          pthread_mutex_unlock(&g_new_pic_m);
          break;
        }
        pthread_mutex_unlock(&g_new_pic_m);
        //do stuff
    }
    pthread_cleanup_pop(1);
    pthread_exit(0);

}

void exitingThreadTask1(void* arg)
{
    printf("Thread of task 1 exiting\n");
    //digitalWrite(OUTPIN, LOW);
    //digitalWrite(INPIN, LOW);
    printf("Pins lowered\n");
}

void exitingThreadTask2(void* arg)
{
    printf("Thread of task 2 exiting\n");
    //digitalWrite(DHTPIN, LOW);
    printf("Pin lowered\n");
}

void exitingThreadTask3(void* arg)
{
    printf("Thread of task 3 exiting\n");
}

void sig_handler(int signo)
{
    return;
}

Answer 2

In general, I recommend not cancelling or killing threads. I also try to minimize signal handling in threaded applications, or at least make the signal handlers very short, nonblocking and simple. It is better to have the threads run a loop, where they e.g. check a cancel flag, or if your thread does I/O with select or epoll, have the master thread write to a pipe to signal the other end to die. With C++ and pthreads, cancelling or killing can be even more disastrous, so for C++, doing a clean shutdown with custom code is even more important.

See e.g. pthread cancel and C++