CODEWITHSUNDEEP
c
Winston Zhao
Winston Zhao

Reputation: 79

Segfault with pthread and concurrency in C

I'm trying to have a thread, that waits until a task is assigned and then will do it, however I'm running into complications.

#include "dispatchQueue.h"
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

pthread_mutex_t mutex;
pthread_cond_t cond;
task_t *task;

void test1() {
    sleep(1);
    printf("test1 running\n");
}

void* do_stuff(void *args) {
    printf("in do stuff\n");
    pthread_mutex_lock(&mutex);
    printf("after do stuff has lock\n");
    task_t *task = (task_t *)args;
    (task->work) (task->params);
    pthread_mutex_unlock(&mutex);

}

int main(int argc, char** argv) {
    pthread_t thread;
    pthread_mutex_init(&mutex, NULL);
    pthread_cond_init(&cond, NULL);
    pthread_mutex_lock(&mutex);
    printf("after main gets lock\n");
    pthread_create(&thread, NULL, do_stuff, task);
    task = task_create(test1, NULL, "test1");
    pthread_mutex_unlock(&mutex);   
    printf("after main unlocks \n");

    pthread_join(thread, NULL);
}

The above code will give a segfault, however if I switch the lines pthread_create and task = task_create(), then it works fine. I'm not familiar with C at all, so I'm wondering why this is?

This is how task is created if that helps, at this point I'm pretty sure it's a problem with the way I'm using pthreads.

task_t *task_create(void (*work)(void *), void *param, char* name) {
    task_t *task_ptr = malloc(sizeof(task_t));
    if (task_ptr == NULL) {
        fprintf(stderr, "Out of memory creating a new task!\n");
        return NULL;
    }   
    task_ptr->work = work;
    task_ptr->params = param;
    strcpy(task_ptr->name, name);
    return task_ptr;
}

Upvotes: 0

Views: 148

Answers (1)

David Schwartz
David Schwartz

Reputation: 182769

pthread_create(&thread, NULL, do_stuff, task);
task = task_create(test1, NULL, "test1");

You're passing junk to the thread. You haven't set task to any particular value here, yet you pass it to the thread as a parameter.

void* do_stuff(void *args) {         // *** args is garbage here
    printf("in do stuff\n");
    pthread_mutex_lock(&mutex);
    printf("after do stuff has lock\n");
    task_t *task = (task_t *)args;   // ** So task is garbage here
    (task->work) (task->params);
    pthread_mutex_unlock(&mutex);
}

Here, you initialize task from args. But args has a garbage value.

If you have some kind of collection that's going to track what tasks a thread is going to work on, you have to pass the thread a parameter that allows it to reliably find that collection. In this particular case, &task would work.

Upvotes: 1

Related Questions