CODEWITHSUNDEEP
cmultithreadingstackmutexsemaphore
liamkr
liamkr

Reputation: 35

Using Semaphors to create a thread safe stack in C?

I'm trying to make a stack that I implemented thread safe using semaphors. It works when I push a single object onto the stack, but terminal freezes up as soon as I try to push a second item onto the stack or pop an item off of the stack. This is what I have so far and am not sure where I'm messing up. Everything complies right, but the terminal just freezes as previously stated

Heres where I create the stack

sem_t selements, sspace;
pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;

BlockingStack *new_BlockingStack(int max_size)
{
    sem_init(&selements, 0, 0);
    sem_init(&sspace, 0, max_size);

    BlockingStack *newBlockingStack = malloc(sizeof(BlockingStack));
    newBlockingStack->maxSize = max_size;
    newBlockingStack->stackTop = -1;
    newBlockingStack->element = malloc(max_size * sizeof(void *));

    if (newBlockingStack == NULL)
    {
        return NULL;
    }

    if (newBlockingStack->element == NULL)
    {
        free(newBlockingStack);
        return NULL;
    }

    return newBlockingStack;
}

And here are the Push and Pop:

bool BlockingStack_push(BlockingStack *this, void *element)
{
    sem_wait(&sspace);
    pthread_mutex_lock(&m);


    if (this->stackTop == this->maxSize - 1)
    {
        return false;
    }
    if (element == NULL)
    {
        return false;
    }
    this->element[++this->stackTop] = element;
    return true;

    pthread_mutex_unlock(&m);
    sem_post(&selements);

}

void *BlockingStack_pop(BlockingStack *this)
{
    sem_wait(&selements);
    pthread_mutex_lock(&m);

    if (this->stackTop == -1)
    {
        return NULL;
    }
    else
    {
        return this->element[this->stackTop--];
    }

    pthread_mutex_unlock(&m);
    sem_post(&sspace);
}

Upvotes: 1

Views: 213

Answers (3)

liamkr
liamkr

Reputation: 35

OK, i was working on this and finally cracked the answer after doing a little bit of internet research and debugging my code. The error was the the mutex_unlock and the sem_post had to come before the return.

Take my pop for example:

void *BlockingStack_pop(BlockingStack *this)
{
    sem_wait(&selements);
    pthread_mutex_lock(&m);

    if (this->stackTop == -1)
    {
        return NULL;
    }
    else
    {
        return this->element[this->stackTop--];
    }

    pthread_mutex_unlock(&m);
    sem_post(&sspace);
}

notice how the pthread_mutex_unlock(&m); and the sem_post(&sspace); come after the return, they actually must be placed before every return like so:

void *BlockingStack_pop(BlockingStack *this)
{
...
    pthread_mutex_unlock(&m);
    sem_post(&sspace);
        return NULL;
...
        pthread_mutex_unlock(&m);
    sem_post(&sspace);
        return this->element[this->stackTop--];
...

}

Upvotes: -1

mar
mar

Reputation: 154

For thread safety you already have mutex used (pthread_mutex_lock(&m) and pthread_mutex_unlock(&m)). Using such mutual exclusion is enough for that purpose. Once one thread obtains the mutex, other thread blocks on pthread_mutex_lock(&m) call. And only the thread currently obtaining the mutex can call pthread_mutex_unlock(&m).

Upvotes: 1

paulsm4
paulsm4

Reputation: 121799

SUGGESTED CHANGES:

sem_t sem;
...
BlockingStack *new_BlockingStack(int max_size)
{
    sem_init(&sem, 0, 1);
    ...

bool BlockingStack_push(BlockingStack *this, void *element)
{
    sem_wait(&sem);
    ...
    sem_post(&sem);
    ...

Specifically:

  • I would only initialize one semaphore object unless I was SURE I needed others

  • I would use the same semaphore for push() and pop()

  • pshared: 0 should be sufficent for synchronizing different pthreads inside your single process.

  • Initialize the semaphore to 1, because the first thing you'll do for either "push" or "pop" is sem_wait().

Upvotes: 0

Related Questions