pthread in C : i can't figure why pthread_cond_signal is not waking up one thread

Question

I want a routine to be done by multiple threads, once they are created they need their work to be all finished before copying their calculated things. So a thread is on cond_wait once it has its work done.

A piece of code that works now:

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

#define MAX_RAYCAST_THREADS 2

typedef struct      s_thread_env
{
    int             id;
    pthread_t       thread;
    int             work_done;
    void            *shared_data;
}                   t_thread_env;

typedef struct      s_shared_data
{
    t_thread_env    *tab_thread_env;
    int             max_thread;
    int             all_work_done;
    pthread_mutex_t mutex;
    pthread_cond_t  cond;
    int             stop;
}                   t_shared_data;

void    set_threads_again(int id, t_shared_data *shared_data)
{
    int i;

    shared_data->all_work_done = 0;
    i = -1;
    while (++i < shared_data->max_thread)
        shared_data->tab_thread_env[i].work_done = 0;
    //i = -1;
    //while (++i < shared_data->max_thread)
    //{
        //if (i != id)
        //{
            //printf("cond_signal to thread %i\n", i);
            //pthread_cond_signal(&shared_data->cond);
        //}
    //}
    pthread_cond_broadcast(&shared_data->cond);
}

void    wait_or_signal(t_thread_env *thread_env, t_shared_data *shared_data)
{
    int i;

    i = 0;
    while (i < shared_data->max_thread && shared_data->tab_thread_env[i].work_done)
        i++;
    if (i == shared_data->max_thread)
    {
        printf("    allworkdone sent by thread %i\n", thread_env->id);
        //printf("      copy_screenpixels() by thread %i\n", thread_env->id);

        set_threads_again(thread_env->id, shared_data);
    }
    else
    {
        printf("    thread number %i is waiting for other threads\n", thread_env->id);
        pthread_cond_wait(&shared_data->cond, &shared_data->mutex);
        printf("ENFIN ! thread number %i woke up from condwait\n", thread_env->id);
    }
}

void    *routine(void *arg)
{
    t_thread_env    *thread_env;
    t_shared_data   *shared_data;
    int             stop;

    thread_env = (t_thread_env *)arg;
    shared_data = (t_shared_data *)thread_env->shared_data;
    pthread_mutex_lock(&shared_data->mutex);
    while (!shared_data->stop)
    {
        printf("new frame> thread_id = %i, thread_env->work_done = %i\n", thread_env->id, thread_env->work_done);
        pthread_mutex_unlock(&shared_data->mutex);
        printf("        raycast() in routine thread %i\n", thread_env->id);
        pthread_mutex_lock(&shared_data->mutex);
        thread_env->work_done++;
        wait_or_signal(thread_env, shared_data);
    }
    pthread_mutex_unlock(&shared_data->mutex);
    return (0);
}

void    init_thread_env(t_shared_data *shared_data, int i)
{
    t_thread_env    *thread_env;

    thread_env = &shared_data->tab_thread_env[i];
    thread_env->id = i;
    thread_env->shared_data = shared_data;
    if (pthread_create(&thread_env->thread, NULL, routine, thread_env) != 0)
        printf("Error pthread_create for %i\n", i);
}

void    free_all(t_shared_data *shared_data)
{
    int i;

    pthread_mutex_lock(&shared_data->mutex);
    shared_data->stop = 1;
    pthread_mutex_unlock(&shared_data->mutex);
    i = -1;
    while (++i < shared_data->max_thread)
        pthread_join(shared_data->tab_thread_env[i].thread, NULL);
    printf("\nEND\n\n");
    //free etc
}

int     main()
{
    int             i;
    t_shared_data   *shared_data;

    shared_data = (t_shared_data*)malloc(sizeof(t_shared_data)); // if (!shared data){free etc}
    shared_data->max_thread = MAX_RAYCAST_THREADS;
    pthread_mutex_init(&shared_data->mutex, 0);
    pthread_cond_init(&shared_data->cond, 0);
    shared_data->tab_thread_env = (t_thread_env*)malloc(sizeof(t_thread_env) * shared_data->max_thread);
    i = -1;
    while (++i < shared_data->max_thread)
        init_thread_env(shared_data, i);
    while (1)
        sleep(1); //program is turning
    free_all(shared_data);
    return (0);
}

The correct output:

new frame> thread_id = 0, thread_env->work_done = 0
                raycast() in routine thread 0
        thread number 0 is waiting for other threads
new frame> thread_id = 1, thread_env->work_done = 0
                raycast() in routine thread 1
        allworkdone sent by thread 1
cond_signal to thread 0
new frame> thread_id = 1, thread_env->work_done = 0
ENFIN ! thread number 0 woke up from condwait
new frame> thread_id = 0, thread_env->work_done = 0
                raycast() in routine thread 0
        thread number 0 is waiting for other threads
                raycast() in routine thread 1
        allworkdone sent by thread 1
cond_signal to thread 0
new frame> thread_id = 1, thread_env->work_done = 0
ENFIN ! thread number 0 woke up from condwait
new frame> thread_id = 0, thread_env->work_done = 0
                raycast() in routine thread 0
        thread number 0 is waiting for other threads
                raycast() in routine thread 1

Thank you for reading me, have a good day!

EDIT: I made a more readable and compilable version with only 1 mutex (old version: https://pastebin.com/4zMyBJi2).

EDIT2: deleted some parts of the original post, and tried to avoid every data races, my code still has some (as it still does not work). But I think I am close to get something working

EDIT3: Ok it is working now, I edited the code. The main issue was my disastrous use of the shared_data variables.

Answer 1

I tried to make my raycasting threads work using 1 call of pthread_create for each thread (in an initialisation function). Is it possible to do it?

Each successful call to pthread_create creates exactly one thread, so this is the only way to do it. But do not get confused between threads' start functions and threads themselves. Multiple threads can be created to run the same thread function, but this requires multiple calls to pthread_create, one for each thread.

I guess it is better (for performances) to do it in this way (rather than an enormous amount of pthread_create and pthread_join calls), is this correct?

Having chosen to use a certain number of threads to do certain pieces of work, the number of pthread_create calls is already determined. If you have performance concerns then they should be about how many threads to use, the details of the work they are to perform, and the nature and granularity of their synchronization.

In order to make it happen, the last thread (number n) to finish his job has to tell the other ones that every thread has finished, so there is a pthread_cond_wait for the (n - 1) first threads, and the last thread (number n) calls pthread_cond_signal for each (n - 1) first ones. Each thread has his own mutex.

That seems a little overkill, even if you can make it technically correct. It doesn't make much sense to have a mutex per thread, because mutexes are useful only when different threads use the same one. It may make sense to have different mutexes for protecting different shared resources, however. You would probably want CVs for some of those, but not all of them.

This appears to be (at least one area) where your code is failing. Your threads are not properly synchronized because they all rely on different mutexes to protect the same resources. It's not clear to me whether it makes sense for your program to have any more than just one mutex, but I would start by reducing your usage to that.