From 6a3aaef4868ed0812a569c1a048a6511dac80ba7 Mon Sep 17 00:00:00 2001 From: rubidium Date: Tue, 20 Jan 2009 03:44:43 +0000 Subject: (svn r15159) -Fix: move the UDP queries that resolve a hostname into threads so they don't freeze OpenTTD when for example the network connection got severed. Thanks to glx for writing the mutex implementation for Windows. --- src/network/network_gamelist.cpp | 47 +++++++++++++- src/network/network_gamelist.h | 1 + src/network/network_udp.cpp | 133 ++++++++++++++++++++++++++------------- src/thread.h | 23 +++++++ src/thread_none.cpp | 12 ++++ src/thread_pthread.cpp | 34 ++++++++++ src/thread_win32.cpp | 34 ++++++++++ 7 files changed, 240 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/src/network/network_gamelist.cpp b/src/network/network_gamelist.cpp index 235e9afe3..e50f5cc38 100644 --- a/src/network/network_gamelist.cpp +++ b/src/network/network_gamelist.cpp @@ -11,6 +11,8 @@ #include "../debug.h" #include "../newgrf_config.h" #include "../core/alloc_func.hpp" +#include "../thread.h" +#include "../string_func.h" #include "network_internal.h" #include "core/game.h" #include "network_udp.h" @@ -19,6 +21,46 @@ NetworkGameList *_network_game_list = NULL; +ThreadMutex *_network_game_list_mutex = ThreadMutex::New(); +NetworkGameList *_network_game_delayed_insertion_list = NULL; + +/** Add a new item to the linked gamelist, but do it delayed in the next tick + * or so to prevent race conditions. + * @param item the item to add. Will be freed once added. + */ +void NetworkGameListAddItemDelayed(NetworkGameList *item) +{ + _network_game_list_mutex->BeginCritical(); + item->next = _network_game_delayed_insertion_list; + _network_game_delayed_insertion_list = item; + _network_game_list_mutex->EndCritical(); +} + +/** Perform the delayed (thread safe) insertion into the game list */ +static void NetworkGameListHandleDelayedInsert() +{ + _network_game_list_mutex->BeginCritical(); + while (_network_game_delayed_insertion_list != NULL) { + NetworkGameList *ins_item = _network_game_delayed_insertion_list; + _network_game_delayed_insertion_list = ins_item->next; + + NetworkGameList *item = NetworkGameListAddItem(ins_item->ip, ins_item->port); + + if (item != NULL) { + if (StrEmpty(item->info.server_name)) { + memset(&item->info, 0, sizeof(item->info)); + strecpy(item->info.server_name, ins_item->info.server_name, lastof(item->info.server_name)); + strecpy(item->info.hostname, ins_item->info.hostname, lastof(item->info.hostname)); + item->online = false; + } + item->manually = ins_item->manually; + UpdateNetworkGameWindow(false); + } + free(ins_item); + } + _network_game_list_mutex->EndCritical(); +} + /** Add a new item to the linked gamelist. If the IP and Port match * return the existing item instead of adding it again * @param ip the IP-address (inet_addr) of the to-be added item @@ -36,8 +78,7 @@ NetworkGameList *NetworkGameListAddItem(uint32 ip, uint16 port) prev_item = item; } - item = MallocT(1); - memset(item, 0, sizeof(*item)); + item = CallocT(1); item->next = NULL; item->ip = ip; item->port = port; @@ -91,6 +132,8 @@ enum { /** Requeries the (game) servers we have not gotten a reply from */ void NetworkGameListRequery() { + NetworkGameListHandleDelayedInsert(); + static uint8 requery_cnt = 0; if (++requery_cnt < REQUERY_EVERY_X_GAMELOOPS) return; diff --git a/src/network/network_gamelist.h b/src/network/network_gamelist.h index a7e8f3578..bf7bc68c2 100644 --- a/src/network/network_gamelist.h +++ b/src/network/network_gamelist.h @@ -21,6 +21,7 @@ struct NetworkGameList { /** Game list of this client */ extern NetworkGameList *_network_game_list; +void NetworkGameListAddItemDelayed(NetworkGameList *item); NetworkGameList *NetworkGameListAddItem(uint32 ip, uint16 port); void NetworkGameListRemoveItem(NetworkGameList *remove); void NetworkGameListRequery(); diff --git a/src/network/network_udp.cpp b/src/network/network_udp.cpp index 9c42d8d31..9d4778708 100644 --- a/src/network/network_udp.cpp +++ b/src/network/network_udp.cpp @@ -20,14 +20,18 @@ #include "../variables.h" #include "../newgrf_config.h" #include "../core/endian_func.hpp" +#include "../core/alloc_func.hpp" #include "../string_func.h" #include "../company_base.h" #include "../company_func.h" #include "../settings_type.h" +#include "../thread.h" #include "../rev.h" #include "core/udp.h" +ThreadMutex *_network_udp_mutex = ThreadMutex::New(); + enum { ADVERTISE_NORMAL_INTERVAL = 30000, // interval between advertising in ticks (15 minutes) ADVERTISE_RETRY_INTERVAL = 300, // readvertise when no response after this many ticks (9 seconds) @@ -352,9 +356,11 @@ void NetworkUDPCloseAll() { DEBUG(net, 1, "[udp] closed listeners"); + _network_udp_mutex->BeginCritical(); _udp_server_socket->Close(); _udp_master_socket->Close(); _udp_client_socket->Close(); + _network_udp_mutex->EndCritical(); _network_udp_server = false; _network_udp_broadcast = 0; @@ -420,44 +426,84 @@ void NetworkUDPSearchGame() _network_udp_broadcast = 300; // Stay searching for 300 ticks } -void NetworkUDPQueryServer(NetworkAddress address, bool manually) +/** Simpler wrapper struct for NetworkUDPQueryServerThread */ +struct NetworkUDPQueryServerInfo : NetworkAddress { + bool manually; ///< Did we connect manually or not? + NetworkUDPQueryServerInfo(const NetworkAddress &address, bool manually) : + NetworkAddress(address), + manually(manually) + { + } +}; + +/** + * Threaded part for resolving the IP of a server and querying it. + * @param pntr the NetworkUDPQueryServerInfo. + */ +void NetworkUDPQueryServerThread(void *pntr) { + NetworkUDPQueryServerInfo *info = (NetworkUDPQueryServerInfo*)pntr; + struct sockaddr_in out_addr; - NetworkGameList *item; + out_addr.sin_family = AF_INET; + out_addr.sin_port = htons(info->GetPort()); + out_addr.sin_addr.s_addr = info->GetIP(); + + /* Clear item in gamelist */ + NetworkGameList *item = CallocT(1); + item->ip = info->GetIP(); + item->port = info->GetPort(); + strecpy(item->info.server_name, info->GetHostname(), lastof(item->info.server_name)); + strecpy(item->info.hostname, info->GetHostname(), lastof(item->info.hostname)); + item->manually = info->manually; + NetworkGameListAddItemDelayed(item); + + _network_udp_mutex->BeginCritical(); + /* Init the packet */ + Packet p(PACKET_UDP_CLIENT_FIND_SERVER); + if (_udp_client_socket != NULL) _udp_client_socket->SendPacket(&p, &out_addr); + _network_udp_mutex->EndCritical(); + + delete info; +} +void NetworkUDPQueryServer(NetworkAddress address, bool manually) +{ // No UDP-socket yet.. if (!_udp_client_socket->IsConnected()) { if (!_udp_client_socket->Listen(0, 0, true)) return; } - out_addr.sin_family = AF_INET; - out_addr.sin_port = htons(address.GetPort()); - out_addr.sin_addr.s_addr = address.GetIP(); - - // Clear item in gamelist - item = NetworkGameListAddItem(address.GetIP(), address.GetPort()); - if (item == NULL) return; - - if (StrEmpty(item->info.server_name)) { - memset(&item->info, 0, sizeof(item->info)); - strecpy(item->info.server_name, address.GetHostname(), lastof(item->info.server_name)); - strecpy(item->info.hostname, address.GetHostname(), lastof(item->info.hostname)); - item->online = false; + NetworkUDPQueryServerInfo *info = new NetworkUDPQueryServerInfo(address, manually); + if (address.IsResolved() || !ThreadObject::New(NetworkUDPQueryServerThread, info)) { + NetworkUDPQueryServerThread(info); } - item->manually = manually; +} - // Init the packet - Packet p(PACKET_UDP_CLIENT_FIND_SERVER); - _udp_client_socket->SendPacket(&p, &out_addr); +void NetworkUDPRemoveAdvertiseThread(void *pntr) +{ + DEBUG(net, 1, "[udp] removing advertise from master server"); - UpdateNetworkGameWindow(false); + /* Find somewhere to send */ + struct sockaddr_in out_addr; + out_addr.sin_family = AF_INET; + out_addr.sin_port = htons(NETWORK_MASTER_SERVER_PORT); + out_addr.sin_addr.s_addr = NetworkResolveHost(NETWORK_MASTER_SERVER_HOST); + + /* Send the packet */ + Packet p(PACKET_UDP_SERVER_UNREGISTER); + /* Packet is: Version, server_port */ + p.Send_uint8 (NETWORK_MASTER_SERVER_VERSION); + p.Send_uint16(_settings_client.network.server_port); + + _network_udp_mutex->BeginCritical(); + if (_udp_master_socket != NULL) _udp_master_socket->SendPacket(&p, &out_addr); + _network_udp_mutex->EndCritical(); } /* Remove our advertise from the master-server */ void NetworkUDPRemoveAdvertise() { - struct sockaddr_in out_addr; - /* Check if we are advertising */ if (!_networking || !_network_server || !_network_udp_server) return; @@ -466,27 +512,37 @@ void NetworkUDPRemoveAdvertise() if (!_udp_master_socket->Listen(_network_server_bind_ip, 0, false)) return; } - DEBUG(net, 1, "[udp] removing advertise from master server"); + if (!ThreadObject::New(NetworkUDPRemoveAdvertiseThread, NULL)) { + NetworkUDPRemoveAdvertiseThread(NULL); + } +} +void NetworkUDPAdvertiseThread(void *pntr) +{ /* Find somewhere to send */ + struct sockaddr_in out_addr; out_addr.sin_family = AF_INET; out_addr.sin_port = htons(NETWORK_MASTER_SERVER_PORT); out_addr.sin_addr.s_addr = NetworkResolveHost(NETWORK_MASTER_SERVER_HOST); + DEBUG(net, 1, "[udp] advertising to master server"); + /* Send the packet */ - Packet p(PACKET_UDP_SERVER_UNREGISTER); - /* Packet is: Version, server_port */ + Packet p(PACKET_UDP_SERVER_REGISTER); + /* Packet is: WELCOME_MESSAGE, Version, server_port */ + p.Send_string(NETWORK_MASTER_SERVER_WELCOME_MESSAGE); p.Send_uint8 (NETWORK_MASTER_SERVER_VERSION); p.Send_uint16(_settings_client.network.server_port); - _udp_master_socket->SendPacket(&p, &out_addr); + + _network_udp_mutex->BeginCritical(); + if (_udp_master_socket != NULL) _udp_master_socket->SendPacket(&p, &out_addr); + _network_udp_mutex->EndCritical(); } /* Register us to the master server This function checks if it needs to send an advertise */ void NetworkUDPAdvertise() { - struct sockaddr_in out_addr; - /* Check if we should send an advertise */ if (!_networking || !_network_server || !_network_udp_server || !_settings_client.network.server_advertise) return; @@ -514,44 +570,37 @@ void NetworkUDPAdvertise() _network_advertise_retries--; _network_last_advertise_frame = _frame_counter; - /* Find somewhere to send */ - out_addr.sin_family = AF_INET; - out_addr.sin_port = htons(NETWORK_MASTER_SERVER_PORT); - out_addr.sin_addr.s_addr = NetworkResolveHost(NETWORK_MASTER_SERVER_HOST); - - DEBUG(net, 1, "[udp] advertising to master server"); - - /* Send the packet */ - Packet p(PACKET_UDP_SERVER_REGISTER); - /* Packet is: WELCOME_MESSAGE, Version, server_port */ - p.Send_string(NETWORK_MASTER_SERVER_WELCOME_MESSAGE); - p.Send_uint8 (NETWORK_MASTER_SERVER_VERSION); - p.Send_uint16(_settings_client.network.server_port); - _udp_master_socket->SendPacket(&p, &out_addr); + if (!ThreadObject::New(NetworkUDPAdvertiseThread, NULL)) { + NetworkUDPAdvertiseThread(NULL); + } } void NetworkUDPInitialize() { assert(_udp_client_socket == NULL && _udp_server_socket == NULL && _udp_master_socket == NULL); + _network_udp_mutex->BeginCritical(); _udp_client_socket = new ClientNetworkUDPSocketHandler(); _udp_server_socket = new ServerNetworkUDPSocketHandler(); _udp_master_socket = new MasterNetworkUDPSocketHandler(); _network_udp_server = false; _network_udp_broadcast = 0; + _network_udp_mutex->EndCritical(); } void NetworkUDPShutdown() { NetworkUDPCloseAll(); + _network_udp_mutex->BeginCritical(); delete _udp_client_socket; delete _udp_server_socket; delete _udp_master_socket; _udp_client_socket = NULL; _udp_server_socket = NULL; _udp_master_socket = NULL; + _network_udp_mutex->EndCritical(); } #endif /* ENABLE_NETWORK */ diff --git a/src/thread.h b/src/thread.h index 71041e9d7..8225df7d0 100644 --- a/src/thread.h +++ b/src/thread.h @@ -40,4 +40,27 @@ public: static bool New(OTTDThreadFunc proc, void *param, ThreadObject **thread = NULL); }; +/** + * Cross-platform Mutex + */ +class ThreadMutex { +public: + static ThreadMutex *New(); + + /** + * Virtual Destructor to avoid compiler warnings. + */ + virtual ~ThreadMutex() {}; + + /** + * Begin the critical section + */ + virtual void BeginCritical() = 0; + + /** + * End of the critical section + */ + virtual void EndCritical() = 0; +}; + #endif /* THREAD_H */ diff --git a/src/thread_none.cpp b/src/thread_none.cpp index 34f8ca1ff..f0863efaf 100644 --- a/src/thread_none.cpp +++ b/src/thread_none.cpp @@ -10,3 +10,15 @@ if (thread != NULL) *thread = NULL; return false; } + +/** Mutex that doesn't do locking because it ain't needed when there're no threads */ +class ThreadMutex_None : ThreadMutex { +public: + virtual void BeginCritical() {} + virtual void EndCritical() {} +}; + +/* static */ ThreadMutex *ThreadMutex::New() +{ + return new ThreadMutex_None; +} diff --git a/src/thread_pthread.cpp b/src/thread_pthread.cpp index 87d524784..2ab3d43f7 100644 --- a/src/thread_pthread.cpp +++ b/src/thread_pthread.cpp @@ -83,3 +83,37 @@ private: if (thread != NULL) *thread = to; return true; } + +/** + * POSIX pthread version of ThreadMutex. + */ +class ThreadMutex_pthread : public ThreadMutex { +private: + pthread_mutex_t mutex; + +public: + ThreadMutex_pthread() + { + pthread_mutex_init(&this->mutex, NULL); + } + + /* virtual */ ~ThreadMutex_pthread() + { + pthread_mutex_destroy(&this->mutex); + } + + /* virtual */ void BeginCritical() + { + pthread_mutex_lock(&this->mutex); + } + + /* virtual */ void EndCritical() + { + pthread_mutex_unlock(&this->mutex); + } +}; + +/* static */ ThreadMutex *ThreadMutex::New() +{ + return new ThreadMutex_pthread(); +} diff --git a/src/thread_win32.cpp b/src/thread_win32.cpp index 0cb2825af..882dea9ca 100644 --- a/src/thread_win32.cpp +++ b/src/thread_win32.cpp @@ -93,3 +93,37 @@ private: if (thread != NULL) *thread = to; return true; } + +/** + * Win32 thread version of ThreadMutex. + */ +class ThreadMutex_Win32 : public ThreadMutex { +private: + CRITICAL_SECTION critical_section; + +public: + ThreadMutex_Win32() + { + InitializeCriticalSection(&this->critical_section); + } + + /* virtual */ ~ThreadMutex_Win32() + { + DeleteCriticalSection(&this->critical_section); + } + + /* virtual */ void BeginCritical() + { + EnterCriticalSection(&this->critical_section); + } + + /* virtual */ void EndCritical() + { + LeaveCriticalSection(&this->critical_section); + } +}; + +/* static */ ThreadMutex *ThreadMutex::New() +{ + return new ThreadMutex_Win32(); +} -- cgit v1.2.3-54-g00ecf