Fix #8464: Crash on game shutdown.

distribution/changelog.txt

@@ -39,10 +39,11 @@
 - Fix: [#8204] Crash when tile element has no surface elements.
 - Fix: [#8335] Rides with arbitrary ride types can crash the game when they break down.
 - Fix: [#8358] Infinite loop when changing vehicle count on stopped ride.
+- Fix: [#8431] Crash when game action logging is enabled.
 - Fix: [#8433] Crash if master server response is not valid JSON.
 - Fix: [#8434] Crash if curl_easy_init fails.
 - Fix: [#8443] Crash when selecting the current vehicle for ride that has none available.
-- Fix: [#8431] Crash when game action logging is enabled.
+- Fix: [#8464] Crash on game shutdown.
 - Fix: [#8469] Crash modifying colour on hacked rides.
 - Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only).
 - Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab.

src/openrct2/Context.cpp

@@ -137,13 +137,15 @@ namespace OpenRCT2
 
         ~Context() override
         {
-            // Requires this as otherwise it will try to access Instance from other destructors.
+            // NOTE: We must shutdown all systems here before Instance is set back to null.
-            // after setting Instance to nullptr.
+            //       If objects use GetContext() in their destructor things won't go well.
+
             if (_objectManager)
             {
                 _objectManager->UnloadAll();
             }
 
+            network_close();
             window_close_all();
             gfx_object_check_all_images_freed();
             gfx_unload_g2();

src/openrct2/network/Network.cpp

@@ -108,7 +108,7 @@ class Network
 {
 public:
     Network();
-    ~Network();
+
     void SetEnvironment(const std::shared_ptr<OpenRCT2::IPlatformEnvironment>& env);
     bool Init();
     void Close();
@@ -203,7 +203,7 @@ private:
 
     bool ProcessConnection(NetworkConnection& connection);
     void ProcessPacket(NetworkConnection& connection, NetworkPacket& packet);
-    void AddClient(ITcpSocket* socket);
+    void AddClient(std::unique_ptr<ITcpSocket>&& socket);
     void RemoveClient(std::unique_ptr<NetworkConnection>& connection);
     NetworkPlayer* AddPlayer(const utf8* name, const std::string& keyhash);
     std::string MakePlayerNameUnique(const std::string& name);
@@ -282,9 +282,10 @@ private:
     bool _closeLock = false;
     bool _requireClose = false;
     bool wsa_initialized = false;
-    ITcpSocket* listening_socket = nullptr;
+    std::unique_ptr<ITcpSocket> _listenSocket;
+    std::unique_ptr<NetworkConnection> _serverConnection;
+    std::unique_ptr<INetworkServerAdvertiser> _advertiser;
     uint16_t listening_port = 0;
-    NetworkConnection* server_connection = nullptr;
     SOCKET_STATUS _lastConnectStatus = SOCKET_STATUS_CLOSED;
     uint32_t last_tick_sent_time = 0;
     uint32_t last_ping_sent_time = 0;
@@ -298,7 +299,6 @@ private:
     std::vector<uint8_t> chunk_buffer;
     std::string _password;
     bool _desynchronised = false;
-    INetworkServerAdvertiser* _advertiser = nullptr;
     uint32_t server_connect_time = 0;
     uint8_t default_group = 0;
     uint32_t game_commands_processed_this_tick = 0;
@@ -390,13 +390,6 @@ Network::Network()
     _server_log_fs << std::unitbuf;
 }
 
-Network::~Network()
-{
-    CloseChatLog();
-    CloseServerLog();
-    CloseConnection();
-}
-
 void Network::SetEnvironment(const std::shared_ptr<IPlatformEnvironment>& env)
 {
     _env = env;
@@ -411,7 +404,6 @@ bool Network::Init()
 
     status = NETWORK_STATUS_READY;
 
-    server_connection = new NetworkConnection();
     ServerName = std::string();
     ServerDescription = std::string();
     ServerGreeting = std::string();
@@ -453,27 +445,17 @@ void Network::CloseConnection()
 {
     if (mode == NETWORK_MODE_CLIENT)
     {
-        delete server_connection->Socket;
+        _serverConnection.reset();
-        server_connection->Socket = nullptr;
     }
     else if (mode == NETWORK_MODE_SERVER)
     {
-        delete listening_socket;
+        _listenSocket.reset();
-        listening_socket = nullptr;
+        _advertiser.reset();
-        delete _advertiser;
-        _advertiser = nullptr;
     }
 
     mode = NETWORK_MODE_NONE;
     status = NETWORK_STATUS_NONE;
     _lastConnectStatus = SOCKET_STATUS_CLOSED;
-    if (server_connection != nullptr)
-    {
-        server_connection->AuthStatus = NETWORK_AUTH_NONE;
-        server_connection->InboundPacket.Clear();
-        server_connection->SetLastDisconnectReason(nullptr);
-        delete server_connection;
-    }
 
     DisposeWSA();
 }
@@ -493,9 +475,9 @@ bool Network::BeginClient(const char* host, uint16_t port)
 
     log_info("Connecting to %s:%u\n", host, port);
 
-    assert(server_connection->Socket == nullptr);
+    _serverConnection = std::make_unique<NetworkConnection>();
-    server_connection->Socket = CreateTcpSocket();
+    _serverConnection->Socket = CreateTcpSocket();
-    server_connection->Socket->ConnectAsync(host, port);
+    _serverConnection->Socket->ConnectAsync(host, port);
     status = NETWORK_STATUS_CONNECTING;
     _lastConnectStatus = SOCKET_STATUS_CLOSED;
 
@@ -585,11 +567,10 @@ bool Network::BeginServer(uint16_t port, const char* address)
 
     log_verbose("Begin listening for clients");
 
-    assert(listening_socket == nullptr);
+    _listenSocket = CreateTcpSocket();
-    listening_socket = CreateTcpSocket();
     try
     {
-        listening_socket->Listen(address, port);
+        _listenSocket->Listen(address, port);
     }
     catch (const std::exception& ex)
     {
@@ -657,7 +638,7 @@ int32_t Network::GetAuthStatus()
 {
     if (GetMode() == NETWORK_MODE_CLIENT)
     {
-        return server_connection->AuthStatus;
+        return _serverConnection->AuthStatus;
     }
     else if (GetMode() == NETWORK_MODE_SERVER)
     {
@@ -702,7 +683,7 @@ void Network::Flush()
 {
     if (GetMode() == NETWORK_MODE_CLIENT)
     {
-        server_connection->SendQueuedPackets();
+        _serverConnection->SendQueuedPackets();
     }
     else
     {
@@ -741,22 +722,22 @@ void Network::UpdateServer()
         _advertiser->Update();
     }
 
-    ITcpSocket* tcpSocket = listening_socket->Accept();
+    std::unique_ptr<ITcpSocket> tcpSocket = _listenSocket->Accept();
     if (tcpSocket != nullptr)
     {
-        AddClient(tcpSocket);
+        AddClient(std::move(tcpSocket));
     }
 }
 
 void Network::UpdateClient()
 {
-    assert(server_connection != nullptr);
+    assert(_serverConnection != nullptr);
 
     switch (status)
     {
         case NETWORK_STATUS_CONNECTING:
         {
-            switch (server_connection->Socket->GetStatus())
+            switch (_serverConnection->Socket->GetStatus())
             {
                 case SOCKET_STATUS_RESOLVING:
                 {
@@ -793,7 +774,7 @@ void Network::UpdateClient()
                 case SOCKET_STATUS_CONNECTED:
                 {
                     status = NETWORK_STATUS_CONNECTED;
-                    server_connection->ResetLastPacketTime();
+                    _serverConnection->ResetLastPacketTime();
                     Client_Send_TOKEN();
                     char str_authenticating[256];
                     format_string(str_authenticating, 256, STR_MULTIPLAYER_AUTHENTICATING, nullptr);
@@ -806,7 +787,7 @@ void Network::UpdateClient()
                 }
                 default:
                 {
-                    const char* error = server_connection->Socket->GetError();
+                    const char* error = _serverConnection->Socket->GetError();
                     if (error != nullptr)
                     {
                         Console::Error::WriteLine(error);
@@ -822,10 +803,10 @@ void Network::UpdateClient()
         }
         case NETWORK_STATUS_CONNECTED:
         {
-            if (!ProcessConnection(*server_connection))
+            if (!ProcessConnection(*_serverConnection))
             {
                 // Do not show disconnect message window when password window closed/canceled
-                if (server_connection->AuthStatus == NETWORK_AUTH_REQUIREPASSWORD)
+                if (_serverConnection->AuthStatus == NETWORK_AUTH_REQUIREPASSWORD)
                 {
                     context_force_close_window_by_class(WC_NETWORK_STATUS);
                 }
@@ -833,9 +814,9 @@ void Network::UpdateClient()
                 {
                     char str_disconnected[256];
 
-                    if (server_connection->GetLastDisconnectReason())
+                    if (_serverConnection->GetLastDisconnectReason())
                     {
-                        const char* disconnect_reason = server_connection->GetLastDisconnectReason();
+                        const char* disconnect_reason = _serverConnection->GetLastDisconnectReason();
                         format_string(str_disconnected, 256, STR_MULTIPLAYER_DISCONNECTED_WITH_REASON, &disconnect_reason);
                     }
                     else
@@ -1022,7 +1003,7 @@ void Network::ShutdownClient()
 {
     if (GetMode() == NETWORK_MODE_CLIENT)
     {
-        server_connection->Socket->Disconnect();
+        _serverConnection->Socket->Disconnect();
     }
 }
 
@@ -1360,8 +1341,8 @@ void Network::Client_Send_TOKEN()
     log_verbose("requesting token");
     std::unique_ptr<NetworkPacket> packet(NetworkPacket::Allocate());
     *packet << (uint32_t)NETWORK_COMMAND_TOKEN;
-    server_connection->AuthStatus = NETWORK_AUTH_REQUESTED;
+    _serverConnection->AuthStatus = NETWORK_AUTH_REQUESTED;
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Client_Send_AUTH(const char* name, const char* password, const char* pubkey, const char* sig, size_t sigsize)
@@ -1375,8 +1356,8 @@ void Network::Client_Send_AUTH(const char* name, const char* password, const cha
     assert(sigsize <= (size_t)UINT32_MAX);
     *packet << (uint32_t)sigsize;
     packet->Write((const uint8_t*)sig, sigsize);
-    server_connection->AuthStatus = NETWORK_AUTH_REQUESTED;
+    _serverConnection->AuthStatus = NETWORK_AUTH_REQUESTED;
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Client_Send_OBJECTS(const std::vector<std::string>& objects)
@@ -1389,7 +1370,7 @@ void Network::Client_Send_OBJECTS(const std::vector<std::string>& objects)
         log_verbose("client requests object %s", object.c_str());
         packet->Write((const uint8_t*)object.c_str(), 8);
     }
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Server_Send_TOKEN(NetworkConnection& connection)
@@ -1539,7 +1520,7 @@ void Network::Client_Send_CHAT(const char* text)
     std::unique_ptr<NetworkPacket> packet(NetworkPacket::Allocate());
     *packet << (uint32_t)NETWORK_COMMAND_CHAT;
     packet->WriteString(text);
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Server_Send_CHAT(const char* text)
@@ -1556,7 +1537,7 @@ void Network::Client_Send_GAMECMD(
     std::unique_ptr<NetworkPacket> packet(NetworkPacket::Allocate());
     *packet << (uint32_t)NETWORK_COMMAND_GAMECMD << gCurrentTicks << eax << (ebx | GAME_COMMAND_FLAG_NETWORKED) << ecx << edx
             << esi << edi << ebp << callback;
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Server_Send_GAMECMD(
@@ -1588,7 +1569,7 @@ void Network::Client_Send_GAME_ACTION(const GameAction* action)
 
     *packet << (uint32_t)NETWORK_COMMAND_GAME_ACTION << gCurrentTicks << action->GetType() << stream;
 
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Server_Send_GAME_ACTION(const GameAction* action)
@@ -1651,7 +1632,7 @@ void Network::Client_Send_PING()
 {
     std::unique_ptr<NetworkPacket> packet(NetworkPacket::Allocate());
     *packet << (uint32_t)NETWORK_COMMAND_PING;
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 void Network::Server_Send_PING()
@@ -1849,7 +1830,7 @@ void Network::ProcessPlayerList()
                 *player = pendingPlayer;
                 if (player->Flags & NETWORK_PLAYER_FLAG_ISSERVER)
                 {
-                    server_connection->Player = player;
+                    _serverConnection->Player = player;
                 }
             }
         }
@@ -1976,17 +1957,22 @@ void Network::EnqueueGameAction(const GameAction* action)
     game_command_queue.emplace(gCurrentTicks, std::move(ga), _commandId++);
 }
 
-void Network::AddClient(ITcpSocket* socket)
+void Network::AddClient(std::unique_ptr<ITcpSocket>&& socket)
 {
     if (gConfigNetwork.pause_server_if_no_clients && game_is_paused())
     {
         game_do_command(0, 1, 0, 0, GAME_COMMAND_TOGGLE_PAUSE, 0, 0);
     }
-    auto connection = std::make_unique<NetworkConnection>();
+
-    connection->Socket = socket;
+    // Log connection info.
     char addr[128];
     snprintf(addr, sizeof(addr), "Client joined from %s", socket->GetHostName());
     AppendServerLog(addr);
+
+    // Store connection
+    auto connection = std::make_unique<NetworkConnection>();
+    connection->Socket = std::move(socket);
+
     client_connection_list.push_back(std::move(connection));
 }
 
@@ -3016,7 +3002,7 @@ void Network::Client_Send_GAMEINFO()
     log_verbose("requesting gameinfo");
     std::unique_ptr<NetworkPacket> packet(NetworkPacket::Allocate());
     *packet << (uint32_t)NETWORK_COMMAND_GAMEINFO;
-    server_connection->QueuePacket(std::move(packet));
+    _serverConnection->QueuePacket(std::move(packet));
 }
 
 static std::string json_stdstring_value(const json_t* string)

src/openrct2/network/NetworkConnection.cpp

@@ -26,7 +26,6 @@ NetworkConnection::NetworkConnection()
 
 NetworkConnection::~NetworkConnection()
 {
-    delete Socket;
     delete[] _lastDisconnectReason;
 }
 

src/openrct2/network/NetworkConnection.h

@@ -14,19 +14,19 @@
 #    include "NetworkKey.h"
 #    include "NetworkPacket.h"
 #    include "NetworkTypes.h"
+#    include "TcpSocket.h"
 
 #    include <list>
 #    include <memory>
 #    include <vector>
 
-interface ITcpSocket;
 class NetworkPlayer;
 struct ObjectRepositoryItem;
 
 class NetworkConnection final
 {
 public:
-    ITcpSocket* Socket = nullptr;
+    std::unique_ptr<ITcpSocket> Socket = nullptr;
     NetworkPacket InboundPacket;
     NETWORK_AUTH AuthStatus = NETWORK_AUTH_NONE;
     NetworkPlayer* Player = nullptr;

src/openrct2/network/NetworkServerAdvertiser.cpp

@@ -26,6 +26,7 @@
 #    include "network.h"
 
 #    include <iterator>
+#    include <memory>
 #    include <string>
 
 #    ifndef DISABLE_HTTP
@@ -261,9 +262,9 @@ private:
     }
 };
 
-INetworkServerAdvertiser* CreateServerAdvertiser(uint16_t port)
+std::unique_ptr<INetworkServerAdvertiser> CreateServerAdvertiser(uint16_t port)
 {
-    return new NetworkServerAdvertiser(port);
+    return std::make_unique<NetworkServerAdvertiser>(port);
 }
 
 #    else // DISABLE_HTTP

src/openrct2/network/NetworkServerAdvertiser.h

@@ -11,6 +11,8 @@
 
 #include "../common.h"
 
+#include <memory>
+
 enum class ADVERTISE_STATUS
 {
     DISABLED,
@@ -28,4 +30,4 @@ interface INetworkServerAdvertiser
     virtual void Update() abstract;
 };
 
-INetworkServerAdvertiser* CreateServerAdvertiser(uint16_t port);
+std::unique_ptr<INetworkServerAdvertiser> CreateServerAdvertiser(uint16_t port);

src/openrct2/network/TcpSocket.cpp

@@ -176,7 +176,7 @@ public:
         _status = SOCKET_STATUS_LISTENING;
     }
 
-    ITcpSocket* Accept() override
+    std::unique_ptr<ITcpSocket> Accept() override
     {
         if (_status != SOCKET_STATUS_LISTENING)
         {
@@ -187,7 +187,7 @@ public:
         };
         socklen_t client_len = sizeof(struct sockaddr_storage);
 
-        ITcpSocket* tcpSocket = nullptr;
+        std::unique_ptr<ITcpSocket> tcpSocket;
         SOCKET socket = accept(_socket, (struct sockaddr*)&client_addr, &client_len);
         if (socket == INVALID_SOCKET)
         {
@@ -212,11 +212,11 @@ public:
                 SetTCPNoDelay(socket, true);
                 if (rc == 0)
                 {
-                    tcpSocket = new TcpSocket(socket, hostName);
+                    tcpSocket = std::unique_ptr<ITcpSocket>(new TcpSocket(socket, hostName));
                 }
                 else
                 {
-                    tcpSocket = new TcpSocket(socket, "");
+                    tcpSocket = std::unique_ptr<ITcpSocket>(new TcpSocket(socket, ""));
                 }
             }
         }
@@ -496,9 +496,9 @@ private:
     }
 };
 
-ITcpSocket* CreateTcpSocket()
+std::unique_ptr<ITcpSocket> CreateTcpSocket()
 {
-    return new TcpSocket();
+    return std::make_unique<TcpSocket>();
 }
 
 bool InitialiseWSA()

src/openrct2/network/TcpSocket.h

@@ -11,6 +11,8 @@
 
 #include "../common.h"
 
+#include <memory>
+
 enum SOCKET_STATUS
 {
     SOCKET_STATUS_CLOSED,
@@ -44,7 +46,7 @@ public:
 
     virtual void Listen(uint16_t port) abstract;
     virtual void Listen(const char* address, uint16_t port) abstract;
-    virtual ITcpSocket* Accept() abstract;
+    virtual std::unique_ptr<ITcpSocket> Accept() abstract;
 
     virtual void Connect(const char* address, uint16_t port) abstract;
     virtual void ConnectAsync(const char* address, uint16_t port) abstract;
@@ -56,7 +58,7 @@ public:
     virtual void Close() abstract;
 };
 
-ITcpSocket* CreateTcpSocket();
+std::unique_ptr<ITcpSocket> CreateTcpSocket();
 
 bool InitialiseWSA();
 void DisposeWSA();