Jay Wang
Jay Wang

Reputation: 2850

Thread don't terminate their job

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.

Upvotes: 0

Views: 54

Answers (1)

Andrew Henle
Andrew Henle

Reputation: 1

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

Upvotes: 2

Related Questions