]> git.lizzy.rs Git - dragonfireclient.git/commitdiff
Server: move shutdown parts to a specific shutdown state object (#7437)
authorLoïc Blot <nerzhul@users.noreply.github.com>
Wed, 13 Jun 2018 19:58:34 +0000 (21:58 +0200)
committerGitHub <noreply@github.com>
Wed, 13 Jun 2018 19:58:34 +0000 (21:58 +0200)
* Server: move shutdown parts to a specific shutdown state object

src/game.cpp
src/main.cpp
src/network/serverpackethandler.cpp
src/server.cpp
src/server.h
src/unittest/CMakeLists.txt
src/unittest/test_server_shutdown_state.cpp [new file with mode: 0644]

index 0da8e166175b642a384b1ad15fbc8e3632de041e..6e25834129d08bf8ea25b937d01e152bed8ddedb 100644 (file)
@@ -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<u32> &current_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;
index d9073cfb859e59806784b7610b9f97b0441e6c02..005e1acc74ed831431618d5572f48cf70d7540b8 100644 (file)
@@ -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
index c961d406e6a2455aaf69159e7c1b5cc5d6aff7af..bb5eed92b6cb96ba5d5f5c6f98861dd993266171 100644 (file)
@@ -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());
        }
 }
 
index 13a3b455271595d8ae11c944ce785a8c8abd5cc5..67910e53aad4152e35428500c50f3470c2ad5f0a 100644 (file)
@@ -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 \""<<m_gamespec.id<<"\"";
-       if(m_simple_singleplayer_mode)
-               infostream<<" in simple singleplayer mode"<<std::endl;
+               infostream << "Server: Kicking players" << std::endl;
+               std::string kick_msg;
+               bool reconnect = false;
+               if (isShutdownRequested()) {
+                       reconnect = m_shutdown_state.should_reconnect;
+                       kick_msg = m_shutdown_state.message;
+               }
+               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.
+       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<<std::endl;
-       infostream<<"- world:  "<<m_path_world<<std::endl;
-       infostream<<"- game:   "<<m_gamespec.path<<std::endl;
+               infostream << std::endl;
+       infostream << "- world:  " << m_path_world << std::endl;
+       infostream << "- game:   " << m_gamespec.path << std::endl;
 
        // Create world if it doesn't exist
-       if(!loadGameConfAndInitWorld(m_path_world, m_gamespec))
+       if (!loadGameConfAndInitWorld(m_path_world, m_gamespec))
                throw ServerError("Failed to initialize world");
 
        // Create server thread
@@ -202,8 +329,7 @@ Server::Server(
        std::string ban_path = m_path_world + DIR_DELIM "ipban.txt";
        m_banmanager = new BanManager(ban_path);
 
-       m_modmgr = std::unique_ptr<ServerModManager>(new ServerModManager(
-               m_path_world));
+       m_modmgr = std::unique_ptr<ServerModManager>(new ServerModManager(m_path_world));
        std::vector<ModSpec> 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"<<std::endl;
+       infostream << "Server: Initializing Lua" << std::endl;
 
        m_script = new ServerScripting(this);
 
@@ -279,74 +405,6 @@ Server::Server(
        m_csm_noderange_limit = g_settings->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;
 
                /*
index 39cf560274076e9f147d89c505e31958f2f19b5f..02c69e4914f1059ca37a6e67f3daf837d66428f6 100644 (file)
@@ -128,6 +128,7 @@ class Server : public con::PeerHandler, public MapEventReceiver,
        ~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 @@ class Server : public con::PeerHandler, public MapEventReceiver,
        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 @@ class Server : public con::PeerHandler, public MapEventReceiver,
        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 @@ class Server : public con::PeerHandler, public MapEventReceiver,
        void SetBlocksNotSent(std::map<v3s16, MapBlock *>& 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 @@ class Server : public con::PeerHandler, public MapEventReceiver,
                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;
index 768959e5e612daeb5fef1cffab46c7a91e1eed9b..5d306ee754f1735a6e27e31571f93efa1a83497e 100644 (file)
@@ -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 (file)
index 0000000..fbb76ff
--- /dev/null
@@ -0,0 +1,122 @@
+/*
+Minetest
+Copyright (C) 2018 nerzhul, Loic BLOT <loic.blot@unix-experience.fr>
+
+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 <server.h>
+#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> 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);
+}