CODEWITHSUNDEEP
c++multithreading
Itsbananas
Itsbananas

Reputation: 611

Keeping a thread locked a long time

Edit 2: To be clear my problem is that I am struggling to write code to block and resume a thread to do some work (in this case, reading some files).

Edit 3: Added the function definition for void Loader::RequestToLoadRegion(Region &region).

I want to create a basic streaming system in my game. I need a thread to read game files that will be blocked most of the time and awaken from time to time. Here is how my current design works.

Everytime the player is approaching a region of the world that is not loaded in RAM, the game sends a request to the (unique) Loader object to load this region (the Loader class represents the streaming system). Each region has a list of files associated with it that will be read to create game data and load it into RAM.

The loading is accomplished in two steps:

The Loader object uses a Reader object to read all the files and push their content into the aforementioned queue. In the constructor of the Loader, the Reader object is constructed, which starts a thread that will be used to read the files. However, this reading thread will be blocked most of the time and only awaken by the Loader when this latter receives a request to load a new region.

At the start of each frame, the Loader will be given control and checks its current state, either loading or not. If it is in the loading state, then it will try to access the queue protected by a mutex. If it gets an item from the queue it will process it and give control back to the game. The next item in the queue will be processed on the next frame and so on until the loading of the region is complete.

I know how to implement this part using a mutex. However I have some troubles to keep to the reading thread blocked and to wake it when a loading request arrives. I've learnt how to use a std::condition_variable for a basic program and my current solution is the following:

// This is the function ran by the reading thread
// m_readingThread = std::thread(&Reader::Run, this);
void Reader::Run() {
  while (true) {
    {
      std::unique_lock<std::mutex> ul(m_canReadMutex);
      m_canReadCondVar.wait(ul, [this](){ return m_quit || m_canRead; });
      // Determine the cause of the wake up
      if (m_quit) { // Does the game wants to quit? We break out of
    break;      // the loop to reach the end of the Run() function
      }             // that way we can join() the reading thread.

      // If control reaches this line, it means the thread was awaken by
      // a load request and can read. The closing bracket "will run the destructtor"
      // of the std::unique_lock to unlock the mutex.
    }

    { // When control reaches this scope, it means m_canRead is true.
      m_loader.OnStartReading(); // will lock the mutex m_canReadMutex
      ReadFiles();
    }
  }// end-while (true)
}// end-function void Run()

void Reader::ReadFiles() {
  /* Read all the files and push data into the queue */
}

void Loader::OnStartReading() {
  m_canReadMutex.lock();
}

// This function unlocks the mutex and wake up the reading thread.
void Loader::RequestToLoadRegion(Region &region) {
  /* Get the list of files associated to the region */
  m_canReadMutex.unlock();
  m_reader.m_canRead = true; // Set the flag so the predicate in the reader's wait() evaluates to true
  m_canReadCondVar.notify_one(); // Wake up the reader thread
}

Upvotes: 0

Views: 522

Answers (2)

Anthony Williams
Anthony Williams

Reputation: 68561

std::unique_lock is very similar to std::lock_guard: it acquires the lock in the constructor and releases it in the destructor. The difference is that it provides additional facilities as well, one of which is that you can use it with std::condition_variable::wait.

When you call wait, then the library marks the thread as waiting, and then unlocks the mutex and blocks. When the condition variable unblocks (either because of a call to notify_one or notify_all or spuriously), it then re-locks the mutex before the call to wait returns. Thus the mutex is locked both before and after the wait call, but not during the wait call.

UPDATE:

OnStartReading locks the mutex directly, and has no corresponding unlock. This is really unusual: it means the mutex will be locked forever, from this point onwards.

I would expect onStartReading to use std::lock_guard to lock the mutex, to ensure it is unlocked afterwards. If you need it to be locked for the whole of ReadFiles, then OnStartReading will need to use unique_lock, and return it, or ReadFiles moved into OnStartReading.

FURTHER UPDATE:

The more code is added, the worse this looks :(

You cannot lock a mutex in one thread and unlock it another.

As a general rule, you should never call lock or unlock on a mutex directly. If you can, just use lock_guard, and contain the lock to a single scope. If you need to use the lock with a condition variable wait, use unique_lock. If you must lock in one scope, and unlock in another, use unique_lock and pass it as a parameter as a return value.

The call to m_canReadCondVar.wait will not return until either m_canRead is set, or m_quit is set, and it can acquire the mutex lock. If another thread holds the mutex (temporarily, because you should only hold the lock for a short time), then the wait cannot return.

RequestToLoadRegion is just wrong. It should set the flag, then unlock the mutex, not the other way round, otherwise there is a data race on m_canRead, and undefined behaviour. However, the mutex unlock here cannot pair with the lock in OnStartReading anyway unless they are in the same thread.

Upvotes: 2

Joseph Larson
Joseph Larson

Reputation: 9058

Your code is good. This is my standard code for waiting on a value:

void
WaitCondition::wait() {
    unique_lock<std::mutex> lock(myMutex);
    while (!value) {
        condVar.wait(lock);
    }
}

Waiting on the condVar actually frees the lock until the wait completes. So you're good.

(Edited with suggestion from Raymond.)

Upvotes: 0

Related Questions