Fix #5496: Attempting to log into servers results in crash

Caused by the user not having the required 'official' objects. This was because the network was being closed during a network update. Disposed memory would then be accessed later in the update loop.

To fix this, a lock has been added to Close() so that it can be deferred to the end of Update(). This isn't particularly nice, but the whole of network will need redesigning to fix this properly for all potentical scenarios where Close() can be called.

src/openrct2/network/network.cpp

@@ -151,6 +151,15 @@ void Network::Close()
 		// which may no longer be valid on Linux and would cause a segfault.
 		return;
 	}
+
+	// HACK Because Close() is closed all over the place, it sometimes gets called inside an Update
+	//      call. This then causes disposed data to be accessed. Therefore, save closing until the
+	//      end of the update loop.
+	if (_closeLock) {
+		_requireClose = true;
+		return;
+	}
+
 	if (mode == NETWORK_MODE_CLIENT) {
 		delete server_connection->Socket;
 		server_connection->Socket = nullptr;
@@ -178,6 +187,8 @@ void Network::Close()
 
 	CloseChatLog();
 	gfx_invalidate_screen();
+
+	_requireClose = false;
 }
 
 bool Network::BeginClient(const char* host, uint16 port)
@@ -362,6 +373,8 @@ uint8 Network::GetPlayerID()
 
 void Network::Update()
 {
+	_closeLock = true;
+
 	switch (GetMode()) {
 	case NETWORK_MODE_SERVER:
 		UpdateServer();
@@ -370,6 +383,12 @@ void Network::Update()
 		UpdateClient();
 		break;
 	}
+
+	// If the Close() was called during the update, close it for real
+	_closeLock = false;
+	if (_requireClose) {
+		Close();
+	}
 }
 
 void Network::UpdateServer()
@@ -405,6 +424,8 @@ void Network::UpdateServer()
 
 void Network::UpdateClient()
 {
+	assert(server_connection != nullptr);
+
 	switch(status){
 	case NETWORK_STATUS_CONNECTING:
 	{

src/openrct2/network/network.h

@@ -195,6 +195,8 @@ private:
 
 	sint32 mode = NETWORK_MODE_NONE;
 	sint32 status = NETWORK_STATUS_NONE;
+	bool _closeLock = false;
+	bool _requireClose = false;
 	bool wsa_initialized = false;
 	ITcpSocket * listening_socket = nullptr;
 	uint16 listening_port = 0;

src/openrct2/object.h

@@ -114,8 +114,8 @@ sint32 object_calculate_checksum(const rct_object_entry * entry, const void * da
 sint32 find_object_in_entry_group(const rct_object_entry* entry, uint8* entry_type, uint8* entry_index);
 void object_create_identifier_name(char* string_buffer, size_t size, const rct_object_entry* object);
 
-rct_object_entry *object_list_find_by_name(const char *name);
-rct_object_entry *object_list_find(rct_object_entry *entry);
+const rct_object_entry * object_list_find_by_name(const char *name);
+const rct_object_entry * object_list_find(rct_object_entry *entry);
 
 void object_entry_get_name(utf8 * buffer, size_t bufferSize, const rct_object_entry * entry);
 void object_entry_get_name_fixed(utf8 * buffer, size_t bufferSize, const rct_object_entry * entry);

src/openrct2/object/ObjectRepository.cpp

@@ -717,18 +717,28 @@ bool IsObjectCustom(const ObjectRepositoryItem * object)
 
 extern "C"
 {
-    rct_object_entry * object_list_find(rct_object_entry * entry)
+    const rct_object_entry * object_list_find(rct_object_entry * entry)
     {
-        IObjectRepository * objRepo = GetObjectRepository();
-        const ObjectRepositoryItem * item = objRepo->FindObject(entry);
-        return (rct_object_entry *)&item->ObjectEntry;
+        const rct_object_entry * result = nullptr;
+        auto objRepo = GetObjectRepository();
+        auto item = objRepo->FindObject(entry);
+        if (item != nullptr)
+        {
+            result = &item->ObjectEntry;
+        }
+        return result;
     }
 
-    rct_object_entry * object_list_find_by_name(const char * name)
+    const rct_object_entry * object_list_find_by_name(const char * name)
     {
-        IObjectRepository * objRepo = GetObjectRepository();
-        const ObjectRepositoryItem * item = objRepo->FindObject(name);
-        return (rct_object_entry *)&item->ObjectEntry;
+        const rct_object_entry * result = nullptr;
+        auto objRepo = GetObjectRepository();
+        auto item = objRepo->FindObject(name);
+        if (item != nullptr)
+        {
+            result = &item->ObjectEntry;
+        }
+        return result;
     }
 
     void object_list_load()

src/openrct2/rct1/S4Importer.cpp

@@ -468,7 +468,7 @@ private:
             std::vector<const char *> objects = RCT1::GetSceneryObjects(sceneryTheme);
             for (const char * objectName : objects)
             {
-                rct_object_entry * foundEntry = object_list_find_by_name(objectName);
+                const rct_object_entry * foundEntry = object_list_find_by_name(objectName);
                 if (foundEntry != nullptr)
                 {
                     uint8 objectType = foundEntry->flags & 0x0F;