1

I'm writing an Audio class that holds an std::thread for refilling some buffers asynchronously. Say we call the main thread A and the background (class member) thread B. I'm using an std::mutex to block thread B whenever the sound is not playing, that way it doesn't run in the background when unnecessary and doesn't use excess CPU power. The mutex locked by thread A by default, so thread B is blocked, then when it's time to play the sound thread A unlocks the mutex and thread B runs (by locking then immediately unlocking it) in a loop.

The issue comes up when thread B sees that it's reached the end of the file. It can stop playback and clean up buffers and such, but it can't stop its own loop because thread B can't lock the mutex from thread A.

Here's the relevant code outline:

class Audio {
private:

    // ...

    std::thread Thread;
    std::mutex PauseMutex;    // mutex that blocks Thread, locked in constructor
    void ThreadFunc();     // assigned to Thread in constructor

 public:

    // ...

    void Play();
    void Stop();
}

_

void Audio::ThreadFunc() {

    // ... (include initial check of mutex here)

    while (!this->EndThread) {    // Thread-safe flag, only set when Audio is destructed

            // ... Check and refill buffers as necessary, etc ...

        if (EOF)
             Stop();

        // Attempt a lock, blocks thread if sound/music is not playing
        this->PauseMutex.lock();
        this->PauseMutex.unlock();
    }
}

void Audio::Play() {
     // ...
     PauseMutex.unlock();     // unlock mutex so loop in ThreadFunc can start
}

void Audio::Stop() {
     // ...
     PauseMutex.lock();     // locks mutex to stop loop in ThreadFunc
     // ^^ This is the issue here
}

In the above setup, when the background thread sees that it's reached EOF, it would call the class's Stop() function, which supposedly locks the mutex to stop the background thread. This doesn't work because the mutex would have to be locked by the main thread, not the background thread (in this example, it crashes in ThreadFunc because the background thread attempts a lock in its main loop after already locking in Stop()).

At this point the only thing I could think of would be to somehow have the background thread lock the mutex as if it was the main thread, giving the main thread ownership of the mutex... if that's even possible? Is there a way for a thread to transfer ownership of a mutex to another thread? Or is this a design flaw in the setup I've created? (If the latter, are there any rational workarounds?) Everything else in the class so far works just as designed.

TheTrueJard
  • 471
  • 4
  • 18
  • 3
    take a look at [condition variables](https://en.cppreference.com/w/cpp/thread/condition_variable) – 463035818_is_not_an_ai Aug 03 '18 at 08:39
  • 3
    This is *begging* for an [RAII](https://en.cppreference.com/w/cpp/language/raii)-based locking scheme. I concur with the above. This needs some genuine predicate data, a single mutex to protected that data, and a condition variable to signal potential state change of that data. If you find yourself manually invoking `lock` and `unlock` with the `std::mutex` class, and that's *all* you seem to be doing, you're probably doing something wrong. – WhozCraig Aug 03 '18 at 08:43
  • @user463035818 Would that not require the main thread to be repeatedly checking it to see whether it should call `Stop()`? – TheTrueJard Aug 03 '18 at 08:43
  • you are already repeatedly locking and unlocking the mutex in the loop. Its more natural to have a loop `while(running) { ...` where `running` can be set by either thread – 463035818_is_not_an_ai Aug 03 '18 at 08:44
  • The mutex is being locked and unlocked repeatedly in the background thread, meanwhile the main thread might be busy with some other completely unrelated task – TheTrueJard Aug 03 '18 at 08:47
  • This is not really the correct use of a mutex. I think a shared variable would work just as well. All you are doing is setting the variable to play or stop. A mutex typically controls access to a resource that is needs to the altered atomically. For example a fifo where you want the reader and write to access it atomically. (Although I tend to design a fifo so that it does not need to be accessed atomically.) – William J Bagshaw Aug 03 '18 at 08:58

2 Answers2

9

I'm not going to even pretend to understand how your code is trying to do what it is doing. There is one thing, however, that is evident. You're trying to use a mutex for conveying some predicate state change, which is the wrong vehicle to drive on that freeway.

Predicate state change is handled by coupling three things:

  • Some predicate datum
  • A mutex to protect the predicate
  • A condition variable to convey possible change in predicate state.

The Goal

The goal in the below example is to demonstrate how a mutex, a condition variable, and predicate data are used in concert when controlling program flow across multiple threads. It shows examples of using both wait and wait_for condition variable functionality, as well as one way to run a member function as a thread proc.


Following is a simple Player class toggles between four possible states:

  • Stopped : The player is not playing, nor paused, nor quitting.
  • Playing : The player is playing
  • Paused : The player is paused, and will continue from whence it left off once it resumes Playing.
  • Quit : The player should stop what it is doing and terminate.

The predicate data is fairly obvious. the state member. It must be protected, which means it cannot be changed nor checked unless under the protection of the mutex. I've added to this a counter that simply increments during the course of maintaining the Playing state for some period of time. more specifically:

  • While Playing, each 200ms the counter increments, then dumps some data to the console.
  • While Paused, counter is not changed, but retains its last value while Playing. This means when resumed it will continue from where it left off.
  • When Stopped, the counter is reset to zero and a newline is injected into the console output. This means switching back to Playing will start the counter sequence all over again.
  • Setting the Quit state has no effect on counter, it will be going away along with everything else.

The Code

#include <iostream>
#include <mutex>
#include <condition_variable>
#include <thread>
#include <unistd.h>

using namespace std::chrono_literals;

struct Player
{
private:
    std::mutex mtx;
    std::condition_variable cv;
    std::thread thr;

    enum State
    {
        Stopped,
        Paused,
        Playing,
        Quit
    };

    State state;
    int counter;

    void signal_state(State st)
    {
        std::unique_lock<std::mutex> lock(mtx);
        if (st != state)
        {
            state = st;
            cv.notify_one();
        }
    }

    // main player monitor
    void monitor()
    {
        std::unique_lock<std::mutex> lock(mtx);
        bool bQuit = false;

        while (!bQuit)
        {
            switch (state)
            {
                case Playing:
                    std::cout << ++counter << '.';
                    cv.wait_for(lock, 200ms, [this](){ return state != Playing; });
                    break;

                case Stopped:
                    cv.wait(lock, [this]() { return state != Stopped; });
                    std::cout << '\n';
                    counter = 0;
                    break;

                case Paused:
                    cv.wait(lock, [this]() { return state != Paused; });
                    break;

                case Quit:
                    bQuit = true;
                    break;
            }
        }
    }

public:
    Player()
        : state(Stopped)
        , counter(0)
    {
        thr = std::thread(std::bind(&Player::monitor, this));
    }

    ~Player()
    {
        quit();
        thr.join();
    }

    void stop() { signal_state(Stopped); }
    void play() { signal_state(Playing); }
    void pause() { signal_state(Paused); }
    void quit() { signal_state(Quit); }
};

int main()
{
    Player player;
    player.play();
    sleep(3);
    player.pause();
    sleep(3);
    player.play();
    sleep(3);
    player.stop();
    sleep(3);
    player.play();
    sleep(3);
}

Output

I can't really demonstrate this. You'll have to run it and see how it works, and I invite you to toy with the states in main() as I have above. Do note, however, that once quit is invoked none of the other stated will be monitored. Setting the Quit state will shut down the monitor thread. For what its worth, a run of the above should look something like this:

1.2.3.4.5.6.7.8.9.10.11.12.13.14.15.16.17.18.19.20.21.22.23.24.25.26.27.28.29.30.
1.2.3.4.5.6.7.8.9.10.11.12.13.14.15.

with the first set of numbers dumped in two groups (1..15, then 16..30), as a result of playing, then pausing, then playing again. Then a stop is issued, followed by another play for a period of ~3 seconds. After that, the object self-destructs, and in doing so, sets the Quit state, and waits for the monitor to terminate.

Summary

Hopefully you get something out of this. If you find yourself trying to manage predicate state by manually latching and releasing mutexes, changes are you need a condition-variable design patter to facilitate detecting those changes.

Hope you get something out of it.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • Rather than having a `bQuit`, can you instead just return from `monitor()` in the `Quit` case? – Caleth Aug 03 '18 at 09:45
  • @Caleth Yes. I'm old school. single exit. Just a habit from the last three decades of doing this stuff. – WhozCraig Aug 03 '18 at 09:52
  • Thank you, I think this all makes sense now. I’m still pretty new to multithreading, I assume `std::condition_variable::wait()` blocks the thread until the condition is met (doesn’t internally loop or something)? Because the intent was to save that CPU usage (if that’s not possible I can live with it) – TheTrueJard Aug 03 '18 at 18:21
  • 1
    @TheJared802 [`wait` and it's siblings are documented](https://en.cppreference.com/w/cpp/thread/condition_variable). If you're not familiar with how pthreads work, it is very similar (and by similar, I mean identical). The most frequent mistake people make in using them is failing to realize they're just a signaling mechanism to announce change-in-predicate state. The actual predicate data itself is separate, and requires mutual exclusion on both read and write to ensure proper concurrency. People frequently try to use cvars as state in themselves; that's wrong. Read the link in this comment. – WhozCraig Aug 03 '18 at 19:01
  • 1
    @TheJared802 Lastly, at the risk of sounding self-gratifying, [**see this answer**](https://stackoverflow.com/questions/14924469/does-pthread-cond-waitcond-t-mutex-unlock-and-then-lock-the-mutex/14925150#14925150). It uses C pthreads, and is a very long winded answer to what was actually a simple question, but demonstrates in detail how a condition variable, mutex, and some predicate datum, can be used in concert for a couple of examples. It would be trivial to adapt to the thread-support library of C++, and probably worth doing as an exercise. – WhozCraig Aug 03 '18 at 19:06
  • After re-reading this I'm still not sure it solves the problem. If the `monitor()` function sees it's time to stop playing (at EOF), how would it signal that? That is, as if `main()` had called `player.stop()` – TheTrueJard Aug 03 '18 at 22:23
  • @TheJared802 The same way `main` would, by invoking `stop()`. If that decision is made within `monitor` to stop while holding the mutex, you can simply set the state directly: `state = STOPPED;`. Outside of holding the mutex, `stop()` would be invoked instead. I *think* that answers your question. – WhozCraig Aug 04 '18 at 00:47
-1
class CtLockCS
{
public:
    //--------------------------------------------------------------------------
    CtLockCS()      { ::InitializeCriticalSection(&m_cs); }
    //--------------------------------------------------------------------------
    ~CtLockCS()     { ::DeleteCriticalSection(&m_cs); }
    //--------------------------------------------------------------------------
    bool TryLock()  { return ::TryEnterCriticalSection(&m_cs) == TRUE; }
    //--------------------------------------------------------------------------
    void Lock()     { ::EnterCriticalSection(&m_cs); }
    //--------------------------------------------------------------------------
    void Unlock()   { ::LeaveCriticalSection(&m_cs); }
    //--------------------------------------------------------------------------
protected:
    CRITICAL_SECTION m_cs;
};


///////////////////////////////////////////////////////////////////////////////
// class CtLockMX - using mutex

class CtLockMX
{
public:
    //--------------------------------------------------------------------------
    CtLockMX(const TCHAR* nameMutex = 0)
        { m_mx = ::CreateMutex(0, FALSE, nameMutex); }
    //--------------------------------------------------------------------------
    ~CtLockMX()
        { if (m_mx) { ::CloseHandle(m_mx); m_mx = NULL; } }
    //--------------------------------------------------------------------------
    bool TryLock()
        {   return m_mx ? (::WaitForSingleObject(m_mx, 0) == WAIT_OBJECT_0) : false; }
    //--------------------------------------------------------------------------
    void Lock()
        {   if (m_mx)   { ::WaitForSingleObject(m_mx, INFINITE); }  }
    //--------------------------------------------------------------------------
    void Unlock()
        {   if (m_mx)   { ::ReleaseMutex(m_mx); }   }
    //--------------------------------------------------------------------------
protected:
    HANDLE  m_mx;
};


///////////////////////////////////////////////////////////////////////////////
// class CtLockSM - using semaphore

class CtLockSM
{
public:
    //--------------------------------------------------------------------------
    CtLockSM(int maxcnt)    { m_sm = ::CreateSemaphore(0, maxcnt, maxcnt, 0); }
    //--------------------------------------------------------------------------
    ~CtLockSM()             { ::CloseHandle(m_sm); }
    //--------------------------------------------------------------------------
    bool TryLock()          { return m_sm ? (::WaitForSingleObject(m_sm, 0) == WAIT_OBJECT_0) : false;  }
    //--------------------------------------------------------------------------
    void Lock()             { if (m_sm) { ::WaitForSingleObject(m_sm, INFINITE); }  }
    //--------------------------------------------------------------------------
    void Unlock()
    {
        if (m_sm){
            LONG prevcnt = 0;
            ::ReleaseSemaphore(m_sm, 1, &prevcnt);
        }
    }
    //--------------------------------------------------------------------------
protected:
    HANDLE  m_sm;
};