Why does the local variable in my thread function get interrupted by other threads?

Question

I create a few threads in parallel performing the following thread function, the input parameter to the thread functions is a integer. different threads get different integers. In the thread function, I define a local variable thread_index to accept this integer which will be used in the calculation. My problem is that these local variables will be messed up with those from other threads, which I think should not happen. In fact, there is a loop in each thread function and the first attempt of the loop is okay for all the threads but after that, the local variable 'thread_index' is messed up. I used semaphore barrier to protect the calculation process, but that should not be my current problem. All the variables not declared are pre-defined in the main function. I could not figure out why the local variables got interrupted by others.

The following is the thread creation process, where N is an integer to control the number of threads, pthread_t t[N/2]. for(long thread_index = 0;thread_index < N/2; ++thread_index){ pthread_t t[N/2]; pthread_create(&t[thread_index],NULL,exchange_swap,(void *) thread_index);

void *exchange_swap(void *ptharg){
    // cout<<"num_phases "<<num_phases<<endl;
    long thread_index = (long) ptharg;
    for (phase = 1; phase <= num_phases;)
    {

        int num_groups = pow(2, (phase-1) % num_stages);
        // cout<<num_groups<<endl;
        int group_size = N / num_groups;
        int gindex = (thread_index) / (group_size / 2);
        int mindex = (thread_index) % (group_size / 2);

        int group_start = gindex * group_size;
        int group_end = (gindex + 1) * group_size - 1;

        int data1 = group_start + mindex;
        int data2 = group_end - mindex;
        // swap
        int temp;
        if (sortArray[data1]>sortArray[data2])
        {
            temp = sortArray[data1];
            sortArray[data1] = sortArray[data2];
            sortArray[data2] = temp;
        }

        sem_wait(&s);
        cout<<"Thread "<<thread_index+1<<" finished stage "<<(phase - 1) / num_stages + 1<<" phase "<<(phase-1) % num_stages + 1<<endl;
        finished_job++;
        if (finished_job == N/2){
            finished_job = 0;
            phase++;
            for (int i = 0; i < N; ++i)
            {
                cout<<sortArray[i]<<" ";
            }
            cout<<endl;
            sem_post(&barrier);
        }
        sem_post(&s);
        sem_wait(&barrier);
        sem_post(&barrier);
    }
    return NULL;
}

Are you sure you mean long thread_index = (long) ptharg; and not long thread_index = *(long*) ptharg;? — phonetagger
How do you create your threads? Can you please create a minimal reproducible example to show us? And why don't you use std::thread (I'm guessing here, but it seems like you don't use it). — Some programmer dude
Yes, I use long thread_index = (long) ptharg, I tried what you suggested but I got the Segmentation fault(core dumped). I also suspect that I pass an pointer inside so after one period the variable remain the same for all. But I don't know where exactly the problem is. — Qiang Yao
for(long thread_index = 0;thread_index < N/2; ++thread_index){ pthread_t t[N/2]; pthread_create(&t[thread_index],NULL,exchange_swap,(void *) thread_index); N is an integer to control the number of threads. } — Qiang Yao
@phonetagger Unfortunately the OP added the thread-creation in a comment (instead of editing the question, so that makes the code hard to read) but the OP is passing a value. When using C thread function it's common to cast values as pointers, and the opposite inside the thread function. — Some programmer dude

Some programmer dude Some programmer dude · Accepted Answer · 2019-03-07T16:46:05

Using std::thread it would be easier and more "natural" to pass values:

std::vector<std::thread> threads(N / 2);  // Create a vector of N/2 threads

// For each thread in the vector
for (unsigned thread_index = 0; thread_index < threads.size(); ++thread_index)
{
    // Create the thread
    threads[thread_index] = std::thread(exchange_swap, thread_index);
}

// Possibly do other work...

// Again for each thread (thread index no longer needed)
for (auto& thread : threads)
{
    // Wait for it to end
    thread.join();
}

Then the thread function is a simple function taking the index by value and doesn't return a value:

void exchange_swap(unsigned thread_index){
    ...
}

And as I said in a comment, passing the value like you do (by casting) is okay in this case.

Why does the local variable in my thread function get interrupted by other threads?

1 Answers