pthread_create skips or repeats

Question

pthread_create function gets skipped or sometimes called twice. The question I am solving is:

Given a global array that contains number from 1 to 100. You are required to make 10 threads and each thread must find the sum of square of 10 numbers.

Thread 1 must calculate from 1 to 10

Thread 2 must calculate from 11 to 20

...so on.

Each thread must return its individual sum to a global variable sum initialized with zero.

My try:

#include<stdio.h>
#include<unistd.h>
#include<pthread.h>
#include<sys/wait.h>
#include<sys/types.h>
#include<stdlib.h>
#include<semaphore.h>

int arr[100];

sem_t s;

int sum=0;

void *calculate(void *i){
    sem_wait(&s);
    int j=(*((int*)i));
    int k;
    printf("j: %d\n",j);
    int temp=0;
    for(k=j*10;k<(j*10)+10;k++){
        temp=temp+(arr[k]*arr[k]);
    }
    sum+=temp;

    printf("sum: %d j: %d\n\n",sum,j);

    sem_post(&s);
    pthread_exit(NULL);
}

int main(){
    sem_init(&s,0,1);
    pthread_t threads_array[10];
    int i=0;
    int *k=(int *)malloc(sizeof(int));
    for(i=0;i<100;i++){
        arr[i]=i+1;
    }

    int temp=0,temp_i;

    for(i=0;i<10;i++){
        (*k)=i;
        printf("k: %d\n",(*k));
        pthread_create(&(threads_array[i]),NULL,calculate,(void*)k);
    }
    for(i=0;i<10;i++){
       pthread_join(threads_array[i],NULL);
    }

    printf("%d",sum);
    return 0;
}

I have used semaphores. So that only one thread access the global resource at a time.

The output I am getting is:

Output screen

My question is why is it repeating some values and skipping some? I am not using pthread_create correctly?

I have also tried using a new value of k each time:

for(i=0;i<2;i++){
    int *k=&i;
    printf("k: %d\n",(*k));
    pthread_create(&(threads_array[i]),NULL,calculate,(void*)k);

}

Instead of linking an image, it would be better to paste the output within the post itself, much like you've done with your code. — Kevin Lee, May 03 '18 at 09:49
`k` is shared among your threads... Use some thread local storage or an array of ten variables... Don't take the semaphore will computing the local sum, just use it to protect the `sum+=` section — Jean-Baptiste Yunès, May 03 '18 at 14:54

Andrew Henle · Accepted Answer · 2018-05-03T11:06:47.407

This code passes the same address of k to each thread, but it changes the value in that memory:

for(i=0;i<10;i++){
    (*k)=i;
    printf("k: %d\n",(*k));
    pthread_create(&(threads_array[i]),NULL,calculate,(void*)k);
}

By the time this code runs

void *calculate(void *i){
    sem_wait(&s);
    int j=(*((int*)i));
        .
        .
        .

the value has likely changed because the main thread changed it.

This would be better, as it passes the value of i, but it depends on the existence of intptr_t and platform-specific behavior allowing the transformation back to int, so it's not strictly compliant C code:

for(i=0;i<10;i++){
    pthread_create(&(threads_array[i]),NULL,calculate,(void*)(intptr_t)i);
}

and

void *calculate(void *i){
    sem_wait(&s);
    int j= (intptr_t)i;

That passes the value of i as a void * pointer value.

But if intptr_t exists, this is better:

intptr_t i;
    .
    .
    .
for(i=0;i<10;i++){
    pthread_create(&(threads_array[i]),NULL,calculate,(void*)i);
}

and

void *calculate(void *i){
    sem_wait(&s);
    intptr_t j= (intptr_t)i;

Realistically, there aren't many platforms out there any more that approach won't work on.

Alternatively, in strictly conforming C, to pass the address of an actual int value, you need a separate int for each thread that's guaranteed to exist when the thread is running:

// this is a local variable, but since it's in the same scope as
// both pthread_create() and pthread_join(), it will still exist
// for the entire lifetime of each thread created
int threadnum[10];

for(i=0;i<10;i++){
    threadnum[i]=i;
    pthread_create(&(threads_array[i]),NULL,calculate,(void*)&(threadnum[i]));
}

and

void *calculate(void *i){
    sem_wait(&s);
    int j=(*((int*)i));

I also tried passing new value of k every time the loop runs. Please check the edit — Jasmine Kaur, May 03 '18 at 10:50
@JasmineKaur You're passing the same address to each thread. Then you modify the value in that memory, but you have no control over which thread runs first, and you don't know *when* they will run. The threads might not even start until your main thread is blocked in the `pthread_join()` loop. Your edit doesn't really help as you're passing the address of a local loop variable that's likely out of scope by the time the thread tries to dereference the pointer. — Andrew Henle, May 03 '18 at 10:55
Awesome. That array idea seems so logical, I learnt my mistake — Jasmine Kaur, May 03 '18 at 18:50

pthread_create skips or repeats

1 Answers1