Using a shared variable at 10 pthreads

Question

The problem is this:

I want to write a short program that creates 10 threads, and each prints the "id" of the tread, which is passed to the stream function with a pointer.

The full program code is given below:

#include <pthread.h> #include <stdio.h> #include <stdlib.h> struct params { pthread_mutex_t mutex; int id; }; typedef struct params params_t; void* hello(void* arg){ int id; pthread_mutex_lock(&(*(params_t*)(arg)).mutex); id = (*(params_t*)(arg)).id; pthread_mutex_unlock(&(*(params_t*)(arg)).mutex); printf("Hello from %d\n", id); } int main() { pthread_t threads[10]; params_t params; pthread_mutex_init (&params.mutex , NULL); int i; for(i = 0; i < 10; i++) { params.id = i; if(pthread_create(&threads[i], NULL, hello, &params)); } for(i = 0; i < 10; i++) { pthread_join(threads[i], NULL); } return 0; } 

Estimated output (not necessarily in this order):

 Hello from 0 .... Hello from 9 

Actual result:

 Hello from 2 Hello from 3 Hello from 3 Hello from 4 Hello from 5 Hello from 6 Hello from 8 Hello from 9 Hello from 9 Hello from 9 

I tried to place the mutex in different places in the hello() function, but that did not help.

How do I synchronize threads?

EDIT: The intended result is not required 0 ... 9, it can be any combination of these numbers, but each of them should appear only once.

Answer 1

There are two problems:

a. You use lock , but main does not know about this lock.

B. A lock in this case is not enough. You want the threads to interact, signaling each other (because you want main not increment this variable until the thread tells you that it was printed). You can use pthread_cond_t to achieve this ( See here for more on this). This boils down to the following code (basically, I added the appropriate use of pthread_cond_t to your code and a bunch of comments explaining what happens):

 #include <pthread.h> #include <stdio.h> #include <stdlib.h> struct params { pthread_mutex_t mutex; pthread_cond_t done; int id; }; typedef struct params params_t; void* hello(void* arg){ int id; /* Lock. */ pthread_mutex_lock(&(*(params_t*)(arg)).mutex); /* Work. */ id = (*(params_t*)(arg)).id; printf("Hello from %d\n", id); /* Unlock and signal completion. */ pthread_mutex_unlock(&(*(params_t*)(arg)).mutex); pthread_cond_signal (&(*(params_t*)(arg)).done); /* After signalling `main`, the thread could actually go on to do more work in parallel. */ } int main() { pthread_t threads[10]; params_t params; pthread_mutex_init (&params.mutex , NULL); pthread_cond_init (&params.done, NULL); /* Obtain a lock on the parameter. */ pthread_mutex_lock (&params.mutex); int i; for(i = 0; i < 10; i++) { /* Change the parameter (I own it). */ params.id = i; /* Spawn a thread. */ pthread_create(&threads[i], NULL, hello, &params); /* Give up the lock, wait till thread is 'done', then reacquire the lock. */ pthread_cond_wait (&params.done, &params.mutex); } for(i = 0; i < 10; i++) { pthread_join(threads[i], NULL); } /* Destroy all synchronization primitives. */ pthread_mutex_destroy (&params.mutex); pthread_cond_destroy (&params.done); return 0; } 

I see that the example you're trying is a toy program that will probably learn about the POSIX thread library. In the real world, as we all know, this can be done much faster without even using threads. But you already know that.

Answer 2

The problem is the following code:

 for(i = 0; i < 10; i++) { params.id = i; if(pthread_create(&threads[i], NULL, hello, &params)); } 

Your params.id value is constantly updated in the main thread, while you are passing the same pointer to all threads.

Create a separate memory for the parameters, dynamically allocating it and passing it to different threads to solve the problem.

EDIT1: Using a mutex for protection is also a bad idea. Although your mutex, if it is used mainly when setting the identifier, can also make the update mutually exclusive, but you cannot get the desired result. Instead of getting values ​​from 0 .. 9 in different threads, you can get all 9 or more threads that can print the same values.

Thus, using thread synchronization is not such a good idea for the output you expect. If you still need to use one parameter variable between all the threads and get an output from 0 to 9 from each of the threads, it is better to move pthread_join to the first loop. This ensures that each thread is created, prints the value, and then returns before the main one spawns the next thread. In this case, you also do not need a mutex.

EDIT2: Regarding the updated question, which asks that there is no need to print the numbers 0..9 in sequence, printing may be random, but only once the problem remains the same or less.

Now let's say that the value of params.id is the first 0 and stream 0 is created, now stream 0 must print it before it is updated in the main stream, otherwise, when stream 0 receives it, the value of params.id would become 1, and you will never get your unique set of values. So, how to ensure that thread 0 prints it before it is basically updated, Two ways for it:

• Make sure thread 0 completes execution and print before major updates value
• Use condition variables and alarms to ensure that the main thread waits for thread 0 to complete printing before it updates the value (see Arjun's answer below for more details)

In my honest opinion, you have chosen the wrong problem for learning synchronization and shared memory. You can try this with some good issues, such as Producer-Consumer, where you really need synchronization to work.

Answer 3

The problem is that you are changing params.id "unprotected" basically. This modification should also mainly be protected by the mutex. You can protect this access by localizing it by creating getId () and setId () functions that would block the mutex and protect access to id, as follows. This is likely to give a problem anyway, because depending on when the thread calls getData (), it will have one value. To solve this problem, you can add the incrementId () function and call it from the hello () function.

 struct params { pthread_mutex_t mutex; int id; }; typedef struct params params_t; int getId(params_t *p) { int id; pthread_mutex_lock(&(p->mutex)); id = p->id; pthread_mutex_unlock(&(p->mutex)); return id; } void setId(params_t *p, int val) { pthread_mutex_lock(&(p->mutex)); p->id = val; pthread_mutex_unlock(&(p->mutex)); } void incrementId(params_t *p) {  pthread_mutex_lock(&(p->mutex));  p->id++;  pthread_mutex_unlock(&(p->mutex)); } void* hello(void* arg){ params_t *p = (params_t*)(arg); incrementId(p); int id = getId(p); // This could possibly be quite messy since it // could print the data for multiple threads at once printf("Hello from %d\n", id); } int main() { pthread_t threads[10]; params_t params; params.id = 0; pthread_mutex_init (&params.mutex , NULL); int i; for(i = 0; i < 10; i++) { if(pthread_create(&threads[i], NULL, hello, &params)); } for(i = 0; i < 10; i++) { pthread_join(threads[i], NULL); } return 0; } 

The best way to get a unique thread identifier is to define a hello method as follows:

 void* hello(void* arg){ pthread_t threadId = pthread_self(); printf("Hello from %d\n", threadId); } 

And to avoid problems with all printing attempts at once, you can do the following:

 void* hello(void* arg){ params_t *p = (params_t*)(arg);  pthread_mutex_lock(&(p->mutex)); p->id++; int id = p->id; printf("Hello from %d\n", id);  pthread_mutex_unlock(&(p->mutex)); } 

Answer 4

The easiest way to get the desired result is to change your main function as follows:

 int main() { pthread_t threads[10]; params_t params; pthread_mutex_init (&params.mutex , NULL); int i; for(i = 0; i < 10; i++) { params.id = i; if(pthread_create(&threads[i], NULL, hello, &params)); pthread_join(threads[i], NULL); //wait for thread to finish } /*for(i = 0; i < 10; i++) { pthread_join(threads[i], NULL); }*/ return 0; } 

Conclusion:

 Hello from 0 ... Hello from 9 

EDIT: Here's the sync for the fixed question:

 #include <pthread.h> #include <stdio.h> #include <stdlib.h> struct params { pthread_mutex_t* mutex; int id; }; typedef struct params params_t; void* hello(void* arg){ int id = 0; params_t* params = (params_t*)arg; if(params != 0) { id = params->id; delete params; params = 0; } printf("Hello from %d\n", id); } int main() { pthread_t threads[10]; params_t* params = 0; pthread_mutex_t main_mutex; pthread_mutex_init (&main_mutex , NULL); int i; for(i = 0; i < 10; i++) { params = new params_t(); //create copy of the id to pass to each thread -> each thread will have it own copy of the id params->id = i; params->mutex = &main_mutex; if(pthread_create(&threads[i], NULL, hello, params)); } for(i = 0; i < 10; i++) { pthread_join(threads[i], NULL); } return 0; } 

Each thread must have its own copy of the identifier, so that other threads do not change the identifier before printing it. A.

Answer 5

I just put it here to provide another solution to this problem - it is not related to mutexes - no synchronization - no conventions, etc. The main problem is that we use pthread_detach to automatically free thread resources after completion.

 #include <pthread.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> #define NUMTHREADS 10 typedef struct params { int id; } params_t; void* hello(void *arg) { params_t *p = (params_t*)arg; int status; status = pthread_detach(pthread_self()); if (status !=0 ) { printf("detaching thread\n"); abort(); } printf("Hello from %d\n", p->id); free(p); return NULL; } int main() { pthread_t thread; params_t *par; int i, status; for (i=0; i<NUMTHREADS; i++) { par = (params_t*)malloc(sizeof(params_t)); if (par == NULL) { printf("allocating params_t"); abort(); } par->id = i; status = pthread_create(&thread, NULL, hello, par); if (status != 0) exit(1); } /* DO some more work ...*/ sleep(3); exit(0); }