C Pthreads numbers

Question

i'm new with C pthreads, and i'm having some little problems on it. I would like to make a program that stops when my var result = 13. I have two threads that add or subtract a randomic number to my result. The problem is that, in the compilated case, the subtract threads never works. Result, Step and Mutex are my global variables. Thanks to everyone

void add(){
int random;
while(result < 101){
    random = rand() % 51;
    pthread_mutex_lock(&mutex);
    result += random;
    step += 1;
    pthread_mutex_unlock(&mutex);
    printf("%d. [random -> %d, result now -> %d] ADD\n",step,random,result);

}
pthread_exit(0);


void sub(){
int random;
while(result > 5){
    random = rand() % 51;
    pthread_mutex_lock(&mutex);
    result -= random;
    step +=1;
    pthread_mutex_unlock(&mutex);
    printf("%d. [random -> %d, result now -> %d] SUB\n",step,random,result);

}
pthread_exit(0);

void main(int argc, char**argv){

pthread_t th1;
pthread_t th2;

pthread_mutex_init(&mutex,NULL);

if(pthread_create(&th1, NULL, add, NULL)!= 0)
    printf("ERROR: Thread 1 not created");

if(pthread_create(&th2, NULL, sub,NULL)!=0)
    printf("ERROR: Thread 2 not created");

while(result != 13){

}
pthread_cancel(th1);
pthread_cancel(th2);

pthread_exit(0);

Answer 1

Apart from some "smaller" issues like the randomly placed pthread_exit() statements, the wrong signature of your thread functions (they must be void* f(void*) , the bad choice for your while loops within add() and sub() and a few other things, your code looks good.

You are probably aware, that your code is not guaranteed to ever terminate as it gambles that main() ever happens to probe result for the value 13 when it has that value. Odds are, though that it will terminate (some day).

When I ran it, the program usually terminates with stepAdd,stepSub in the range of 2000. Running it with a 4 core freebsd VM.

You are under the impression that your sub- thread never runs because your printf statements create that impression. If time slicing is 100ms you get a ton of printf output and maybe even the buffer of your console or whatever you use gets filled.

My advice on using POSIX threads is to spend the small amount of extra time to make sure, the default values do not hold some unexpected surprises for you in stock.

For example, pthread_mutex_init() by default creates non-recursive mutexes. That means if you (accidently or purposely) write lock() lock(), the thread deadlocks itself. While it could be argued, that this is a "feature", I am inclined to say that it produces problems in larger systems, once the function call trees get a bit larger.

Another example could be the default stack size for the threads. It is quite small on some systems.

In my example I made sure, the scheduler mode is SCHED_RR (it is the default on most systems, I would think, but I saw systems which had SCHED_FIFO as default, too).

Moved the printf to main() to be able to see the step- values of both thread functions.

Added some somewhat neater and more realistic shutdown code.

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

typedef void*(*ThreadFunc)(void*);

static volatile int running = 1;
static volatile int result = 0;
static pthread_mutex_t mtx1;
static volatile int stepAdd = 0;
static volatile int stepSub = 0;

void* add( void *)
{
    int random;

    while( running )
    {
        if( result < 101 )
        {
            pthread_mutex_lock(&mtx1);
            random = rand() % 51;
            result += random;
            stepAdd += 1;
            pthread_mutex_unlock(&mtx1);
            //printf("%d. [random -> %d, result now -> %d] ADD\n",step,random,result);
        }
        // else - one could avoid some useless spinning of the thread. (pthread_yield())
    }
    pthread_exit(NULL);
}

void* sub( void *)
{
    int random;
    while( running )
    {
        if( result > 5 )
        {
            pthread_mutex_lock(&mtx1);
            random = rand() % 51;
            result -= random;
            stepSub += 1;
            pthread_mutex_unlock(&mtx1);
//          printf("%d. [random -> %d, result now -> %d] ADD\n",step,random,result);
        }
    }
    pthread_exit(NULL);
}



static int RunThread(pthread_t *handle, ThreadFunc f, void* context )
{
    int result;
    pthread_attr_t thread_attribute;
    pthread_attr_init(&thread_attribute);
    pthread_attr_setschedpolicy(&thread_attribute,SCHED_RR);
    result = pthread_create(handle,&thread_attribute, f, context );
    pthread_attr_destroy(&thread_attribute);
    return result;
}

int main(int argc, const char * argv[])
{
    void *addResult = NULL;
    void *subResult = NULL;
    pthread_t addThread = 0;
    pthread_t subThread = 0;
    // Default mutex behavior sucks, btw.
    // It can auto-deadlock as it is not recursive.
    pthread_mutex_init(&mtx1,NULL); 
    RunThread(&addThread, add, NULL );
    RunThread(&subThread, sub, NULL );

    while( running )
    {
        if( 13 == result )
        {
            running = 0;
        }
        else
        {
            printf("Steps: add(%d) sub(%d) -- result = %d\n", stepAdd, stepSub, result );
        }
    }

    pthread_join( addThread, &addResult );
    pthread_join( subThread, &subResult );

    puts( "All done - bye." );

    return 0;
}
// compiled with:
// clang++ -std=c++11 -stdlib=libc++ -lpthread baby_threading.cpp
// but should compile as C as well.

Answer 2

There are multiple things wrong here:

---

rand is not thread safe, therefore it should be moved into the critical section protected by the mutex.

---

Your threads should do a while(true) combined with a pthread_yield/sched_yield instead of the while(result > 5)and while(result < 101):

while (true) {
    if (result >= 101) {
        pthread_yield();
        continue;
    }
    ...

otherwise it is possible that your threads stops early. E.g. imagine result is 100 and the add function adds 42 to it, the next iteration this thread would be stopped.

---

In general pthread_cancel is used in connection with pthread_join.

With the solution discussed above it might take a while for the pthread_cancel to finish since printf is not required to be a cancellation point and threads can only be cancelled on such cancellation points. See also here (Cancellation points and ).

So what you might do is to add a boolean parameter stop_thread and if true stop the while loop and therefore exit the loop. Now if result is 13 set stop_thread in main to true and pthread_join on the threads. After the join result might be changed though. If result should not be changed then you should check for 13 in the threads themselves.

---

Your function signatures are wrong. pthread_create expects void *(*start_routine)(void*). So change your functions to

void *add(void*) {
    ...
    return NULL; /* no pthread_exit needed */
}

With such a signature it would even be possible to hand in the parameters to the threads instead of relying on global variables.

---

No need to call pthread_exit in main.

---

Even with all these changes it is unlikely that the program will ever finish. main would have to be scheduled exactly when result is 13 which is very unlikely since there is no cancellation point, no yield etc. in case of 13 in the threads. So what you should most likely do is check in the threads themselves if result == 13 and stop them. You have to ensure that no other thread that is waiting on the mutex will modify result though.

You should also add a pthread_yield in the loop in main to ensure that the other threads will be run.

Answer 3

You have lock starvation issue. The time threads perform operation outside critical section is too short, and the probability of scheduler attempt to reschedule thread while the another one owns the lock is too high.

You can change operation by:

• Increasing the time outside the critical section

  This case assumes you can find some task for thread, which is long enough to keep it busy for a time quantum. It is a waste of CPU resources though.

• Forcing reschedule with pthread_yield()

  This would force scheduler to switch between threads in more or less RR manner after they leave critical section. This might or might not suite your needs, as in real application threads usually behave less deterministic.

• Using some random delays with sleep() or equivalent

  It is a combined approach, so that threads have some random delays outside critical section, that simulate some work amount, but without loading CPU. Again, this method has some limitations, as if in a real life system you have many threads and CPU doesn't have available capacity, the system may behave differently.