From 9a1d3584c22013860ec4b664858076ab6ddf3ea1 Mon Sep 17 00:00:00 2001 From: Loïc Blot Date: Wed, 13 Jun 2018 21:58:34 +0200 Subject: Server: move shutdown parts to a specific shutdown state object (#7437) * Server: move shutdown parts to a specific shutdown state object --- src/game.cpp | 3 +- src/main.cpp | 2 + src/network/serverpackethandler.cpp | 7 +- src/server.cpp | 270 +++++++++++++++------------- src/server.h | 28 ++- src/unittest/CMakeLists.txt | 1 + src/unittest/test_server_shutdown_state.cpp | 122 +++++++++++++ 7 files changed, 296 insertions(+), 137 deletions(-) create mode 100644 src/unittest/test_server_shutdown_state.cpp diff --git a/src/game.cpp b/src/game.cpp index 0da8e1661..6e2583412 100644 --- a/src/game.cpp +++ b/src/game.cpp @@ -1065,7 +1065,7 @@ void Game::run() while (RenderingEngine::run() && !(*kill || g_gamecallback->shutdown_requested - || (server && server->getShutdownRequested()))) { + || (server && server->isShutdownRequested()))) { const irr::core::dimension2d ¤t_screen_size = RenderingEngine::get_video_driver()->getScreenSize(); @@ -1271,6 +1271,7 @@ bool Game::createSingleplayerServer(const std::string &map_dir, } server = new Server(map_dir, gamespec, simple_singleplayer_mode, bind_addr, false); + server->init(); server->start(); return true; diff --git a/src/main.cpp b/src/main.cpp index d9073cfb8..005e1acc7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -870,6 +870,7 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings & // Create server Server server(game_params.world_path, game_params.game_spec, false, bind_addr, true, &iface); + server.init(); g_term_console.setup(&iface, &kill, admin_nick); @@ -904,6 +905,7 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings & // Create server Server server(game_params.world_path, game_params.game_spec, false, bind_addr, true); + server.init(); server.start(); // Run server diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index c961d406e..bb5eed92b 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -402,11 +402,8 @@ void Server::handleCommand_ClientReady(NetworkPacket* pkt) m_clients.event(peer_id, CSE_SetClientReady); m_script->on_joinplayer(playersao); // Send shutdown timer if shutdown has been scheduled - if (m_shutdown_timer > 0.0f) { - std::wstringstream ws; - ws << L"*** Server shutting down in " - << duration_to_string(myround(m_shutdown_timer)).c_str() << "."; - SendChatMessage(pkt->getPeerId(), ws.str()); + if (m_shutdown_state.isTimerRunning()) { + SendChatMessage(pkt->getPeerId(), m_shutdown_state.getShutdownTimerMessage()); } } diff --git a/src/server.cpp b/src/server.cpp index 13a3b4552..67910e53a 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -139,7 +139,60 @@ v3f ServerSoundParams::getPos(ServerEnvironment *env, bool *pos_exists) const return v3f(0,0,0); } +void Server::ShutdownState::reset() +{ + m_timer = 0.0f; + message.clear(); + should_reconnect = false; + is_requested = false; +} + +void Server::ShutdownState::trigger(float delay, const std::string &msg, bool reconnect) +{ + m_timer = delay; + message = msg; + should_reconnect = reconnect; +} +void Server::ShutdownState::tick(float dtime, Server *server) +{ + if (m_timer <= 0.0f) + return; + + // Timed shutdown + static const float shutdown_msg_times[] = + { + 1, 2, 3, 4, 5, 10, 20, 40, 60, 120, 180, 300, 600, 1200, 1800, 3600 + }; + + // Automated messages + if (m_timer < shutdown_msg_times[ARRLEN(shutdown_msg_times) - 1]) { + for (float t : shutdown_msg_times) { + // If shutdown timer matches an automessage, shot it + if (m_timer > t && m_timer - dtime < t) { + std::wstring periodicMsg = getShutdownTimerMessage(); + + infostream << wide_to_utf8(periodicMsg).c_str() << std::endl; + server->SendChatMessage(PEER_ID_INEXISTENT, periodicMsg); + break; + } + } + } + + m_timer -= dtime; + if (m_timer < 0.0f) { + m_timer = 0.0f; + is_requested = true; + } +} + +std::wstring Server::ShutdownState::getShutdownTimerMessage() const +{ + std::wstringstream ws; + ws << L"*** Server shutting down in " + << duration_to_string(myround(m_timer)).c_str() << "."; + return ws.str(); +} /* Server @@ -174,22 +227,96 @@ Server::Server( { m_lag = g_settings->getFloat("dedicated_server_step"); - if (path_world.empty()) + if (m_path_world.empty()) throw ServerError("Supplied empty world path"); - if(!gamespec.isValid()) + if (!gamespec.isValid()) throw ServerError("Supplied invalid gamespec"); +} + +Server::~Server() +{ + infostream << "Server destructing" << std::endl; + + // Send shutdown message + SendChatMessage(PEER_ID_INEXISTENT, ChatMessage(CHATMESSAGE_TYPE_ANNOUNCE, + L"*** Server shutting down")); + + if (m_env) { + MutexAutoLock envlock(m_env_mutex); + + infostream << "Server: Saving players" << std::endl; + m_env->saveLoadedPlayers(); - infostream<<"Server created for gameid \""<get("kick_msg_shutdown"); + } + m_env->kickAllPlayers(SERVER_ACCESSDENIED_SHUTDOWN, + kick_msg, reconnect); + } + + // Do this before stopping the server in case mapgen callbacks need to access + // server-controlled resources (like ModStorages). Also do them before + // shutdown callbacks since they may modify state that is finalized in a + // callback. + if (m_emerge) + m_emerge->stopThreads(); + + if (m_env) { + MutexAutoLock envlock(m_env_mutex); + + // Execute script shutdown hooks + infostream << "Executing shutdown hooks" << std::endl; + m_script->on_shutdown(); + + infostream << "Server: Saving environment metadata" << std::endl; + m_env->saveMeta(); + } + + // Stop threads + if (m_thread) { + stop(); + delete m_thread; + } + + // Delete things in the reverse order of creation + delete m_emerge; + delete m_env; + delete m_rollback; + delete m_banmanager; + delete m_itemdef; + delete m_nodedef; + delete m_craftdef; + + // Deinitialize scripting + infostream << "Server: Deinitializing scripting" << std::endl; + delete m_script; + + // Delete detached inventories + for (auto &detached_inventory : m_detached_inventories) { + delete detached_inventory.second; + } +} + +void Server::init() +{ + infostream << "Server created for gameid \"" << m_gamespec.id << "\""; + if (m_simple_singleplayer_mode) + infostream << " in simple singleplayer mode" << std::endl; else - infostream<(new ServerModManager( - m_path_world)); + m_modmgr = std::unique_ptr(new ServerModManager(m_path_world)); std::vector unsatisfied_mods = m_modmgr->getUnsatisfiedMods(); // complain about mods with unsatisfied dependencies if (!m_modmgr->isConsistent()) { @@ -214,10 +340,10 @@ Server::Server( MutexAutoLock envlock(m_env_mutex); // Create the Map (loads map_meta.txt, overriding configured mapgen params) - ServerMap *servermap = new ServerMap(path_world, this, m_emerge); + ServerMap *servermap = new ServerMap(m_path_world, this, m_emerge); // Initialize scripting - infostream<<"Server: Initializing Lua"<getU32("csm_flavour_noderange_limit"); } -Server::~Server() -{ - infostream << "Server destructing" << std::endl; - - // Send shutdown message - SendChatMessage(PEER_ID_INEXISTENT, ChatMessage(CHATMESSAGE_TYPE_ANNOUNCE, - L"*** Server shutting down")); - - { - MutexAutoLock envlock(m_env_mutex); - - infostream << "Server: Saving players" << std::endl; - m_env->saveLoadedPlayers(); - - infostream << "Server: Kicking players" << std::endl; - std::string kick_msg; - bool reconnect = false; - if (getShutdownRequested()) { - reconnect = m_shutdown_ask_reconnect; - kick_msg = m_shutdown_msg; - } - if (kick_msg.empty()) { - kick_msg = g_settings->get("kick_msg_shutdown"); - } - m_env->kickAllPlayers(SERVER_ACCESSDENIED_SHUTDOWN, - kick_msg, reconnect); - } - - // Do this before stopping the server in case mapgen callbacks need to access - // server-controlled resources (like ModStorages). Also do them before - // shutdown callbacks since they may modify state that is finalized in a - // callback. - m_emerge->stopThreads(); - - { - MutexAutoLock envlock(m_env_mutex); - - // Execute script shutdown hooks - infostream << "Executing shutdown hooks" << std::endl; - m_script->on_shutdown(); - - infostream << "Server: Saving environment metadata" << std::endl; - m_env->saveMeta(); - } - - // Stop threads - stop(); - delete m_thread; - - // Delete things in the reverse order of creation - delete m_emerge; - delete m_env; - delete m_rollback; - delete m_banmanager; - delete m_itemdef; - delete m_nodedef; - delete m_craftdef; - - // Deinitialize scripting - infostream << "Server: Deinitializing scripting" << std::endl; - delete m_script; - - // Delete detached inventories - for (auto &detached_inventory : m_detached_inventories) { - delete detached_inventory.second; - } -} - void Server::start() { infostream << "Starting server on " << m_bind_addr.serializeString() @@ -916,38 +974,7 @@ void Server::AsyncRunStep(bool initial_step) } } - // Timed shutdown - static const float shutdown_msg_times[] = - { - 1, 2, 3, 4, 5, 10, 15, 20, 25, 30, 45, 60, 120, 180, 300, 600, 1200, 1800, 3600 - }; - - if (m_shutdown_timer > 0.0f) { - // Automated messages - if (m_shutdown_timer < shutdown_msg_times[ARRLEN(shutdown_msg_times) - 1]) { - for (u16 i = 0; i < ARRLEN(shutdown_msg_times) - 1; i++) { - // If shutdown timer matches an automessage, shot it - if (m_shutdown_timer > shutdown_msg_times[i] && - m_shutdown_timer - dtime < shutdown_msg_times[i]) { - std::wstringstream ws; - - ws << L"*** Server shutting down in " - << duration_to_string(myround(m_shutdown_timer - dtime)).c_str() - << "."; - - infostream << wide_to_utf8(ws.str()).c_str() << std::endl; - SendChatMessage(PEER_ID_INEXISTENT, ws.str()); - break; - } - } - } - - m_shutdown_timer -= dtime; - if (m_shutdown_timer < 0.0f) { - m_shutdown_timer = 0.0f; - m_shutdown_requested = true; - } - } + m_shutdown_state.tick(dtime, this); } void Server::Receive() @@ -3398,16 +3425,13 @@ void Server::requestShutdown(const std::string &msg, bool reconnect, float delay { if (delay == 0.0f) { // No delay, shutdown immediately - m_shutdown_requested = true; + m_shutdown_state.is_requested = true; // only print to the infostream, a chat message saying // "Server Shutting Down" is sent when the server destructs. infostream << "*** Immediate Server shutdown requested." << std::endl; - } else if (delay < 0.0f && m_shutdown_timer > 0.0f) { - // Negative delay, cancel shutdown if requested - m_shutdown_timer = 0.0f; - m_shutdown_msg = ""; - m_shutdown_ask_reconnect = false; - m_shutdown_requested = false; + } else if (delay < 0.0f && m_shutdown_state.isTimerRunning()) { + // Negative delay, cancel shutdown if requested + m_shutdown_state.reset(); std::wstringstream ws; ws << L"*** Server shutdown canceled."; @@ -3428,9 +3452,7 @@ void Server::requestShutdown(const std::string &msg, bool reconnect, float delay SendChatMessage(PEER_ID_INEXISTENT, ws.str()); } - m_shutdown_timer = delay; - m_shutdown_msg = msg; - m_shutdown_ask_reconnect = reconnect; + m_shutdown_state.trigger(delay, msg, reconnect); } PlayerSAO* Server::emergePlayer(const char *name, session_t peer_id, u16 proto_version) @@ -3518,7 +3540,7 @@ void dedicated_server_loop(Server &server, bool &kill) } server.step(steplen); - if (server.getShutdownRequested() || kill) + if (server.isShutdownRequested() || kill) break; /* diff --git a/src/server.h b/src/server.h index 39cf56027..02c69e491 100644 --- a/src/server.h +++ b/src/server.h @@ -128,6 +128,7 @@ public: ~Server(); DISABLE_CLASS_COPY(Server); + void init(); void start(); void stop(); // This is mainly a way to pass the time to the server. @@ -201,7 +202,7 @@ public: inline double getUptime() const { return m_uptime.m_value; } // read shutdown state - inline bool getShutdownRequested() const { return m_shutdown_requested; } + inline bool isShutdownRequested() const { return m_shutdown_state.is_requested; } // request server to shutdown void requestShutdown(const std::string &msg, bool reconnect, float delay = 0.0f); @@ -348,9 +349,25 @@ public: std::mutex m_env_mutex; private: - friend class EmergeThread; friend class RemoteClient; + friend class TestServerShutdownState; + + struct ShutdownState { + friend class TestServerShutdownState; + public: + bool is_requested = false; + bool should_reconnect = false; + std::string message; + + void reset(); + void trigger(float delay, const std::string &msg, bool reconnect); + void tick(float dtime, Server *server); + std::wstring getShutdownTimerMessage() const; + bool isTimerRunning() const { return m_timer > 0.0f; } + private: + float m_timer = 0.0f; + }; void SendMovement(session_t peer_id); void SendHP(session_t peer_id, u16 hp); @@ -368,7 +385,7 @@ private: void SetBlocksNotSent(std::map& block); - void SendChatMessage(session_t peer_id, const ChatMessage &message); + virtual void SendChatMessage(session_t peer_id, const ChatMessage &message); void SendTimeOfDay(session_t peer_id, u16 time, f32 time_speed); void SendPlayerHP(session_t peer_id); @@ -586,10 +603,7 @@ private: Random stuff */ - bool m_shutdown_requested = false; - std::string m_shutdown_msg; - bool m_shutdown_ask_reconnect = false; - float m_shutdown_timer = 0.0f; + ShutdownState m_shutdown_state; ChatInterface *m_admin_chat; std::string m_admin_nick; diff --git a/src/unittest/CMakeLists.txt b/src/unittest/CMakeLists.txt index 768959e5e..5d306ee75 100644 --- a/src/unittest/CMakeLists.txt +++ b/src/unittest/CMakeLists.txt @@ -20,6 +20,7 @@ set (UNITTEST_SRCS ${CMAKE_CURRENT_SOURCE_DIR}/test_random.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_schematic.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_serialization.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_server_shutdown_state.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_settings.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_socket.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_serveractiveobjectmap.cpp diff --git a/src/unittest/test_server_shutdown_state.cpp b/src/unittest/test_server_shutdown_state.cpp new file mode 100644 index 000000000..fbb76ff6a --- /dev/null +++ b/src/unittest/test_server_shutdown_state.cpp @@ -0,0 +1,122 @@ +/* +Minetest +Copyright (C) 2018 nerzhul, Loic BLOT + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this program; if not, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#include +#include "test.h" + +#include "util/string.h" +#include "util/serialize.h" + +class FakeServer : public Server +{ +public: + // clang-format off + FakeServer() : Server("fakeworld", SubgameSpec("fakespec", "fakespec"), true, + Address(), true, nullptr) + { + } + // clang-format on + +private: + void SendChatMessage(session_t peer_id, const ChatMessage &message) + { + // NOOP + } +}; + +class TestServerShutdownState : public TestBase +{ +public: + TestServerShutdownState() { TestManager::registerTestModule(this); } + const char *getName() { return "TestServerShutdownState"; } + + void runTests(IGameDef *gamedef); + + void testInit(); + void testReset(); + void testTrigger(); + void testTick(); +}; + +static TestServerShutdownState g_test_instance; + +void TestServerShutdownState::runTests(IGameDef *gamedef) +{ + TEST(testInit); + TEST(testReset); + TEST(testTrigger); + TEST(testTick); +} + +void TestServerShutdownState::testInit() +{ + Server::ShutdownState ss; + UASSERT(!ss.is_requested); + UASSERT(!ss.should_reconnect); + UASSERT(ss.message.empty()); + UASSERT(ss.m_timer == 0.0f); +} + +void TestServerShutdownState::testReset() +{ + Server::ShutdownState ss; + ss.reset(); + UASSERT(!ss.is_requested); + UASSERT(!ss.should_reconnect); + UASSERT(ss.message.empty()); + UASSERT(ss.m_timer == 0.0f); +} + +void TestServerShutdownState::testTrigger() +{ + Server::ShutdownState ss; + ss.trigger(3.0f, "testtrigger", true); + UASSERT(!ss.is_requested); + UASSERT(ss.should_reconnect); + UASSERT(ss.message == "testtrigger"); + UASSERT(ss.m_timer == 3.0f); +} + +void TestServerShutdownState::testTick() +{ + std::unique_ptr fakeServer(new FakeServer()); + Server::ShutdownState ss; + ss.trigger(28.0f, "testtrigger", true); + ss.tick(0.0f, fakeServer.get()); + + // Tick with no time should not change anything + UASSERT(!ss.is_requested); + UASSERT(ss.should_reconnect); + UASSERT(ss.message == "testtrigger"); + UASSERT(ss.m_timer == 28.0f); + + // Tick 2 seconds + ss.tick(2.0f, fakeServer.get()); + UASSERT(!ss.is_requested); + UASSERT(ss.should_reconnect); + UASSERT(ss.message == "testtrigger"); + UASSERT(ss.m_timer == 26.0f); + + // Tick remaining seconds + additional expire + ss.tick(26.1f, fakeServer.get()); + UASSERT(ss.is_requested); + UASSERT(ss.should_reconnect); + UASSERT(ss.message == "testtrigger"); + UASSERT(ss.m_timer == 0.0f); +} -- cgit v1.2.3