I'm currently learning how to do multithreading in C++. One of my learning projects is a Tetris game. In this project I have a Game class that contains all game state data. It has methods for moving the block around and a few other things. This object will be accessed by the user (who will use the arrow keys to move the block, from the main thread) and at the same time a threaded timer is implementing the gravity on the active block (periodically lowering it).
At first I thought that I could make the Game class thread safe by adding a mutex member variable and lock it inside each method call. But problem with this is that it only protects individual method calls, not changes that involve multiple method calls. For example:
// This is not thread-safe.
while (!game.isGameOver())
{
game.dropCurrentBlock();
}
One solution that I tried is adding an accessor method for the mutex variable to lock it also from the outside:
// Extra scope added to limit the lifetime of the scoped_lock.
{
// => deadlock, unless a recursive mutex is used
boost::mutex::scoped_lock lock(game.getMutex());
while (!game.isGameOver())
{
game.dropCurrentBlock();
}
}
However, this will deadlock unless a recursive mutex is used. Now, looking at some posts on StackOverflow, there seems to be a majority that strongly disapproves the use of recursive mutexes.
But if recursive mutexes are a non-option, doesn't that mean that it becomes impossible to create a thread-safe class (that supports coordinated changes)?
The only valid solution seems to be to never lock the mutex inside the method calls, and instead always rely on the user to do the locking from the outside.
However, if that is the case, then wouldn't it be better to simply leave the Game class as it is, and create a wrapper class that pairs a Game object with a mutex?
Update
I gave the wrapper idea a try and created a class called ThreadSafeGame (cpp) that looks like this:
class ThreadSafeGame
{
public:
ThreadSafeGame(std::auto_ptr<Game> inGame) : mGame(inGame.release) {}
const Game * getGame() const
{ return mGame.get(); }
Game * getGame()
{ return mGame.get(); }
boost::mutex & getMutex() const
{ return mMutex; }
private:
boost::scoped_ptr<Game> mGame;
mutable boost::mutex mMutex;
};
// Usage example, assuming "threadSafeGame" is pointer to a ThreadSafeGame object.
{
// First lock the game object.
boost::mutex::scoped_lock lock(threadSafeGame->getMutex());
// Then access it.
Game * game = threadSafeGame->getGame();
game->move(Direction_Down);
}
It has the same drawback in that it depends on the user to lock the mutex from the outside. But apart from that this seems like a workable solution to me.
Am I doing it right?
whilethat includes all code but thesleep. You do not really want to sleep while holding the lock. – Slick