Нужны отзывы о том, как сделать класс «потоко-безопасным»

В вашем случае у вас есть игра большого класса, которая требует синхронизации. Вы уже отметили проблему, при которой каждый метод синхронизируется, но один из них не может безопасно выполнять операции.

Если мы посмотрим на класс ThreadSafeGame, я думаю, что интерфейс для него можно улучшить, чтобы мы могли получить доступ к состоянию игры, только если мы находимся в синхронизированном режиме. Есть несколько способов сделать это. Один из способов - заставить getGame возвращать класс, который одновременно содержит и блокировку, и экземпляр. Вы определяете operator-> для этого класса, чтобы он возвращал Game *. Когда класс уничтожается, блокировка снимается.

В моих примерах используются некоторые функции C ++ 0x (лямбда-выражения, семантика перемещения, auto и decltype), но возможно сделать его совместимым с C ++ 98.

Я продемонстрирую другой способ сделать это, используя метод посещения:

template<typename TValue>
struct threadsafe_container : boost::noncopyable
{
   explicit threadsafe_container (TValue && value)
      :  m_value (std::move (value))
   {
   }

   // visit executes action when have the lock
   template<typename TAction>
   auto visit (TAction action) -> decltype (action (m_value))
   {
      boost::mutex::scope_lock lock (&m_mutex);

      TValue & value (m_value);

      return action (value);
   }

private:
   boost::mutex m_mutex;
   TValue m_value;
};

// Extra paranthesis necessary otherwise c++ interprets it as a function declaration
threadsafe_container<game> s_state ((ConstructAGameSomehow ())); 

void EndTheGame ()
{
   s_state.visit ([](game & state)
      {
         // In here we are synchronized
         while (!state.is_game_over ()) 
         { 
            state.drop_current_block (); 
         } 
      });
}

bool IsGameOver ()
{
   return s_state.visit ([](game & state) {return state.is_game_over ();});
}

И метод класса блокировки:

template<typename TValue>
struct threadsafe_container2 : boost::noncopyable
{
   struct lock : boost::noncopyable
   {
      lock (TValue * value, mutex * mtx)
         :  m_value  (value)
         ,  m_lock   (mtx)
      {
      }

      // Support move semantics
      lock (lock && l);

      TValue * get () const 
      {
         return m_value;
      }

      TValue * operator-> () const
      {
         return get ();
      }
   private:
      TValue *                   m_value;
      boost::mutex::scope_lock   m_lock;
   };

   explicit threadsafe_container2 (TValue && value)
      :  m_value (std::move (value))
   {
   }

   lock get ()
   {
      return lock (&m_value, &m_mutex);
   }

private:
   boost::mutex   m_mutex;
   TValue         m_value;
};

// Extra paranthesis necessary otherwise c++ interprets it as a function declaration
threadsafe_container2<game> s_state ((ConstructAGameSomehow ())); 

void EndTheGame ()
{
   auto lock = s_state2.get ();
   // In here we are synchronized
   while (!lock->is_game_over ()) 
   { 
      lock->drop_current_block ();   
   } 
}

bool IsGameOver ()
{
   auto lock = s_state2.get ();
   // In here we are synchronized
   reutrn lock->is_game_over ();
}

Но основная идея та же. Убедитесь, что мы можем получить доступ к состоянию игры только тогда, когда у нас есть блокировка. Конечно, это C ++, поэтому мы всегда можем найти способы нарушить правила, но процитируем Херба Саттера: Защитите от Мерфи, а не от Макиавелли, т.е. защитите себя от ошибок, а не от программистов, которые намереваются нарушить правила (они всегда найдут способ сделать это)

Теперь перейдем ко второй части комментария:

Крупнозернистая блокировка против мелкозернистой блокировки? Крупномасштабную блокировку довольно легко реализовать, но она страдает от проблем с производительностью, мелкомасштабную блокировку очень сложно реализовать, но она может иметь лучшую производительность.

Я бы сказал; сделайте все возможное, чтобы избежать блокировки вообще. Я не имею в виду; скрестим пальцы и надеюсь, что не получу условия гонки. Я имею в виду структуру вашей программы так, чтобы только один поток управлял изменяемым состоянием и изолировал это изменяемое состояние, чтобы оно не могло быть изменено по ошибке несколькими потоками.

В вашем случае у вас есть поток ввода, принимающий вводимые пользователем данные и обновляющий состояние. Один поток обновляет состояние игры по таймеру.

А что насчет входящего потока, принимающего состояние пользователя, он отправляет сообщение в поток диспетчера состояний игры, в котором говорится: : «Это то, что сделал пользователь». Затем поток состояния игры потребляет сообщения и действует соответствующим образом. Таким образом, состояние игры доступно только этому потоку, и не может возникнуть никаких условий гонки и тупиковых блокировок.

Иногда это называют «шаблоном активного объекта».

Читатели оповещения говорят: Но ведь очередь сообщений должна быть потокобезопасной! Это правда, но очередь сообщений сравнительно тривиальна, чтобы сделать потокобезопасным.

ИМО, этот шаблон является одним из наиболее важных для создания поддерживаемых параллельных проектов.