games/vcmi
1
0
Fork 0
mirror of https://github.com/vcmi/vcmi.git synced 2024-11-28 08:48:48 +02:00
Code Issues Releases Activity

Added semi-workaround method for network thread shutdown:

Browse Source 
Currently closing game while network thread is waiting for something is
very bug-prone, since network thread may resume during shutdown and
access partially destroyed client state.

Now if exit has been requested, the very first step would be semi-
graceful shutdown of network thread (via exception throwing). This may
in theory skip some cleanup in non-RAII code, but since game is shutting
down this does not matters much.

This logic applies to:
- shutting down while network thread is waiting for dialogs
- shuttind down while network thread waiting for animations in combat
This commit is contained in:
Ivan Savenko 2024-05-18 11:04:10 +00:00
parent 0dfa781655
commit 9bfe000724
27 changed files with 163 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AI/Nullkiller/AIGateway.h
View File

@ -21,7 +21,6 @@
#include "../../lib/CTownHandler.h"
#include "../../lib/mapObjects/MiscObjects.h"
#include "../../lib/spells/CSpellHandler.h"
#include "../../lib/CondSh.h"
#include "Pathfinding/AIPathfinder.h"
#include "Engine/Nullkiller.h"

1
AI/VCAI/VCAI.h
View File

@ -23,7 +23,6 @@
#include "../../lib/CTownHandler.h"
#include "../../lib/mapObjects/MiscObjects.h"
#include "../../lib/spells/CSpellHandler.h"
#include "../../lib/CondSh.h"
#include "Pathfinding/AIPathfinder.h"
VCMI_LIB_NAMESPACE_BEGIN

5
client/CMT.cpp
View File

@ -442,6 +442,8 @@ static void mainLoop()
[[noreturn]] static void quitApplication()
{
CSH->endNetwork();
if(!settings["session"]["headless"].Bool())
{
if(CSH->client)
@ -450,6 +452,8 @@ static void mainLoop()
GH.windows().clear();
}
vstd::clear_pointer(CSH);
CMM.reset();
if(!settings["session"]["headless"].Bool())
@ -473,7 +477,6 @@ static void mainLoop()
vstd::clear_pointer(graphics);
}
vstd::clear_pointer(CSH);
vstd::clear_pointer(VLC);
// sometimes leads to a hang. TODO: investigate

1
client/CMakeLists.txt
View File

@ -365,6 +365,7 @@ set(client_HEADERS
Client.h
ClientCommandManager.h
ClientNetPackVisitors.h
ConditionalWait.h
HeroMovementController.h
GameChatHandler.h
LobbyClientNetPackVisitors.h

24
client/CPlayerInterface.cpp
View File

@ -72,7 +72,6 @@
#include "../lib/CStopWatch.h"
#include "../lib/CThreadHelper.h"
#include "../lib/CTownHandler.h"
#include "../lib/CondSh.h"
#include "../lib/GameConstants.h"
#include "../lib/RoadHandler.h"
#include "../lib/StartInfo.h"
@ -140,7 +139,7 @@ CPlayerInterface::CPlayerInterface(PlayerColor Player):
battleInt = nullptr;
castleInt = nullptr;
makingTurn = false;
showingDialog = new CondSh<bool>(false);
showingDialog = new ConditionalWait();
cingconsole = new CInGameConsole();
firstCall = 1; //if loading will be overwritten in serialize
autosaveCount = 0;
@ -1005,7 +1004,7 @@ void CPlayerInterface::showInfoDialog(const std::string &text, const std::vector
if (makingTurn && GH.windows().count() > 0 && LOCPLINT == this)
{
CCS->soundh->playSound(static_cast<soundBase::soundID>(soundID));
showingDialog->set(true);
showingDialog->setBusy();
movementController->requestMovementAbort(); // interrupt movement to show dialog
GH.windows().pushWindow(temp);
}
@ -1028,7 +1027,7 @@ void CPlayerInterface::showInfoDialogAndWait(std::vector<Component> & components
void CPlayerInterface::showYesNoDialog(const std::string &text, CFunctionList<void()> onYes, CFunctionList<void()> onNo, const std::vector<std::shared_ptr<CComponent>> & components)
{
movementController->requestMovementAbort();
LOCPLINT->showingDialog->setn(true);
LOCPLINT->showingDialog->setBusy();
CInfoWindow::showYesNoDialog(text, components, onYes, onNo, playerID);
}
@ -1217,7 +1216,7 @@ void CPlayerInterface::loadGame( BinaryDeserializer & h )
void CPlayerInterface::moveHero( const CGHeroInstance *h, const CGPath& path )
{
assert(h);
assert(!showingDialog->get());
assert(!showingDialog->isBusy());
assert(dialogs.empty());
LOG_TRACE(logGlobal);
@ -1227,7 +1226,7 @@ void CPlayerInterface::moveHero( const CGHeroInstance *h, const CGPath& path )
return; //can't find hero
//It shouldn't be possible to move hero with open dialog (or dialog waiting in bg)
if (showingDialog->get() || !dialogs.empty())
if (showingDialog->isBusy() || !dialogs.empty())
return;
if (localState->isHeroSleeping(h))
@ -1395,9 +1394,7 @@ void CPlayerInterface::waitWhileDialog()
}
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
boost::unique_lock<boost::mutex> un(showingDialog->mx);
while(showingDialog->data)
showingDialog->cond.wait(un);
showingDialog->waitWhileBusy();
}
void CPlayerInterface::showShipyardDialog(const IShipyard *obj)
@ -1502,9 +1499,9 @@ void CPlayerInterface::update()
return;
//if there are any waiting dialogs, show them
if ((CSH->howManyPlayerInterfaces() <= 1 || makingTurn) && !dialogs.empty() && !showingDialog->get())
if ((CSH->howManyPlayerInterfaces() <= 1 || makingTurn) && !dialogs.empty() && !showingDialog->isBusy())
{
showingDialog->set(true);
showingDialog->setBusy();
GH.windows().pushWindow(dialogs.front());
dialogs.pop_front();
}
@ -1516,6 +1513,11 @@ void CPlayerInterface::update()
GH.windows().simpleRedraw();
}
void CPlayerInterface::endNetwork()
{
showingDialog->requestTermination();
}
int CPlayerInterface::getLastIndex( std::string namePrefix)
{
using namespace boost::filesystem;

5
client/CPlayerInterface.h
View File

@ -25,7 +25,7 @@ struct CGPath;
class CCreatureSet;
class CGObjectInstance;
struct UpgradeInfo;
template <typename T> struct CondSh;
class ConditionalWait;
struct CPathsInfo;
VCMI_LIB_NAMESPACE_END
@ -74,7 +74,7 @@ public: // TODO: make private
std::unique_ptr<PlayerLocalState> localState;
//minor interfaces
CondSh<bool> *showingDialog; //indicates if dialog box is displayed
ConditionalWait * showingDialog; //indicates if dialog box is displayed
bool makingTurn; //if player is already making his turn
@ -202,6 +202,7 @@ public: // public interface for use by client via LOCPLINT access
void proposeLoadingGame();
void performAutosave();
void gamePause(bool pause);
void endNetwork();
///returns true if all events are processed internally
bool capturedAllEvents();

21
client/CServerHandler.cpp
View File

@ -29,6 +29,7 @@
#include "../lib/CConfigHandler.h"
#include "../lib/CGeneralTextHandler.h"
#include "ConditionalWait.h"
#include "../lib/CThreadHelper.h"
#include "../lib/StartInfo.h"
#include "../lib/TurnTimerInfo.h"
@ -131,6 +132,17 @@ CServerHandler::~CServerHandler()
}
}
void CServerHandler::endNetwork()
{
if (client)
client->endNetwork();
networkHandler->stop();
{
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
threadNetwork.join();
}
}
CServerHandler::CServerHandler()
: networkHandler(INetworkHandler::createHandler())
, lobbyClient(std::make_unique<GlobalLobbyClient>())
@ -158,7 +170,14 @@ void CServerHandler::threadRunNetwork()
{
logGlobal->info("Starting network thread");
setThreadName("runNetwork");
networkHandler->run();
try {
networkHandler->run();
}
catch (const TerminationRequestedException & e)
{
logGlobal->info("Terminating network thread");
return;
}
logGlobal->info("Ending network thread");
}

2
client/CServerHandler.h
View File

@ -13,7 +13,6 @@
#include "../lib/network/NetworkInterface.h"
#include "../lib/StartInfo.h"
#include "../lib/CondSh.h"
VCMI_LIB_NAMESPACE_BEGIN
@ -208,6 +207,7 @@ public:
void startGameplay(VCMI_LIB_WRAP_NAMESPACE(CGameState) * gameState = nullptr);
void showHighScoresAndEndGameplay(PlayerColor player, bool victory);
void endNetwork();
void endGameplay();
void restartGameplay();
void startCampaignScenario(HighScoreParameter param, std::shared_ptr<CampaignState> cs = {});

13
client/Client.cpp
View File

@ -346,6 +346,19 @@ void CClient::save(const std::string & fname)
sendRequest(&save_game, PlayerColor::NEUTRAL);
}
void CClient::endNetwork()
{
if (CPlayerInterface::battleInt)
CPlayerInterface::battleInt->endNetwork();
for(auto & i : playerint)
{
auto interface = std::dynamic_pointer_cast<CPlayerInterface>(i.second);
if (interface)
interface->endNetwork();
}
}
void CClient::endGame()
{
#if SCRIPTING_ENABLED

1
client/Client.h
View File

@ -135,6 +135,7 @@ public:
void serialize(BinaryDeserializer & h);
void save(const std::string & fname);
void endNetwork();
void endGame();
void initMapHandler();

8
client/ClientCommandManager.cpp
View File

@ -179,11 +179,6 @@ void ClientCommandManager::handleRedrawCommand()
GH.windows().totalRedraw();
}
void ClientCommandManager::handleNotDialogCommand()
{
LOCPLINT->showingDialog->setn(false);
}
void ClientCommandManager::handleTranslateGameCommand()
{
std::map<std::string, std::map<std::string, std::string>> textsByMod;
@ -584,9 +579,6 @@ void ClientCommandManager::processCommand(const std::string & message, bool call
else if(commandName == "redraw")
handleRedrawCommand();
else if(commandName == "not dialog")
handleNotDialogCommand();
else if(message=="translate" || message=="translate game")
handleTranslateGameCommand();

3
client/ClientCommandManager.h
View File

@ -45,9 +45,6 @@ class ClientCommandManager //take mantis #2292 issue about account if thinking a
// Redraw the current screen
void handleRedrawCommand();
// Set the state indicating if dialog box is active to "no"
void handleNotDialogCommand();
// Extracts all translateable game texts into Translation directory, separating files on per-mod basis
void handleTranslateGameCommand();

77
client/ConditionalWait.h Normal file
View File

@ -0,0 +1,77 @@
/*
* ConditionalWait.h, part of VCMI engine
*
* Authors: listed in file AUTHORS in main folder
*
* License: GNU General Public License v2.0 or later
* Full text of license available in license.txt file, in main folder
*
*/
#pragma once
#include <condition_variable>
VCMI_LIB_NAMESPACE_BEGIN
class TerminationRequestedException : public std::exception
{
public:
using exception::exception;
const char* what() const noexcept override
{
return "Thread termination requested";
}
};
class ConditionalWait
{
bool isBusyValue = false;
bool isTerminating = false;
std::condition_variable cond;
std::mutex mx;
void set(bool value)
{
boost::unique_lock<std::mutex> lock(mx);
isBusyValue = value;
}
public:
ConditionalWait() = default;
void setBusy()
{
set(true);
}
void setFree()
{
set(false);
cond.notify_all();
}
void requestTermination()
{
isTerminating = true;
setFree();
}
bool isBusy()
{
std::unique_lock<std::mutex> lock(mx);
return isBusyValue;
}
void waitWhileBusy()
{
std::unique_lock<std::mutex> un(mx);
while(isBusyValue)
cond.wait(un);
if (isTerminating)
throw TerminationRequestedException();
}
};
VCMI_LIB_NAMESPACE_END

4
client/HeroMovementController.cpp
View File

@ -22,7 +22,7 @@
#include "../CCallback.h"
#include "../lib/CondSh.h"
#include "ConditionalWait.h"
#include "../lib/pathfinder/CGPathNode.h"
#include "../lib/mapObjects/CGHeroInstance.h"
#include "../lib/networkPacks/PacksForClient.h"
@ -237,7 +237,7 @@ void HeroMovementController::onMoveHeroApplied()
assert(currentlyMovingHero);
const auto * hero = currentlyMovingHero;
bool canMove = LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0 && !LOCPLINT->showingDialog->get();
bool canMove = LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0 && !LOCPLINT->showingDialog->isBusy();
bool wantStop = stoppingMovement;
bool canStop = !canMove || canHeroStopAtNode(LOCPLINT->localState->getPath(hero).currNode());

16
client/battle/BattleInterface.cpp
View File

@ -38,7 +38,6 @@
#include "../../lib/CConfigHandler.h"
#include "../../lib/CGeneralTextHandler.h"
#include "../../lib/CHeroHandler.h"
#include "../../lib/CondSh.h"
#include "../../lib/gameState/InfoAboutArmy.h"
#include "../../lib/mapObjects/CGTownInstance.h"
#include "../../lib/networkPacks/PacksForClientBattle.h"
@ -95,7 +94,7 @@ BattleInterface::BattleInterface(const BattleID & battleID, const CCreatureSet *
obstacleController.reset(new BattleObstacleController(*this));
adventureInt->onAudioPaused();
ongoingAnimationsState.set(true);
ongoingAnimationsState.setBusy();
GH.windows().pushWindow(windowObject);
windowObject->blockUI(true);
@ -744,6 +743,11 @@ void BattleInterface::castThisSpell(SpellID spellID)
actionsController->castThisSpell(spellID);
}
void BattleInterface::endNetwork()
{
ongoingAnimationsState.requestTermination();
}
void BattleInterface::executeStagedAnimations()
{
EAnimationEvents earliestStage = EAnimationEvents::COUNT;
@ -775,19 +779,19 @@ void BattleInterface::executeAnimationStage(EAnimationEvents event)
void BattleInterface::onAnimationsStarted()
{
ongoingAnimationsState.setn(true);
ongoingAnimationsState.setBusy();
}
void BattleInterface::onAnimationsFinished()
{
ongoingAnimationsState.setn(false);
ongoingAnimationsState.setFree();
}
void BattleInterface::waitForAnimations()
{
{
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
ongoingAnimationsState.waitUntil(false);
ongoingAnimationsState.waitWhileBusy();
}
assert(!hasAnimations());
@ -802,7 +806,7 @@ void BattleInterface::waitForAnimations()
bool BattleInterface::hasAnimations()
{
return ongoingAnimationsState.get();
return ongoingAnimationsState.isBusy();
}
void BattleInterface::checkForAnimations()

5
client/battle/BattleInterface.h
View File

@ -12,7 +12,7 @@
#include "BattleConstants.h"
#include "../gui/CIntObject.h"
#include "../../lib/spells/CSpellHandler.h" //CSpell::TAnimation
#include "../../lib/CondSh.h"
#include "../ConditionalWait.h"
VCMI_LIB_NAMESPACE_BEGIN
@ -99,7 +99,7 @@ class BattleInterface
};
/// Conditional variables that are set depending on ongoing animations on the battlefield
CondSh<bool> ongoingAnimationsState;
ConditionalWait ongoingAnimationsState;
/// List of events that are waiting to be triggered
std::vector<AwaitingAnimationEvents> awaitingEvents;
@ -186,6 +186,7 @@ public:
void setBattleQueueVisibility(bool visible);
void setStickyHeroWindowsVisibility(bool visible);
void endNetwork();
void executeStagedAnimations();
void executeAnimationStage( EAnimationEvents event);
void onAnimationsStarted();

6
client/battle/BattleInterfaceClasses.cpp
View File

@ -52,7 +52,7 @@
#include "../../lib/CTownHandler.h"
#include "../../lib/CHeroHandler.h"
#include "../../lib/StartInfo.h"
#include "../../lib/CondSh.h"
#include "../ConditionalWait.h"
#include "../../lib/mapObjects/CGTownInstance.h"
#include "../../lib/networkPacks/PacksForClientBattle.h"
#include "../../lib/TextOperations.h"
@ -771,7 +771,7 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
void BattleResultWindow::activate()
{
owner.showingDialog->set(true);
owner.showingDialog->setBusy();
CIntObject::activate();
}
@ -864,7 +864,7 @@ void BattleResultWindow::buttonPressed(int button)
//Result window and battle interface are gone. We requested all dialogs to be closed before opening the battle,
//so we can be sure that there is no dialogs left on GUI stack.
intTmp.showingDialog->setn(false);
intTmp.showingDialog->setFree();
CCS->videoh->close();
}

2
client/battle/BattleStacksController.cpp
View File

@ -39,7 +39,7 @@
#include "../../lib/battle/BattleHex.h"
#include "../../lib/CRandomGenerator.h"
#include "../../lib/CStack.h"
#include "../../lib/CondSh.h"
#include "../ConditionalWait.h"
#include "../../lib/TextOperations.h"
static void onAnimationFinished(const CStack *stack, std::weak_ptr<CreatureAnimation> anim)

2
client/gui/CGuiHandler.cpp
View File

@ -9,7 +9,7 @@
*/
#include "StdInc.h"
#include "CGuiHandler.h"
#include "../lib/CondSh.h"
#include "ConditionalWait.h"
#include "CIntObject.h"
#include "CursorHandler.h"

1
client/gui/CGuiHandler.h
View File

@ -10,7 +10,6 @@
#pragma once
VCMI_LIB_NAMESPACE_BEGIN
template <typename T> struct CondSh;
class Point;
class Rect;
VCMI_LIB_NAMESPACE_END

2
client/mainmenu/CMainMenu.cpp
View File

@ -58,7 +58,7 @@
#include "../../lib/CConfigHandler.h"
#include "../../lib/GameConstants.h"
#include "../../lib/CRandomGenerator.h"
#include "../../lib/CondSh.h"
#include "../ConditionalWait.h"
std::shared_ptr<CMainMenu> CMM;
ISelectionScreenInfo * SEL;

6
client/windows/CTutorialWindow.cpp
View File

@ -12,7 +12,7 @@
#include "../eventsSDL/InputHandler.h"
#include "../../lib/CConfigHandler.h"
#include "../../lib/CondSh.h"
#include "../ConditionalWait.h"
#include "../../lib/CGeneralTextHandler.h"
#include "../CPlayerInterface.h"
#include "../CGameInfo.h"
@ -67,7 +67,7 @@ void CTutorialWindow::openWindowFirstTime(const TutorialMode & m)
if(GH.input().hasTouchInputDevice() && !persistentStorage["gui"]["tutorialCompleted" + std::to_string(m)].Bool())
{
if(LOCPLINT)
LOCPLINT->showingDialog->set(true);
LOCPLINT->showingDialog->setBusy();
GH.windows().pushWindow(std::make_shared<CTutorialWindow>(m));
Settings s = persistentStorage.write["gui"]["tutorialCompleted" + std::to_string(m)];
@ -78,7 +78,7 @@ void CTutorialWindow::openWindowFirstTime(const TutorialMode & m)
void CTutorialWindow::exit()
{
if(LOCPLINT)
LOCPLINT->showingDialog->setn(false);
LOCPLINT->showingDialog->setFree();
close();
}

6
client/windows/GUIClasses.cpp
View File

@ -56,7 +56,7 @@
#include "../lib/CGeneralTextHandler.h"
#include "../lib/CHeroHandler.h"
#include "../lib/GameSettings.h"
#include "../lib/CondSh.h"
#include "ConditionalWait.h"
#include "../lib/CSkillHandler.h"
#include "../lib/filesystem/Filesystem.h"
#include "../lib/TextOperations.h"
@ -402,7 +402,7 @@ CLevelWindow::CLevelWindow(const CGHeroInstance * hero, PrimarySkill pskill, std
{
OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
LOCPLINT->showingDialog->setn(true);
LOCPLINT->showingDialog->setBusy();
if(!skills.empty())
{
@ -445,7 +445,7 @@ CLevelWindow::~CLevelWindow()
if (box && box->selectedIndex() != -1)
cb(box->selectedIndex());
LOCPLINT->showingDialog->setn(false);
LOCPLINT->showingDialog->setFree();
}
CTavernWindow::CTavernWindow(const CGObjectInstance * TavernObj, const std::function<void()> & onWindowClosed)

6
client/windows/InfoWindows.cpp
View File

@ -29,7 +29,7 @@
#include "../../CCallback.h"
#include "../../lib/CConfigHandler.h"
#include "../../lib/CondSh.h"
#include "../ConditionalWait.h"
#include "../../lib/gameState/InfoAboutArmy.h"
#include "../../lib/mapObjects/CGCreature.h"
#include "../../lib/mapObjects/CGHeroInstance.h"
@ -140,7 +140,7 @@ void CInfoWindow::close()
WindowBase::close();
if(LOCPLINT)
LOCPLINT->showingDialog->setn(false);
LOCPLINT->showingDialog->setFree();
}
void CInfoWindow::showAll(Canvas & to)
@ -158,7 +158,7 @@ void CInfoWindow::showInfoDialog(const std::string & text, const TCompsInfo & co
void CInfoWindow::showYesNoDialog(const std::string & text, const TCompsInfo & components, const CFunctionList<void()> & onYes, const CFunctionList<void()> & onNo, PlayerColor player)
{
assert(!LOCPLINT || LOCPLINT->showingDialog->get());
assert(!LOCPLINT || LOCPLINT->showingDialog->isBusy());
std::vector<std::pair<AnimationPath, CFunctionList<void()>>> pom;
pom.emplace_back(AnimationPath::builtin("IOKAY.DEF"), nullptr);
pom.emplace_back(AnimationPath::builtin("ICANCEL.DEF"), nullptr);

1
docs/players/Cheat_Codes.md
View File

@ -142,5 +142,4 @@ Below a list of supported commands, with their arguments wrapped in `<>`
`activate <0/1/2>` - activate game windows (no current use, apparently broken long ago)
`redraw` - force full graphical redraw
`screen` - show value of screenBuf variable, which prints "screen" when adventure map has current focus, "screen2" otherwise, and dumps values of both screen surfaces to .bmp files
`not dialog` - set the state indicating if dialog box is active to "no"
`tell hs <hero ID> <artifact slot ID>` - write what artifact is present on artifact slot with specified ID for hero with specified ID. (must be called during gameplay)

1
lib/CMakeLists.txt
View File

@ -649,7 +649,6 @@ set(lib_MAIN_HEADERS
CGameInterface.h
CGeneralTextHandler.h
CHeroHandler.h
CondSh.h
ConstTransitivePtr.h
Color.h
CPlayerState.h

71
lib/CondSh.h
View File

@ -1,71 +0,0 @@
/*
* CondSh.h, part of VCMI engine
*
* Authors: listed in file AUTHORS in main folder
*
* License: GNU General Public License v2.0 or later
* Full text of license available in license.txt file, in main folder
*
*/
#pragma once
VCMI_LIB_NAMESPACE_BEGIN
/// Used for multithreading, wraps boost functions
template <typename T> struct CondSh
{
T data;
boost::condition_variable cond;
boost::mutex mx;
CondSh() : data(T()) {}
CondSh(T t) : data(t) {}
// set data
void set(T t)
{
boost::unique_lock<boost::mutex> lock(mx);
data = t;
}
// set data and notify
void setn(T t)
{
set(t);
cond.notify_all();
};
// get stored value
T get()
{
boost::unique_lock<boost::mutex> lock(mx);
return data;
}
// waits until data is set to false
void waitWhileTrue()
{
boost::unique_lock<boost::mutex> un(mx);
while(data)
cond.wait(un);
}
// waits while data is set to arg
void waitWhile(const T & t)
{
boost::unique_lock<boost::mutex> un(mx);
while(data == t)
cond.wait(un);
}
// waits until data is set to arg
void waitUntil(const T & t)
{
boost::unique_lock<boost::mutex> un(mx);
while(data != t)
cond.wait(un);
}
};
VCMI_LIB_NAMESPACE_END