1

I want to create three threads and pass an integer to each of these thread. I used pthread_create, and my code is as follows.

#include<pthread.h>
#include<unistd.h>
#include<stdio.h>
#define NUM_OF_JOB 3

void* doJob(void* arg){
    int i = *((int*)arg);
    printf("%d\n",i);
}

void init_job() {
    pthread_t thread[NUM_OF_JOB];
    int index[NUM_OF_JOB]={-1,-1,-1};
    for(int i=0;i<NUM_OF_JOB;i++){
    pthread_create(&thread[i],NULL,doJob,(void*)(&index[i]));
    }
}

int main(){
    init_job();
    sleep(1);
    return 0;
}

I intended to pass -1 to each of the thread, but after running the code for server times, I found that it did not print three -1. Instead, they could produce strange outputs. So why is that?

similar code can be found in pass arguments to the pthread_create function I can't find anything different between his code and mine. But in his code the threads could get the expected argument.

thank you very much:)

btw, I try to change the code from the above link a little, and run it.surprisingly, my mordified version don't produce expected results either.(I think each thread should get their own unique integer as argument) Here is my mordified code. can anyone explain that?

#include <stdio.h>
#include <pthread.h>

#define THREAD_NUM 10

void *thread_func(void *arg)
{
    int v = *(int*)arg;

    printf("v = %d\n", v);

    return (void*)0;
}

int main(int argc, const char *argv[])
{
    pthread_t pids[THREAD_NUM];
    int rv;
    int i;

    for (i = 0; i < THREAD_NUM; i++) {
        rv = pthread_create(&pids[i], NULL, thread_func, (void*)(&i));
        if (rv != 0) {
           perror("failed to create child thread");
           return 1;
        }
    }
    for (i = 0; i < THREAD_NUM; i++) {
        pthread_join(pids[i], NULL);
    }
    return 0; 
}

thank you again:)

Community
  • 1
  • 1
Casualet
  • 295
  • 3
  • 12

4 Answers4

5

In the first example, the main thread returns from the function doJob, and the object index stops existing. Then the threads try to print the values of the array index. Since the object index, doesn't exist anymore, the behavior is undefined.

The first example would be correct if you joined with the threads before returning from the function.

The second example is also undefined because there is a data race. The variable i is being read by multiple threads and modified by the main thread, without any synchronization.

To fix the second example, use an array of integers, and pass each element separately to created threads.

2501
  • 25,460
  • 4
  • 47
  • 87
  • The second problem is different from the first, the variable `i` will not go out of scope. – Some programmer dude Aug 24 '16 at 08:12
  • @JoachimPileborg Thanks. – 2501 Aug 24 '16 at 08:14
  • Thank you,but in the original link,the code was like: `pthread_create(&pids[i], NULL, thread_func, (void*)(i));` and in **thread_func**, it used int **v = (int)arg**, and the result was right. Why wasn't there a data race. Also, that code compiles using gcc, but with g++, casting from void* to int is not allowed, so how can I intergrate that piece of code with my cpp code? :) – Casualet Aug 24 '16 at 08:48
  • @Casualet I don't know, i haven't seen that code. Post a new question. https://stackoverflow.com/questions/ask – 2501 Aug 24 '16 at 09:01
3

The second problem is because all threads get the exact same pointer. The problem with this is that you have no control of when a threads start to run and copy the value. If the first thread doesn't start immediately and the loop creating the threads continue it will increase the variable, and once the threads starts it will see the wrong value.

The first example have another problem, but is also related to the problem described above. The problem is that you pass pointers to something that will go out of scope. Once the init_job returns the array index will go out of scope and exist no more, and if no threads have started running yet then the pointers they will dereference will not point to what you think they are pointing.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1

As previous comments mentioned, the problem is your index array is out of scope for the thread context. I'd like to offer you the following solution:

#include<pthread.h>
#include<unistd.h>
#include<stdio.h>
#define NUM_OF_JOB 3

int index[NUM_OF_JOB]={-1,-1,-1};

void* doJob(void* arg){
    int i = *((int*)arg);
    printf("%d\n",i);
}

void init_job() {
    pthread_t thread[NUM_OF_JOB];
    for(int i=0;i<NUM_OF_JOB;i++){
        pthread_create(&thread[i],NULL,doJob,&index[i]);
    }
}

int main(){
    init_job();
    sleep(1);
    return 0;
}
Ishay Peled
  • 2,783
  • 1
  • 23
  • 37
  • the call to `sleep()` is not a good idea. Rather, in a loop, call `pthread_join()` for each of the threads (save the thread IDs globally, so they will be available for the calls to `pthread_join()` – user3629249 Aug 24 '16 at 23:35
1

You will need to block the calling thread until a local copy of data can be made. Semaphores can be useful here.

struct thread_param {
    void * args; sem_t * ready;
};

void * child(void * args)
{
    struct thread_param * param = args;
    /* make local copies. */
    sem_post(param->ready);
    /* do work. */
    return NULL;
}

int main(int argc, char * argv)
{
    /* setup (semaphore and other). */
    struct thread_param param;
    param.ready = &ready;

    for (i = 0; i < n; i++) {
        param.args = &i; /* or whatever. */
        pthread_create(&thread[i], &attrib, child, &param);
        sem_wait(param.ready);
    }
    /* cleanup, join on threads. */
    return 0;
}

Note: Code is untested. I apologize for any typos/errors.