From c8594e8e7be98243a5d0a71b656a2a7b8ca6cb0e Mon Sep 17 00:00:00 2001 From: lotodore Date: Sun, 7 Jun 2009 23:25:13 +0000 Subject: [PATCH] Canceling asio timers has bad semantics, and causes loads of trouble. Therefore, cancelled timers are simply marked as such, and the asio callback is silently ignored. This fixes several crashes, if players were removed from a game due to an error, because some timers were not cancelled. *sigh* This one took a lot of nerve. --- src/core/common/timermanager.cpp | 37 +++++++++++++-------------- src/core/timermanager.h | 7 ++--- src/net/common/senderhelper.cpp | 4 +-- src/net/common/serveraccepthelper.cpp | 4 +-- src/net/common/servergame.cpp | 14 +++++++--- src/net/common/servergamestate.cpp | 16 ++++++------ src/net/common/serverlobbythread.cpp | 11 ++++---- src/net/serveraccepthelper.h | 2 +- src/net/servergame.h | 3 +++ src/net/serverlobbythread.h | 2 +- 10 files changed, 56 insertions(+), 44 deletions(-) diff --git a/src/core/common/timermanager.cpp b/src/core/common/timermanager.cpp index ab5d2195..cc781bd3 100644 --- a/src/core/common/timermanager.cpp +++ b/src/core/common/timermanager.cpp @@ -35,31 +35,30 @@ TimerManager::RegisterTimer(unsigned timeoutMsec, boost::function timerH boost::recursive_mutex::scoped_lock lock(m_timerMutex); // Use a unique id for each timer. unsigned id = GetNextTimerId(); - TimerData data; - data.timer.reset( + boost::shared_ptr data(new TimerData); + data->timer.reset( new boost::asio::deadline_timer(*m_ioService, boost::posix_time::milliseconds(timeoutMsec))); - data.userHandler = timerHandler; - data.durationMsec = timeoutMsec; - data.autoRestart = autoRestart; - data.timer->async_wait(boost::bind(&TimerManager::Handler, boost::asio::placeholders::error, data)); + data->userHandler = timerHandler; + data->durationMsec = timeoutMsec; + data->autoRestart = autoRestart; + data->cancelled = false; + data->timer->async_wait(boost::bind(&TimerManager::Handler, boost::asio::placeholders::error, data)); m_timerMap.insert(TimerMap::value_type(id, data)); return id; } bool -TimerManager::RestartTimer(unsigned timerId, unsigned timeoutMsec, boost::function timerHandler) +TimerManager::AddTimer(unsigned timerId, unsigned timeoutMsec, boost::function timerHandler) { boost::recursive_mutex::scoped_lock lock(m_timerMutex); bool restarted = false; TimerMap::iterator pos = m_timerMap.find(timerId); if (pos != m_timerMap.end()) { - pos->second.userHandler = timerHandler; - boost::shared_ptr timer(pos->second.timer); - timer->cancel(); - timer->expires_from_now(boost::posix_time::milliseconds(timeoutMsec)); - timer->async_wait(boost::bind(&TimerManager::Handler, boost::asio::placeholders::error, pos->second)); + pos->second->userHandler = timerHandler; + pos->second->timer->expires_from_now(boost::posix_time::milliseconds(timeoutMsec)); + pos->second->timer->async_wait(boost::bind(&TimerManager::Handler, boost::asio::placeholders::error, pos->second)); restarted = true; } return restarted; @@ -74,7 +73,7 @@ TimerManager::UnregisterTimer(unsigned timerId) TimerMap::iterator pos = m_timerMap.find(timerId); if (pos != m_timerMap.end()) { - pos->second.timer->cancel(); + pos->second->cancelled = true; m_timerMap.erase(pos); unregistered = true; } @@ -82,15 +81,15 @@ TimerManager::UnregisterTimer(unsigned timerId) } void -TimerManager::Handler(boost::system::error_code ec, TimerManager::TimerData data) +TimerManager::Handler(const boost::system::error_code &ec, boost::shared_ptr data) { - if (!ec) + if (!ec && data && !data->cancelled) { - data.userHandler(); - if (data.autoRestart) + data->userHandler(); + if (data->autoRestart) { - data.timer->expires_from_now(boost::posix_time::milliseconds(data.durationMsec)); - data.timer->async_wait(boost::bind(&TimerManager::Handler, boost::asio::placeholders::error, data)); + data->timer->expires_from_now(boost::posix_time::milliseconds(data->durationMsec)); + data->timer->async_wait(boost::bind(&TimerManager::Handler, boost::asio::placeholders::error, data)); } } } diff --git a/src/core/timermanager.h b/src/core/timermanager.h index 8be9ab73..cb66a915 100644 --- a/src/core/timermanager.h +++ b/src/core/timermanager.h @@ -32,7 +32,7 @@ public: TimerManager(boost::shared_ptr ioService); unsigned RegisterTimer(unsigned timeoutMsec, boost::function timerHandler, bool autoRestart = false); - bool RestartTimer(unsigned timerId, unsigned timeoutMsec, boost::function timerHandler); + bool AddTimer(unsigned timerId, unsigned timeoutMsec, boost::function timerHandler); bool UnregisterTimer(unsigned timerId); protected: @@ -43,10 +43,11 @@ protected: boost::function userHandler; unsigned durationMsec; bool autoRestart; + bool cancelled; }; - static void Handler(boost::system::error_code ec, TimerData data); - typedef std::map TimerMap; + static void Handler(const boost::system::error_code &ec, boost::shared_ptr data); + typedef std::map > TimerMap; unsigned GetNextTimerId(); diff --git a/src/net/common/senderhelper.cpp b/src/net/common/senderhelper.cpp index 8ab7323a..c86532f0 100644 --- a/src/net/common/senderhelper.cpp +++ b/src/net/common/senderhelper.cpp @@ -48,7 +48,7 @@ class SendDataManager : public boost::enable_shared_from_this { } - void HandleWrite(const boost::system::error_code& error); + void HandleWrite(const boost::system::error_code &error); void AsyncSendNextPacket(bool handlerMode = false); @@ -61,7 +61,7 @@ class SendDataManager : public boost::enable_shared_from_this void -SendDataManager::HandleWrite(const boost::system::error_code& error) +SendDataManager::HandleWrite(const boost::system::error_code &error) { // TODO error handling AsyncSendNextPacket(true); diff --git a/src/net/common/serveraccepthelper.cpp b/src/net/common/serveraccepthelper.cpp index 009cc6e9..da9245c2 100644 --- a/src/net/common/serveraccepthelper.cpp +++ b/src/net/common/serveraccepthelper.cpp @@ -24,7 +24,7 @@ #include -#define NET_SERVER_LISTEN_BACKLOG 5 +#define NET_SERVER_LISTEN_BACKLOG 20 using namespace std; using boost::asio::ip::tcp; @@ -96,7 +96,7 @@ ServerAcceptHelper::InternalListen(unsigned serverPort, bool ipv6, bool sctp) void ServerAcceptHelper::HandleAccept(boost::shared_ptr acceptedSocket, - const boost::system::error_code& error) + const boost::system::error_code &error) { if (!error) { diff --git a/src/net/common/servergame.cpp b/src/net/common/servergame.cpp index 7dfc2b89..0ce56764 100644 --- a/src/net/common/servergame.cpp +++ b/src/net/common/servergame.cpp @@ -46,7 +46,16 @@ ServerGame::ServerGame(ServerLobbyThread &lobbyThread, u_int32_t id, const strin LOG_VERBOSE("Game object " << GetId() << " created."); m_receiver.reset(new ReceiverHelper); +} +ServerGame::~ServerGame() +{ + LOG_VERBOSE("Game object " << GetId() << " destructed."); +} + +void +ServerGame::Init() +{ m_voteKickTimerId = GetLobbyThread().GetTimerManager().RegisterTimer( SERVER_CHECK_VOTE_KICK_INTERVAL_MSEC, boost::bind(&ServerGame::TimerVoteKick, this), @@ -55,12 +64,11 @@ ServerGame::ServerGame(ServerLobbyThread &lobbyThread, u_int32_t id, const strin SetState(SERVER_INITIAL_STATE::Instance()); } -ServerGame::~ServerGame() +void +ServerGame::Exit() { GetLobbyThread().GetTimerManager().UnregisterTimer(m_voteKickTimerId); GetLobbyThread().GetTimerManager().UnregisterTimer(m_stateTimerId); - - LOG_VERBOSE("Game object " << GetId() << " destructed."); } u_int32_t diff --git a/src/net/common/servergamestate.cpp b/src/net/common/servergamestate.cpp index b09da84f..f90ef46e 100644 --- a/src/net/common/servergamestate.cpp +++ b/src/net/common/servergamestate.cpp @@ -400,7 +400,7 @@ ServerGameStateInit::TimerAdminWarning(ServerGame &server) server.GetLobbyThread().GetSender().Send(session.sessionData, warning); } // Start timeout timer. - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), SERVER_GAME_ADMIN_WARNING_REMAINING_SEC * 1000, boost::bind(&ServerGameStateInit::TimerAdminTimeout, this, boost::ref(server))); @@ -701,7 +701,7 @@ ServerGameStateHand::TimerLoop(ServerGame &server) server.SendToAllPlayers(allIn, SessionData::Game); curGame.getCurrentHand()->setCardsShown(true); - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), SERVER_SHOW_CARDS_DELAY_SEC * 1000, boost::bind(&ServerGameStateHand::TimerLoop, this, boost::ref(server))); @@ -710,7 +710,7 @@ ServerGameStateHand::TimerLoop(ServerGame &server) { SendNewRoundCards(server, curGame, newRound); - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), GetDealCardsDelaySec(server) * 1000, boost::bind(&ServerGameStateHand::TimerLoop, this, boost::ref(server))); @@ -741,7 +741,7 @@ ServerGameStateHand::TimerLoop(ServerGame &server) // If the player is computer controlled, let the engine act. if (curPlayer->getMyType() == PLAYER_TYPE_COMPUTER) { - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), SERVER_COMPUTER_ACTION_DELAY_SEC * 1000, boost::bind(&ServerGameStateHand::TimerComputerAction, this, boost::ref(server))); @@ -750,7 +750,7 @@ ServerGameStateHand::TimerLoop(ServerGame &server) else if (!server.GetSessionManager().IsPlayerConnected(curPlayer->getMyName())) { PerformPlayerAction(server, curPlayer, PLAYER_ACTION_FOLD, 0); - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), SERVER_LOOP_DELAY_MSEC, boost::bind(&ServerGameStateHand::TimerLoop, this, boost::ref(server))); @@ -833,14 +833,14 @@ ServerGameStateHand::TimerLoop(ServerGame &server) else if (playersWithCash.size() == 1) { // View a dialog for a new game - delayed. - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), SERVER_DELAY_NEXT_GAME_SEC * 1000, boost::bind(&ServerGameStateHand::TimerNextGame, this, boost::ref(server))); } else { - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), SERVER_DELAY_NEXT_HAND_SEC * 1000, boost::bind(&ServerGameStateHand::TimerNextHand, this, boost::ref(server))); @@ -855,7 +855,7 @@ ServerGameStateHand::TimerShowCards(ServerGame &server) Game &curGame = server.GetGame(); SendNewRoundCards(server, curGame, curGame.getCurrentHand()->getCurrentRound()); - server.GetLobbyThread().GetTimerManager().RestartTimer( + server.GetLobbyThread().GetTimerManager().AddTimer( server.GetStateTimerId(), GetDealCardsDelaySec(server) * 1000, boost::bind(&ServerGameStateHand::TimerLoop, this, boost::ref(server))); diff --git a/src/net/common/serverlobbythread.cpp b/src/net/common/serverlobbythread.cpp index 7547e168..d5ea02d0 100644 --- a/src/net/common/serverlobbythread.cpp +++ b/src/net/common/serverlobbythread.cpp @@ -563,7 +563,7 @@ ServerLobbyThread::RegisterTimers() } void -ServerLobbyThread::HandleRead(SessionId sessionId, const boost::system::error_code& error, size_t bytesRead) +ServerLobbyThread::HandleRead(SessionId sessionId, const boost::system::error_code &error, size_t bytesRead) { // Find the session. SessionWrapper session = m_sessionManager.GetSessionById(sessionId); @@ -949,11 +949,11 @@ ServerLobbyThread::HandleNetPacketCreateGame(SessionWrapper session, const NetPa session.playerData->GetUniqueId(), GetGui(), m_playerConfig)); - - MoveSessionToGame(*game, session); - + game->Init(); // Add game to list of games. InternalAddGame(game); + + MoveSessionToGame(*game, session); } void @@ -1043,7 +1043,7 @@ ServerLobbyThread::TimerRemoveGame() ++next; boost::shared_ptr tmpGame = i->second; if (!tmpGame->GetSessionManager().HasSessions()) - InternalRemoveGame(tmpGame); // This will delete the entry from the map. + InternalRemoveGame(tmpGame); // This will delete the game. i = next; } } @@ -1154,6 +1154,7 @@ ServerLobbyThread::InternalRemoveGame(boost::shared_ptr game) // Remove all sessions left in the game. game->ResetComputerPlayerList(); game->RemoveAllSessions(); + game->Exit(); // Notify all players. boost::shared_ptr packet = CreateNetPacketGameListUpdate(game->GetId(), GAME_MODE_CLOSED); m_sessionManager.SendLobbyMsgToAllSessions(GetSender(), packet, SessionData::Established); diff --git a/src/net/serveraccepthelper.h b/src/net/serveraccepthelper.h index aa52dc9c..c4bd57bc 100644 --- a/src/net/serveraccepthelper.h +++ b/src/net/serveraccepthelper.h @@ -43,7 +43,7 @@ protected: void InternalListen(unsigned serverPort, bool ipv6, bool sctp); void HandleAccept(boost::shared_ptr acceptedSocket, - const boost::system::error_code& error); + const boost::system::error_code &error); ServerCallback &GetCallback(); diff --git a/src/net/servergame.h b/src/net/servergame.h index 7517c7da..6d0662f2 100644 --- a/src/net/servergame.h +++ b/src/net/servergame.h @@ -43,6 +43,9 @@ public: ServerLobbyThread &lobbyThread, u_int32_t id, const std::string &name, const std::string &pwd, const GameData &gameData, unsigned adminPlayerId, GuiInterface &gui, ConfigFile *playerConfig); virtual ~ServerGame(); + void Init(); + void Exit(); + u_int32_t GetId() const; const std::string &GetName() const; diff --git a/src/net/serverlobbythread.h b/src/net/serverlobbythread.h index 2a7793af..04ddac40 100644 --- a/src/net/serverlobbythread.h +++ b/src/net/serverlobbythread.h @@ -114,7 +114,7 @@ protected: virtual void Main(); void RegisterTimers(); - void HandleRead(SessionId sessionId, const boost::system::error_code& error, size_t bytesRead); + void HandleRead(SessionId sessionId, const boost::system::error_code &error, size_t bytesRead); void HandlePacket(SessionWrapper session, boost::shared_ptr packet); void HandleNetPacketInit(SessionWrapper session, const NetPacketInit &tmpPacket); void HandleNetPacketAvatarHeader(SessionWrapper session, const NetPacketAvatarHeader &tmpPacket);