11

I have to code a multithreaded(say 2 threads) program where each of these threads do a different task. Also, these threads must keep running infinitely in the background once started. Here is what I have done. Can somebody please give me some feedback if the method is good and if you see some problems. Also, I would like to know how to shut the threads in a systematic way once I terminate the execution say with Ctrl+C.

The main function creates two threads and let them run infinitely as below.

Here is the skeleton:

void    *func1();
void    *func2();

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

    pthread_t th1,th2;
    pthread_create(&th1, NULL, func1, NULL);
    pthread_create(&th2, NULL, func2, NULL);

    fflush (stdout);
    for(;;){
    }
    exit(0); //never reached
}

void *func1()
{
    while(1){
    //do something
    }
}

void *func2()
{
    while(1){
    //do something
    }
} 

Thanks.

Edited code using inputs from the answers: Am I exiting the threads properly?

#include <stdlib.h>     /*  exit() */
#include <stdio.h>      /* standard in and output*/
#include <pthread.h>
#include <unistd.h>
#include <time.h>
#include <sys/time.h>
#include <sys/types.h>
#include <signal.h>
#include <semaphore.h>

sem_t end;

void    *func1();
void    *func2();

void ThreadTermHandler(int signo){
    if (signo == SIGINT) {
        printf("Ctrl+C detected !!! \n");
        sem_post(&end);
        }
}
void *func1()
{
    int value;
    for(;;){
        sem_getvalue(&end, &value);
        while(!value){
            printf("in thread 1 \n");
        }
    }
    return 0;
}

void *func2()
{
    int value;
    for(;;){
        sem_getvalue(&end, &value);
        while(!value){
            printf("value = %d\n", value);
        }
    }
    return 0;
}

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


    sem_init(&end, 0, 0);
    pthread_t th1,th2;
    int value  = -2;
    pthread_create(&th1, NULL, func1, NULL);
    pthread_create(&th2, NULL, func2, NULL);

    struct sigaction sa;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = SA_SIGINFO;
    sa.sa_sigaction = ThreadTermHandler;
    // Establish a handler to catch CTRL+c and use it for exiting.
    if (sigaction(SIGINT, &sa, NULL) == -1) {
        perror("sigaction for Thread Termination failed");
        exit( EXIT_FAILURE );
    }

    /* Wait for SIGINT. */
    while (sem_wait(&end)!=0){}
    //{
        printf("Terminating Threads.. \n");
        sem_post(&end);
                sem_getvalue(&end, &value);
        /* SIGINT received, cancel threads. */
        pthread_cancel(th1);
        pthread_cancel(th2);
        /* Join threads. */
        pthread_join(th1, NULL);
        pthread_join(th2, NULL);
    //}
    exit(0);
}
user489152
  • 907
  • 1
  • 23
  • 42
  • 1
    here is a nice tutorial to get the basics: https://computing.llnl.gov/tutorials/pthreads/ – Karoly Horvath Jul 08 '11 at 08:23
  • Your function looks good. Replace for(;;){ } by a scanf function to read character in console, you may check if character = 'q' then broadcast a signal so that those two threads could stop properly. Check ways to stop threads here http://stackoverflow.com/questions/2084830/kill-thread-in-pthread – longbkit Jul 08 '11 at 08:26
  • Hi All, can anybody please take a look at the edited code and let me know if it right. Thanks a lot – user489152 Jul 20 '11 at 13:20

7 Answers7

14

There are mainly two approaches for thread termination.

  • Use a cancellation point. The thread will terminate when requested to cancel and it reaches a cancellation point, thus ending execution in a controlled fashion;
  • Use a signal. Have the threads install a signal handler which provides a mechanism for termination (setting a flag and reacting to EINTR).

Both approaches has caveats. Refer to Kill Thread in Pthread Library for more details.

In your case, it seems a good opportunity to use cancellation points. I will work with a commented example. The error-checking has been omitted for clarity.

#define _POSIX_C_SOURCE 200809L
#include <pthread.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

void sigint(int signo) {
    (void)signo;
}

void *thread(void *argument) {
    (void)argument;
    for (;;) {
        // Do something useful.
        printf("Thread %u running.\n", *(unsigned int*)argument);

        // sleep() is a cancellation point in this example.
        sleep(1);
    }
    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 = sigint;
    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;
}

The need for blocking/unblocking signals is that if you send SIGINT to the process, any thread may be able to catch it. You do so before spawning the threads to avoid having them doing it by themselves and needing to synchronize with the parent. After the threads are created, you restore the mask and install a handler.

Cancellation points can be tricky if the threads allocates a lot of resources; in that case, you will have to use pthread_cleanup_push() and pthread_cleanup_pop(), which are a mess. But the approach is feasible and rather elegant if used properly.

Community
  • 1
  • 1
alecov
  • 4,882
  • 2
  • 29
  • 55
  • Thanks Alek. I did read up on cancellation point. Well, I am spawning another thread which is a socket server. I guess I have to use either accept() select() or recv() as cancellation points. – user489152 Aug 10 '11 at 09:54
  • @user489152: All three of them are cancellation points (you don't _choose_ a function to perform that role; instead, a number of them are _defined_ to do so, if you happen to call them). One point worth noticing is that you might need to `pthread_cleanup_push()` a handler for `close()`ing the accepted socket just after `accept()` returns, to guarantee that the file-descriptor is properly closed in the event of a cancellation in `read()` or `select()`. – alecov Aug 10 '11 at 16:19
  • suppose a thread (not the main one) uses the signal SIGALRM with sigwait while it is blocked for the other thread including the main one: does the occurring of SIGALRM wake up the main thread from pause even if it is blocked on its mask? – roschach Feb 03 '18 at 11:22
7

The answer depends a lot on what you want to do when the user presses CtrlC.

If your worker threads are not modifying data that needs to be saved on exit, you don't need to do anything. The default action of SIGINT is to terminate the process, and that includes all threads that make up the process.

If your threads do need to perform cleanup, however, you've got some work to do. There are two separate issues you need to consider:

  • How you handle the signal and get the message to threads that they need to terminate.
  • How your threads receive and handle the request to terminate.

First of all, signal handlers are a pain. Unless you're very careful, you have to assume most library functions are not legal to call from a signal handler. Fortunately, sem_post is specified to be async-signal-safe, and can meet your requirements perfectly:

  1. At the beginning of your program, initialize a semaphore with sem_init(&exit_sem, 0, 0);
  2. Install a signal handler for SIGINT (and any other termination signals you want to handle, like SIGTERM) that performs sem_post(&exit_sem); and returns.
  3. Replace the for(;;); in the main thread with while (sem_wait(&exit_sem)!=0).
  4. After sem_wait succeeds, the main thread should inform all other threads that they should exit, then wait for them all to exit.

The above can also be accomplished without semaphores using signal masks and sigwaitinfo, but I prefer the semaphore approach because it doesn't require you to learn lots of complicated signal semantics.

Now, there are several ways you could handle informing the worker threads that it's time to quit. Some options I see:

  1. Having them check sem_getvalue(&exit_sem) periodically and cleanup and exit if it returns a nonzero value. Note however that this will not work if the thread is blocked indefinitely, for example in a call to read or write.
  2. Use pthread_cancel, and carefully place cancellation handlers (pthread_cleanup_push) all over the place.
  3. Use pthread_cancel, but also use pthread_setcancelstate to disable cancellation during most of your code, and only re-enable it when you're going to perform blocking IO operations. This way you need only put the cleanup handlers just in the places where cancellation is enabled.
  4. Learn advanced signal semantics, and setup an additional signal and interrupting signal handler which you send to all threads via pthread_kill which will cause blocking syscalls to return with an EINTR error. Then your threads can act on this and exit the normal C way via a string of failure returns all the way back up the the start function.

I would not recommend approach 4 for beginners, because it's hard to get right, but for advanced C programmers it may be the best because it allows you to use the existing C idiom of reporting exceptional conditions via return values rather than "exceptions".

Also note that with pthread_cancel, you will need to periodically call pthread_testcancel if you are not calling any other functions which are cancellation points. Otherwise the cancellation request will never be acted upon.

R.. GitHub STOP HELPING ICE
  • 208,859
  • 35
  • 376
  • 711
  • @R: thanks for all your input. I think I will have to do some homework- some code tweaks/tests and then will share the output with you all. Oh btw, my two threads clear away data(free memory etc). But since they are socket servers, immediately after I stop execution and restart, the socket complains that it is already bound. Therefore I thought I must really "kill/cancel" all threads when I stop execution of program – user489152 Jul 08 '11 at 14:28
  • No, you just need to use the `SO_REUSEADDR` option. By the way, you should have asked your question about the "already bound" issue to begin with... – R.. GitHub STOP HELPING ICE Jul 08 '11 at 15:20
  • @R.. You really explored some possibilities here. – cnicutar Jul 08 '11 at 15:36
  • @R & cnicutar: Could you please take a look at my edited code – user489152 Jul 12 '11 at 08:41
  • Remove the `exit(0);` calls from your thread functions. `exit` terminates the whole program not the thread. You should merely use `return 0;` there. – R.. GitHub STOP HELPING ICE Jul 12 '11 at 15:48
  • And same with the signal handler. – R.. GitHub STOP HELPING ICE Jul 12 '11 at 15:48
  • And since you're relying on checking `sem_getvalue` in the threads, (1) you need to check the result stored in `value`, not the return value of `sem_getvalue`,and (2) the main thread needs to `sem_post` after calling `sem_wait` because `sem_wait` decrements the semaphore back to 0. Finally, mixing cancellation and polling of exit flag like this seems like a bad idea...and if you're going to use cancellation in real code you'll need cancellation handlers. – R.. GitHub STOP HELPING ICE Jul 12 '11 at 16:09
  • And why do I get "warning: assignment from incompatible pointer type" in line: sa.sa_sigaction = ThreadTermHandler; ?? any idea? – user489152 Jul 14 '11 at 09:50
  • I did not get point (3). Do explain. I have altered my code. Please feel free to edit it if something is wrong – user489152 Jul 14 '11 at 11:28
  • Nah, not very practical. You don't want to use a semaphore since it does not integrate with event loops. Same for sending signals to other threads. – Maxim Egorushkin Aug 05 '11 at 09:58
  • Signals work very well with event loops. `pselect` waits for file activity *or* a signal. – R.. GitHub STOP HELPING ICE Aug 05 '11 at 11:01
  • My opinion is this is rather overcomplicated. – alecov Aug 05 '11 at 16:26
  • I agree. Ideally the first 2 paragraphs of my answer should be all that's needed. If you have "permanent" threads that need to perform nontrivial cleanup before the program terminates, you have a complicated problem to solve. – R.. GitHub STOP HELPING ICE Aug 05 '11 at 16:34
3

This is a bad idea:

for(;;){
}

because your main thread will execute unnecessary CPU instructions.

If you need to wait in the main thread, use pthread_join as answered in this question: Multiple threads in C program

Community
  • 1
  • 1
SirDarius
  • 41,440
  • 8
  • 86
  • 100
3

What you have done works, I see no obvious problems with it (except that you are ignoring the return value of pthread_create). Unfortunately, stopping threads is more involved than you might think. The fact that you want to use signals is another complication. Here's what you could do.

  • In the "children" threads, use pthread_sigmask to block signals
  • In the main thread, use sigsuspend to wait for a signal
  • Once you receive the signal, cancel (pthread_cancel) the children threads

Your main thread could look something like this:

/* Wait for SIGINT. */
sigsuspend(&mask);

/* SIGINT received, cancel threads. */
pthread_cancel(th1);
pthread_cancel(th2);

/* Join threads. */
pthread_join(th1, NULL);
pthread_join(th2, NULL);

Obviously, you should read more about pthread_cancel and cancellation points. You could also install a cleanup handler. And of course, check every return value.

cnicutar
  • 178,505
  • 25
  • 365
  • 392
  • Are signals the best way to stop thread?? I mean what if I don#t do the "signal" way? – user489152 Jul 08 '11 at 09:11
  • @user489152 If you look closer at my answer, you'l see signals are only used to notify the main thread. The secondary threads are stopped via `pthread_cancel`. I merely noted that signals are a complication, they introduce lots of (sometimes subtle) issues. – cnicutar Jul 08 '11 at 09:13
  • +1 this is the only correct answer I've seen so far, even if it's not complete. – R.. GitHub STOP HELPING ICE Jul 08 '11 at 14:14
  • 1
    In fact, you want to block the signals before creating any threads. Otherwise there is a race condition window between when the thread has started and blocked the signal. – Maxim Egorushkin Aug 05 '11 at 09:59
1

Wouldn't it be much easier to just call pthread_cancel and use pthread_cleanup_push in the thread function to potentially clean up the data that was dynamically allocated by the thread or do any termination tasks that was required before the thread stops.

So the idea would be:

  1. write the code to handle signals
  2. when you do ctrl+c ... the handling function is called
  3. this function cancels the thread
  4. each thread which was created set a thread cleanup function using pthread_cleanup_push
  5. when the tread is cancelled the pthread_cleanup_push's function is called
  6. join all threads before exiting

It seems like a simple and natural solution.

static void cleanup_handler(void *arg)
{
    printf("Called clean-up handler\n");
}

static void *threadFunc(void *data)
{
    ThreadData *td = (ThreadData*)(data);
    pthread_cleanup_push(cleanup_handler, (void*)something);
    while (1) {
    pthread_testcancel(); /* A cancellation point */
    ...
    }
    pthread_cleanup_pop(cleanup_pop_arg);
    return NULL;
}

mast4as
  • 11
  • 2
1

Looked at your updated coded and it still does not look right.

Signal handling must be done in only one thread. Signals targeted for a process (such as SIGINT) get delivered to any thread that does not have that signal blocked. In other words, there is no guarantee that given the three threads you have it is going to be the main thread that receives SIGINT. In multi-threaded programs the best practise is too block all signals before creating any threads, and once all threads have been created unblock the signals in the main thread only (normally it is the main thread that is in the best position to handle signals). See Signal Concepts and Signalling in a Multi-Threaded Process for more.

pthread_cancel is best avoided, there no reason to ever use it. To stop the threads you should somehow communicate to them that they should terminate and wait till they have terminated voluntarily. Normally, the threads will have some sort of event loop, so it should be relatively straightforward to send the other thread an event.

Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
  • Thanks Maxim. I will look into them. The thing is my threads run in an infinite loop and only when I terminate them(which normally I wouldn't as these are purported to be daemon threads) with a CTrl+C should all of them exit. – user489152 Aug 09 '11 at 08:55
0

You don't need the foor loop in the main. A th1->join(); th2->join(); will suffice as a wait condition since the threads never end.

To stop the threads you could use a global shared var like bool stop = false;, then when catching the signal (Ctrl+Z is a signal in UNIX), set stop = true aborting the threads, since you are waiting with join() the main program will also exit.

example

void *func1(){
  while(!stop){
    //do something
  }
}
RedX
  • 14,749
  • 1
  • 53
  • 76
  • This is invalid. You must hold a mutex while checking the value of `stop`. – R.. GitHub STOP HELPING ICE Jul 08 '11 at 13:47
  • Strictly speaking you are right. But since you are writing only in one thread (the main) and only reading from the other two threads, this works. You could prepend it with volatile to signal to the compiler that the variable is modified from outside the scope and should not be optimized away and reread from every memory on each access. – RedX Jul 08 '11 at 14:08
  • No, the compiler is perfectly justified in replacing the `while(!stop)` with `if(!stop) while(1)` unless the while loop contains code which the compiler cannot prove will not modify `stop`. – R.. GitHub STOP HELPING ICE Jul 08 '11 at 14:12
  • I thought that volatile was meant to be used exactly for these circumstances. Where it looks like the variable won't be modified within the loop, but asynchronously from another place that the compiler (might) not be able to infer. – RedX Jul 08 '11 at 15:12