What is the best solution to pause and resume pthr

Question

I've found the following topic (here) about pthreads but there are many good solutions.

I wanted to know if the following piece of code is valid and if so, why the same lock is used to call pthread_cond_wait as well as access it and then unlocked right away:

void suspendMe()
{
    pthread_mutex_lock(&m_SuspendMutex);
    pthread_cond_wait(&m_ResumeCond, &m_SuspendMutex);
    pthread_mutex_unlock(&m_SuspendMutex);
}

Would it not be better to use 2 separate mutexes here, or is this the correct way to suspend a pthread??

Thanks in advance!

EDIT:

Awesome replies, thanks all. Just one more related question. Since I want to resume a thread separately in another function, would this be more proper in resuming it?

void suspendMe()
{
    pthread_mutex_lock(&m_SuspendMutex);
    pthread_cond_wait(&m_ResumeCond, &m_SuspendMutex);
}
void resumeMe()
{
    pthread_cond_signal(&m_ResumeCond);
    pthread_mutex_unlock(&m_SuspendMutex);
}

Thanks again everyone!! :~)

Answer 1

Actually, this code isn't thread safe. The mutex isn't actually protecting anything, leaving the implied predicate vulnerable to race conditions.

Look at this code -- what is the mutex protecting? What protects the suspend/resume state?

void suspendMe()
{
    pthread_mutex_lock(&m_SuspendMutex);
    pthread_cond_wait(&m_ResumeCond, &m_SuspendMutex);
}
void resumeMe()
{
    pthread_cond_signal(&m_ResumeCond);
    pthread_mutex_unlock(&m_SuspendMutex);
}

This is correct:

void suspendMe()
{ // tell the thread to suspend
    pthread_mutex_lock(&m_SuspendMutex);
    m_SuspendFlag = 1;
    pthread_mutex_unlock(&m_SuspendMutex);
}
void resumeMe()
{ // tell the thread to resume
    pthread_mutex_lock(&m_SuspendMutex);
    m_SuspendFlag = 0;
    phtread_cond_broadcast(&m_ResumeCond);
    pthread_mutex_unlock(&m_SuspendMutex);
}
void checkSuspend()
{ // if suspended, suspend until resumed
    pthread_mutex_lock(&m_SuspendMutex);
    while (m_SuspendFlag != 0) pthread_cond_wait(&m_ResumeCond, &m_SuspendMutex);
    pthread_mutex_unlock(&m_SuspendMutex);
}

The thread should call checkSuspend at safe points where it can be suspended. Other threads can call suspendMe and resumeMe to suspend/resume the thread.

Notice that now the mutex protects the m_SuspendFlag variable, ensuring that the thread is told to suspend, told to resume, and checks whether it should suspend or stay suspended under protection, making the code thread-safe.

Would it not be better to use 2 separate mutexes here, or is this the correct way to suspend a pthread??

Using two mutexes would defeat the entire point of condition variables. The whole mechanism by which they work is that you can check whether there is something you should wait for and then atomically wait for it without either holding the lock while you wait or having to release the lock and then wait. If you hold the lock while you wait, how can any other thread change the state? And if you release the lock and then wait, what happens if you miss the change in state?

By the way, it almost never makes sense to pause or resume a thread. If you feel like you need to pause a thread from the outside, that just indicates that you coded the thread to do something you didn't actually want it to do. Questions about pausing or resuming threads often indicate an incorrect mental model of thread programming. A thread might need to wait for something, but it shouldn't be "paused" from the outside because it should already know by its own coding when it shouldn't do some particular bit of work.

Answer 2

This is the correct way. pthread_cond_wait unlocks m_SuspendMutex, then waits on m_ResumeCond and then locks m_SuspendMutex again before returning.

The reason it works this way is because condition variables are used to signal a change of some state, and since this state is shared it must be locked while some thread is accessing it. E.g. consider implementing a queue of events:

T get_item()
{
    pthread_mutex_lock(&mutex);
    while(qu.empty())
        pthread_cond_wait(&cond, &mutex);
    T ret = qu.front();
    qu.pop();
    pthread_mutex_unlock(&mutex);
    return ret; // we got an item from a queue
}

void add_item(T x)
{
    pthread_mutex_lock(&mutex);
    qu.push(x);
    pthread_mutex_unlock(&mutex);
    pthread_cond_signal(&cond);
}

Note that:

1. All the access to the queue qu is synchronized using the mutex.
2. When we wait on the condition variable, we must unlock the mutex in order to allow the other threads to add items to the queue, and when it have been changed we must lock the mutex again to actually inspect the queue. This is exactly what pthread_cond_signal does.

Answer 3

I wanted to know if the following piece of code is valid

Yes.

and if so, why the same lock is used to call pthread_cond_wait as well as access it and then unlocked right away:

The pthread_cond_wait() requires a locked mutex. This is because of the way condition varaibles are used. Normally they are used like this:

pthread_mutex_lock(&m_SuspendMutex);
while (!ready())
{
    pthread_cond_wait(&m_ResumeCond, &m_SuspendMutex);
}

// Modify the resources protexted by the lock.
pthread_mutex_unlock(&m_SuspendMutex);

So you acquire the lock saying you want to modify the resources protected by the lock. But if the object is not in a ready() state then you suspend your thread with pthread_cond_wait() which suspends the thread and unlocks the mutex (thus allowing other threads to aquire the lock and potentially transform the object into a ready() state).

When the ready state is achieved the m_ResumeCond is signalled and a waiting thread is released from suspension. But before it is allowed to exit pthread_cond_wait() it must reaquire the lock on m_SuspendMutex to make sure it is the only thread modifying the resources protected by the mutex.

To make sure the above is done correctly in an atomic fashion (the unlock/suspend/resume/lock) must by done by pthread_cond_wait().

Would it not be better to use 2 separate mutexes here

No.

or is this the correct way to suspend a pthread??

As good as any other I suppose.