Thread don't terminate their job

Question

I am writing a concurrent C program where I want to wait for all threads to finish in the main().

Based on this solution, I wrote the following code in main():

// Create threads
pthread_t cid[num_mappers];
int t_iter;
for (t_iter = 0; t_iter < num_mappers; t_iter++){
    pthread_create(&(cid[t_iter]), NULL, &map_consumer, NULL);
}

// Wait for all threads to finish
for (t_iter = 0; t_iter < num_mappers; t_iter++){
    printf("Joining %d\n", t_iter);
    int result = pthread_join(cid[t_iter], NULL);
}

printf("Done mapping.\n");

The function passed into threads is defined as:

// Consumer function for mapping phase
void *map_consumer(void *arg){
    while (1){
        pthread_mutex_lock(&g_lock);
        if (g_cur >= g_numfull){
            // No works to do, just quit
            return NULL;
        }

        // Get the file name
        char *filename = g_job_queue[g_cur];
        g_cur++;
        pthread_mutex_unlock(&g_lock);

        // Do the mapping
        printf("%s\n", filename);
        g_map(filename);
    }
}

The threads are all successfully created and executed, but the join loop will never finish if num_mappers >= 2.

What happens in `map_consumer` when `g_cur` is big enough to quit ? does the thread keeps its `pthread_mutex_lock` ? — grorel, Apr 17 '18 at 14:22
@grorel Oh I see! I have to `unlock` before returning, otherwise all other threads are just waiting for getting the lock. Thanks! — Jay Wang, Apr 17 '18 at 14:23

score 2 · Accepted Answer · answered Apr 17 '18 at 14:25

You return without unlocking the mutex:

    pthread_mutex_lock(&g_lock);
    if (g_cur >= g_numfull){
        // No works to do, just quit
        return NULL;  <-- mutex is still locked here
    }

    // Get the file name
    char *filename = g_job_queue[g_cur];
    g_cur++;
    pthread_mutex_unlock(&g_lock);

So only one thread ever returns and ends - the first one, but since it never unlocks the mutex, the other threads remain blocked.

You need something more like

    pthread_mutex_lock(&g_lock);
    if (g_cur >= g_numfull){
        // No works to do, just quit
        pthread_mutex_unlock(&g_lock);
        return NULL;
    }

    // Get the file name
    char *filename = g_job_queue[g_cur];
    g_cur++;
    pthread_mutex_unlock(&g_lock);

It would be neat to have some sort of RAII or `try`/`finally` in C for these situations, but I guess that's why C++ exists. — vgru, Apr 17 '18 at 15:08
@Groo: Exactly my thought too, RAII rules! ;) I have many times successfully used C++ in combination with pure pthreads, in cases where the thin std::thread layer on top of pthreads hides some more exotic pthreads settings/config I need. — Erik Alapää, Apr 17 '18 at 18:36

Thread don't terminate their job

1 Answers1