CODEWITHSUNDEEP
cthread-safetypthreadsdeadlock
Joachim
Joachim

Reputation: 3260

Implementing an thread-safe queue with pThreads: Deadlock?

I'm trying to implement a thread-safe queue based on a fixed array. The queue holds an array of void pointers.

pthread_t a;
pthread_t b;
Queue *q;

Queue* queue_new(int size)
{
    // malloc stuff
}

void queue_put(Queue* q, void* item)
{
    pthread_mutex_lock(&(q->lock));
    // details on array queue managment
    pthread_mutex_unlock(&(q->lock));
}

void* queue_get(Queue* q)
{
    pthread_mutex_lock(&(q->lock));
    // more details ...
    return q->queue[old_front];
    pthread_mutex_unlock(&(q->lock));
}


void *func_a (void *ptr)
{
    void *ptr1 = malloc(sizeof(int));
    *((int*)ptr1) = 5;
    queue_put(q, ptr1);

    void *ptr2 = malloc(sizeof(int));
    *((int*)ptr2) = 4;
    queue_put(q, ptr2);
    return NULL;
}

void *func_b (void *ptr)
{
    void *ptr3 = malloc(sizeof(int));
    *((int*)ptr3) = 7;
    queue_put(q, ptr3);

    queue_get(q); // critical part !

    return NULL;
}


int main ()
{
    q = queue_new(3);
    pthread_create(&a, NULL, func_a, NULL);
    pthread_create(&b, NULL, func_b, NULL);
    pthread_join(a, NULL);
    pthread_join(b, NULL);

    queue_print(q);

    return 0;
}

I think this a pretty straight-forward approach. Unfortunately, the program freezes. However, when I remove the queue_get(q); in func_b it works just fine. I think this must be a deadlock of some kind. Any ideas? The non-threadsafe version of the Queue has been tested and works just fine. The code is hidden for clarity. Any ideas?

Upvotes: 1

Views: 1742

Answers (3)

Kninnug
Kninnug

Reputation: 8053

You'll want to move the unlock-line in queue_get above the return, as of now it is never reached. So the lock is never released.

pthread_mutex_unlock(&(q->lock));
return q->queue[old_front];

or, what you'll probably want, to avoid touching it outside a lock:

void * ret = q->queue[old_front];
pthread_mutex_unlock(&(q->lock));
return ret;

(From a more stylistic point: your value-allocating would be much 'cleaner' this way:

int * ptr1 = malloc(sizeof(*ptr1));
*ptr1 = 5;
queue_put(q, ptr1);

note the lack of casts needed)

Upvotes: 1

Geoffrey
Geoffrey

Reputation: 11354

You are returning before you unlock the mutex in queue_get:

return q->queue[old_front];
pthread_mutex_unlock(&(q->lock));

This should be:

void *retValue = q->queue[old_front];
pthread_mutex_unlock(&(q->lock));
return retValue;

Upvotes: 1

Cemafor
Cemafor

Reputation: 1653

I believe the problem lies within queue_get. You are returning before the mutex is unlocked. Try storing the return value into a temperary variable, unlock the mutex, then return the value.

void* queue_get(Queue* q)
{
    void* temp;
    pthread_mutex_lock(&(q->lock));
    // more details ...
    temp = q->queue[old_front];
    pthread_mutex_unlock(&(q->lock));
    return temp;
}

Upvotes: 1

Related Questions