HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Maintaining a count with two threads using conditional variables

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withthreadsconditionaltwousingvariablescountmaintaining

Problem

This is just a program to show the use of conditional variables when two threads are involved. One thread wants a non-zero value of count, and other thread is responsible for signaling it when the count is non-zero.

Is there something in this code which still needs an improvement? Is there something which could have been done in a better way?

``
#include

/ Declaration of a Mutex variable
mutexA. /
pthread_mutex_t mutexA;

/ Declaration of a Condition Variable
conditionVariableA. /
pthread_cond_t conditionVariableA;

/*
functionA and functionB are the argument functions of the two threads (declared
below) */
void functionA (void);
void functionB (void);

/*
count is the variable shared between threads A and B.
Thread
A wants it to be non zero. Thread B will be responsible for making it
non-zero and then issuing a signal. */
int count = -100;

int main ()
{
// Declaration of two threads namely
A, and B.
pthread_t A, B;

// Initializing the mutex lock to be shared between the threads.
pthread_mutex_init (&mutexA, NULL);

/* The function
pthread_cond_init() initialises the Condition Variable referenced by
variable
conditionVariableA with attributes referenced by variable attributes. If
attributes` is NULL, the default condition variable attributes are used. */
pthread_cond_init (&conditionVariableA, NULL);

/ Definition of two threads namely A and B /
pthread_create (&A, NULL, functionA, NULL);
pthread_create (&B, NULL, functionB, NULL);

pthread_join (A, NULL);
pthread_join (B, NULL);
}

void functionA (void argA)
{
while (1)
{
pthread_mutex_lock (&mutexA);

if (count 0)
{
pthread_cond_signal (&conditionVariableA);
std :: cout << "\nSignaled!\n";
return 0;
}
else
{
std :: cout << "\nNot signaled yet!";
}

pthread_mutex_unlock (&mutexA);
}

Solution

Couple of issues:

The code is not exception safe.

Any resource that has an open/close symantic should be handled via RAII

{
    pthread_mutex_lock (&mutexA);

    // Code
    // This may throw an exception thus will
    // cause the code to miss the unlock.

    pthread_mutex_unlock (&mutexA);
}


To fix this you should us an RAII locker object.

{
    MutexLocker    lock(mutexA);  // constructor calls lock.
                                  // detructor calls unlock.
    // Code
    // This may throw an exception
    // and it still works correctly.
}


It may work in this simple example:

if (count <= 0)
    {
        std :: cout << "\ngnitiaW!\n";
        pthread_cond_wait (&conditionVariableA, &mutexA);   
    }


But normally you need to place the wait() inside a loop.

while (count <= 0)
    {
        std :: cout << "\ngnitiaW!\n";
        pthread_cond_wait (&conditionVariableA, &mutexA);   
    }


If there are multiple threads that could enter this function then between the signal from the producer thread and this thread waking up from the wait another thread could have stolen the object. Thus you need to recheck the state you were waiting for and go back to sleep if it was stolen.

This would have been fixed by the MutexLocker above. But the code as it stands leaves the lock locked.

else
    {
        // Do something.
        std :: cout << "\nTime to enjoy!\n";

        // You are  returning early without unlock the lock.
        // This causes all your other threads to stall
        // and probably causes deadlock.
        return 0;
    }

    // This point is not reached if you return early.
    pthread_mutex_unlock (&mutexA);

Code Snippets

{
    pthread_mutex_lock (&mutexA);

    // Code
    // This may throw an exception thus will
    // cause the code to miss the unlock.

    pthread_mutex_unlock (&mutexA);
}
{
    MutexLocker    lock(mutexA);  // constructor calls lock.
                                  // detructor calls unlock.
    // Code
    // This may throw an exception
    // and it still works correctly.
}
if (count <= 0)
    {
        std :: cout << "\ngnitiaW!\n";
        pthread_cond_wait (&conditionVariableA, &mutexA);   
    }
while (count <= 0)
    {
        std :: cout << "\ngnitiaW!\n";
        pthread_cond_wait (&conditionVariableA, &mutexA);   
    }
else
    {
        // Do something.
        std :: cout << "\nTime to enjoy!\n";

        // You are  returning early without unlock the lock.
        // This causes all your other threads to stall
        // and probably causes deadlock.
        return 0;
    }

    // This point is not reached if you return early.
    pthread_mutex_unlock (&mutexA);

Context

StackExchange Code Review Q#14214, answer score: 10

Revisions (0)

No revisions yet.