Pthread_join not working as it should to popular a C array

Question

I have the following code:

#include <stdio.h>
#include <pthread.h>
#include <semaphore.h>
#include <locale.h>
#include <time.h>

//declaração das variáveis globais
float *x;
float *y;
float *z;
pthread_mutex_t *mutexes;

typedef struct
{
    float* vetor;
    unsigned int posInicial;
    unsigned int posFinal;
    unsigned int contMutexThread;
} Thread_preenche;

typedef struct
{
    float* x;
    float* y;
    float* z;
    unsigned int posInicial;
    unsigned int posFinal;
    unsigned int contMutexThread;
} Thread_soma;

void *preencheVetores(void *argPtr){
    Thread_preenche *thread_p = (Thread_preenche*)argPtr;
    unsigned int inicio = thread_p->posInicial;
    unsigned int final = thread_p->posFinal;

    for(;inicio <= final; inicio++){
        pthread_mutex_lock(&mutexes[thread_p->contMutexThread]);
        thread_p->vetor[inicio] = (float)rand()/(float)(RAND_MAX/1.0);
        printf("x[%u] e o valor é: %f \n", inicio, x[inicio]);
        pthread_mutex_unlock(&mutexes[thread_p->contMutexThread]);
    }
    pthread_exit(0);
}

int main() {
    setlocale(LC_ALL, "Portuguese");

    //obtendo o tamanho do vetor e o número de threads que o usuário deseja
    int tamanho_vetor;
    int n;
    while (1){
        printf("Digite o tamanho do vetor: ");
        scanf("%d", &tamanho_vetor);
        printf("Digite o número de threads: ");
        scanf("%d", &n);
        if (tamanho_vetor % n != 0){
              printf("O número de threads deve ser múltiplo do tamanho do vetor. \n");
        }else{
            break;
        }
    }

    //inicialização dos vetores
    x =(float*) malloc(sizeof(float)*tamanho_vetor);
    y =(float*) malloc(sizeof(float)*tamanho_vetor);
    z =(float*) malloc(sizeof(float)*tamanho_vetor);
    mutexes = (pthread_mutex_t*) malloc(sizeof(pthread_mutex_t)*n);
    int divisao_vetor[n+1];
    for (int i = 0; i <= n; i++){
        divisao_vetor[i] = ((tamanho_vetor/n)*(i));
    }
    pthread_t thread[n];
    for (int i = 0; i < n; ++i){
        pthread_mutex_init(&mutexes[i],NULL);
        Thread_preenche thread_preenche = {
            .vetor = &x[0],
            .posInicial = (divisao_vetor[i]),
            .posFinal = divisao_vetor[i+1]-1,
            .contMutexThread = i
        };
        pthread_create(&thread[i], NULL, preencheVetores, &(thread_preenche));
        //jeito errado, o join logo em seguida do create da thread.
        //pthread_join(thread[i], NULL);
    }
    //Assim seria o jeito certo de fazer, o join sendo depois
    //mas não está funcionando.
    for (int i = 0; i < n; i++){
        pthread_join(thread[i], NULL);
    }

    free(x);
    free(y);
    free(z);
    free(mutexes);

    #ifdef _WIN32
        system("pause");
    #else __linux__
        system("read -p 'Press Enter to continue...\n' key");
    #endif
    
   return 0;
}

void *somaVetores(void* argPtr);

I create several threads, each one to popular a part of the vector. If I do it the wrong way it is to do the Join right after the create of the threads inside a is, like this:

pthread_create(&thread[i], NULL, preencheVetores, &(thread_preenche));
pthread_join(thread[i], NULL);

I have the expected result, with all fields populated. But, has been removed all competition from threads, which is not desired.

If I apply Join to threads after they’ve all been created, this way:

//abstraindo o for que cria as threads.
pthread_create(&thread[i], NULL, preencheVetores, &(thread_preenche));

for (int i = 0; i < n; i++){
    pthread_join(thread[i], NULL);
}

I don’t have the desired result, are populated only the last elements of the array, as a basis in the division I did according to the number of threads, it is as if all threads were populating the same range of the array, although it had defined different ranges for each popular thread the array. Why is this happening? Where I am wrong ?

As an example, I have the following result (x is the array I am populating):

Digite o tamanho do vetor: 32
Digite o número de threads: 4
x[24] e o valor é: 0.840188 
x[25] e o valor é: 0.394383 
x[26] e o valor é: 0.783099 
x[27] e o valor é: 0.798440 
x[28] e o valor é: 0.911647 
x[29] e o valor é: 0.197551 
x[30] e o valor é: 0.335223 
x[31] e o valor é: 0.768230 
x[24] e o valor é: 0.277775 
x[25] e o valor é: 0.553970 
x[26] e o valor é: 0.477397 
x[27] e o valor é: 0.628871 
x[28] e o valor é: 0.364784 
x[29] e o valor é: 0.513401 
x[30] e o valor é: 0.952230 
x[31] e o valor é: 0.916195 
x[24] e o valor é: 0.635712 
x[25] e o valor é: 0.717297 
x[26] e o valor é: 0.141603 
x[27] e o valor é: 0.606969 
x[28] e o valor é: 0.016301 
x[29] e o valor é: 0.242887 
x[30] e o valor é: 0.137232 
x[31] e o valor é: 0.804177 
x[24] e o valor é: 0.156679 
x[25] e o valor é: 0.400944 
x[26] e o valor é: 0.129790 
x[27] e o valor é: 0.108809 
x[28] e o valor é: 0.998924 
x[29] e o valor é: 0.218257 
x[30] e o valor é: 0.512932 
x[31] e o valor é: 0.839112

Answer 1

There are some problems in the program, but the main one that causes this behavior is the struct initialization and Thread_preenche inside the looping.

Although the structure is declared within the looping, the compiler does not allocate memory for a new structure at each iteration, so it is treated as if it were declared only once outside the looping.

With that, all the threads are initialized with the last value placed in the struct, generating the output only with "the latest values".

The solution to this problem is to declare a array of structures Thread_preenche and initialize each thread with its own structure:

Thread_preenche thread_preenche[n];
...
for (int i = 0; i < n; ++i){                
    thread_preenche[i].vetor = &x[0];
    thread_preenche[i].posInicial = (divisao_vetor[i]);
    thread_preenche[i].posFinal = divisao_vetor[i+1]-1;
    thread_preenche[i].contMutexThread = i;

    pthread_mutex_init(&mutexes[i],NULL);
    pthread_create(&thread[i], NULL, preencheVetores, &(thread_preenche[i]));
...
    

---

Other improvements that can be made, depending on the objective of the programme:

1. There’s no point in declaring a lock mutex for each thread. The The aim of the lock is to block the other threads during access to a shared resource (e.g., a file, device, shared memory, etc.), so the latch needs to be unique by recourse and all of them, shared between the threads. In the case of your program, if the goal is update only memory, as there is already a division of the vector for each thread, the lock is not necessary, however, the printf accesses a I/O device and in this case it may make sense to use one lock to access it.

2. It may not be necessary to pre-calculate the division of vector no array divisao_vetor, because it can be calculated directly inside the looping that creates the threads. However, if the programme is just an example of a more complex system, this storage can make sense.

Follows an optimized and commented version of the question program, just as an example:

#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>
#include <semaphore.h>
#include <locale.h>
#include <time.h>

//declaração das variáveis globais
float *x;
float *y;
float *z;

// AQUI => Cria apenas um mutex, compartilhado entre as threads
static pthread_mutex_t mutex;

typedef struct
{
    float* vetor;
    unsigned int posInicial;
    unsigned int posFinal;
    unsigned int contMutexThread;
} Thread_preenche;

typedef struct
{
    float* x;
    float* y;
    float* z;
    unsigned int posInicial;
    unsigned int posFinal;
    unsigned int contMutexThread;
} Thread_soma;

void *preencheVetores(void *argPtr){
    Thread_preenche *thread_p = (Thread_preenche*)argPtr;
    unsigned int inicio = thread_p->posInicial;
    unsigned int final = thread_p->posFinal;
    
    for(;inicio <= final; inicio++){
        pthread_mutex_lock(&mutex);
        thread_p->vetor[inicio] = (float)rand()/(float)(RAND_MAX/1.0);
        printf("x[%u] e o valor é: %f \n", inicio, x[inicio]);
        pthread_mutex_unlock(&mutex);
    }
    
    pthread_exit(0);
}

int main() {
    setlocale(LC_ALL, "Portuguese");

    //obtendo o tamanho do vetor e o número de threads que o usuário deseja
    int tamanho_vetor;
    int n;
    while (1){
        printf("Digite o tamanho do vetor: ");
        scanf("%d", &tamanho_vetor);
        printf("Digite o número de threads: ");
        scanf("%d", &n);
        if (tamanho_vetor % n != 0){
            printf("O número de threads deve ser múltiplo do tamanho do vetor. \n");
        }else{
            break;
        }
    }

    //inicialização dos vetores
    x =(float*) malloc(sizeof(float)*tamanho_vetor);
    y =(float*) malloc(sizeof(float)*tamanho_vetor);
    z =(float*) malloc(sizeof(float)*tamanho_vetor);

    // AQUI => Excluído a declaração do vetor de travas e divisão
    //
    //    mutexes = (pthread_mutex_t*) malloc(sizeof(pthread_mutex_t)*n);
    //    int divisao_vetor[n+1];
    //    for (int i = 0; i <= n; i++){
    //        divisao_vetor[i] = ((tamanho_vetor/n)*(i));
    //    }
    //
    // AQUI => Calcula o tamanho do job de cada thread
    int num_job = tamanho_vetor / n;
    
    // AQUI => precisa declarar um parâmetro para cada thread para
    //         não sobrescrever os valores
    Thread_preenche thread_preenche[n];

    // AQUI => Inicializa apenas um MUTEX
    pthread_mutex_init(&mutex, NULL);

    pthread_t thread[n];

    for (int i = 0; i < n; ++i){
        // AQUI => Inicializa cada parâmetro separadamente e já
        // calcula o job de cada thread
        thread_preenche[i].vetor = &x[0];
        thread_preenche[i].posInicial = i*num_job;
        thread_preenche[i].posFinal = i*num_job+num_job-1;
        thread_preenche[i].contMutexThread = i;
        
        pthread_create(&thread[i], NULL, preencheVetores, &thread_preenche[i]);
    }
    
    //Assim seria o jeito certo de fazer, o join sendo depois
    //mas não está funcionando.
    for (int i = 0; i < n; i++){
        pthread_join(thread[i], NULL);
    }

    free(x);
    free(y);
    free(z);
    //free(mutexes);
    pthread_mutex_destroy(&mutex);

#ifdef _WIN32
    system("pause");
#else
    system("read -p 'Press Enter to continue...\n' key");
#endif
    return 0;
}

void *somaVetores(void* argPtr);