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);

}

Answer 1

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));