From 5a82846fb0dc229d4ff3a8245f117183ce943d87 Mon Sep 17 00:00:00 2001 From: rubidium Date: Sun, 17 Nov 2013 11:24:39 +0000 Subject: [PATCH] (svn r26020) [1.3] -Backport from trunk: - Fix: [Windows] Conditional expression with enumeral with non-enumeral type (r26009) - Fix: Game script showing vehicle on e.g. a goal, then the vehicle being removed and eventually being replaced by a non-user vehicle (most likely smoke) causing an assertion to trigger [FS#5804] (r26007, r26006) - Fix: Crash when transferring savegame from server to client [FS#5478] (r26005) --- src/lang/english.txt | 3 + src/network/network_server.cpp | 129 ++++++++++++++++++++++----------- src/network/network_server.h | 3 - src/strings.cpp | 10 +-- src/video/cocoa/event.mm | 2 +- src/video/win32_v.cpp | 2 +- 6 files changed, 96 insertions(+), 53 deletions(-) diff --git a/src/lang/english.txt b/src/lang/english.txt index ddfdd6bfae..1a07af8e94 100644 --- a/src/lang/english.txt +++ b/src/lang/english.txt @@ -2804,6 +2804,9 @@ STR_NEWGRF_INVALID_CARGO_QUANTITY :{COMMA} of STR_NEWGRF_INVALID_INDUSTRYTYPE : +# Placeholders for other invalid stuff, e.g. vehicles that have gone (Game Script). +STR_INVALID_VEHICLE : + # NewGRF scanning window STR_NEWGRF_SCAN_CAPTION :{WHITE}Scanning NewGRFs STR_NEWGRF_SCAN_MESSAGE :{BLACK}Scanning NewGRFs. Depending on the amount this can take a while... diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp index bab9f4ae3d..716b41744d 100644 --- a/src/network/network_server.cpp +++ b/src/network/network_server.cpp @@ -57,30 +57,90 @@ struct PacketWriter : SaveFilter { ServerNetworkGameSocketHandler *cs; ///< Socket we are associated with. Packet *current; ///< The packet we're currently writing to. size_t total_size; ///< Total size of the compressed savegame. + Packet *packets; ///< Packet queue of the savegame; send these "slowly" to the client. + ThreadMutex *mutex; ///< Mutex for making threaded saving safe. /** * Create the packet writer. * @param cs The socket handler we're making the packets for. */ - PacketWriter(ServerNetworkGameSocketHandler *cs) : SaveFilter(NULL), cs(cs), current(NULL), total_size(0) + PacketWriter(ServerNetworkGameSocketHandler *cs) : SaveFilter(NULL), cs(cs), current(NULL), total_size(0), packets(NULL) { - this->cs->savegame_mutex = ThreadMutex::New(); + this->mutex = ThreadMutex::New(); } /** Make sure everything is cleaned up. */ ~PacketWriter() { - /* Prevent double frees. */ - if (this->cs != NULL) { - if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->BeginCritical(); - this->cs->savegame = NULL; - if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->EndCritical(); + if (this->mutex != NULL) this->mutex->BeginCritical(); - delete this->cs->savegame_mutex; - this->cs->savegame_mutex = NULL; + if (this->cs != NULL && this->mutex != NULL) { + this->mutex->WaitForSignal(); + } + + /* This must all wait until the Destroy function is called. */ + + while (this->packets != NULL) { + Packet *p = this->packets->next; + delete this->packets; + this->packets = p; } delete this->current; + + if (this->mutex != NULL) this->mutex->EndCritical(); + + delete this->mutex; + this->mutex = NULL; + } + + /** + * Begin the destruction of this packet writer. It can happen in two ways: + * in the first case the client disconnected while saving the map. In this + * case the saving has not finished and killed this PacketWriter. In that + * case we simply set cs to NULL, triggering the appending to fail due to + * the connection problem and eventually triggering the destructor. In the + * second case the destructor is already called, and it is waiting for our + * signal which we will send. Only then the packets will be removed by the + * destructor. + */ + void Destroy() + { + if (this->mutex != NULL) this->mutex->BeginCritical(); + + this->cs = NULL; + + if (this->mutex != NULL) this->mutex->SendSignal(); + + if (this->mutex != NULL) this->mutex->EndCritical(); + } + + /** + * Checks whether there are packets. + * It's not 100% threading safe, but this is only asked for when checking + * whether there still is something to send. Then another call will be made + * to actually get the Packet, which will be the only one popping packets + * and thus eventually setting this on false. + */ + bool HasPackets() + { + return this->packets != NULL; + } + + /** + * Pop a single created packet from the queue with packets. + */ + Packet *PopPacket() + { + if (this->mutex != NULL) this->mutex->BeginCritical(); + + Packet *p = this->packets; + this->packets = p->next; + p->next = NULL; + + if (this->mutex != NULL) this->mutex->EndCritical(); + + return p; } /** Append the current packet to the queue. */ @@ -88,7 +148,7 @@ struct PacketWriter : SaveFilter { { if (this->current == NULL) return; - Packet **p = &this->cs->savegame_packets; + Packet **p = &this->packets; while (*p != NULL) { p = &(*p)->next; } @@ -104,7 +164,7 @@ struct PacketWriter : SaveFilter { if (this->current == NULL) this->current = new Packet(PACKET_SERVER_MAP_DATA); - if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->BeginCritical(); + if (this->mutex != NULL) this->mutex->BeginCritical(); byte *bufe = buf + size; while (buf != bufe) { @@ -119,7 +179,7 @@ struct PacketWriter : SaveFilter { } } - if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->EndCritical(); + if (this->mutex != NULL) this->mutex->EndCritical(); this->total_size += size; } @@ -129,7 +189,7 @@ struct PacketWriter : SaveFilter { /* We want to abort the saving when the socket is closed. */ if (this->cs == NULL) SlError(STR_NETWORK_ERROR_LOSTCONNECTION); - if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->BeginCritical(); + if (this->mutex != NULL) this->mutex->BeginCritical(); /* Make sure the last packet is flushed. */ this->AppendQueue(); @@ -143,7 +203,7 @@ struct PacketWriter : SaveFilter { p->Send_uint32((uint32)this->total_size); this->cs->NetworkTCPSocketHandler::SendPacket(p); - if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->EndCritical(); + if (this->mutex != NULL) this->mutex->EndCritical(); } }; @@ -172,9 +232,10 @@ ServerNetworkGameSocketHandler::~ServerNetworkGameSocketHandler() if (_redirect_console_to_client == this->client_id) _redirect_console_to_client = INVALID_CLIENT_ID; OrderBackup::ResetUser(this->client_id); - if (this->savegame_mutex != NULL) this->savegame_mutex->BeginCritical(); - if (this->savegame != NULL) this->savegame->cs = NULL; - if (this->savegame_mutex != NULL) this->savegame_mutex->EndCritical(); + if (this->savegame != NULL) { + this->savegame->Destroy(); + this->savegame = NULL; + } /* Make sure the saving is completely cancelled. * Yes, we need to handle the save finish as well @@ -182,14 +243,6 @@ ServerNetworkGameSocketHandler::~ServerNetworkGameSocketHandler() * just be requesting the map and such. */ WaitTillSaved(); ProcessAsyncSaveFinish(); - - while (this->savegame_packets != NULL) { - Packet *p = this->savegame_packets->next; - delete this->savegame_packets; - this->savegame_packets = p; - } - - delete this->savegame_mutex; } Packet *ServerNetworkGameSocketHandler::ReceivePacket() @@ -205,13 +258,6 @@ Packet *ServerNetworkGameSocketHandler::ReceivePacket() return p; } -void ServerNetworkGameSocketHandler::SendPacket(Packet *packet) -{ - if (this->savegame_mutex != NULL) this->savegame_mutex->BeginCritical(); - this->NetworkTCPSocketHandler::SendPacket(packet); - if (this->savegame_mutex != NULL) this->savegame_mutex->EndCritical(); -} - NetworkRecvStatus ServerNetworkGameSocketHandler::CloseConnection(NetworkRecvStatus status) { assert(status != NETWORK_RECV_STATUS_OKAY); @@ -556,18 +602,14 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendMap() } if (this->status == STATUS_MAP) { - if (this->savegame_mutex != NULL) this->savegame_mutex->BeginCritical(); - bool last_packet = false; + bool has_packets = false; - for (uint i = 0; i < sent_packets && this->savegame_packets != NULL; i++) { - Packet *p = this->savegame_packets; + for (uint i = 0; (has_packets = this->savegame->HasPackets()) && i < sent_packets; i++) { + Packet *p = this->savegame->PopPacket(); last_packet = p->buffer[2] == PACKET_SERVER_MAP_DONE; - /* Remove the packet from the savegame queue and put it in the real queue. */ - this->savegame_packets = p->next; - p->next = NULL; - this->NetworkTCPSocketHandler::SendPacket(p); + this->SendPacket(p); if (last_packet) { /* There is no more data, so break the for */ @@ -575,10 +617,11 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendMap() } } - if (this->savegame_mutex != NULL) this->savegame_mutex->EndCritical(); - if (last_packet) { /* Done reading, make sure saving is done as well */ + this->savegame->Destroy(); + this->savegame = NULL; + WaitTillSaved(); /* Set the status to DONE_MAP, no we will wait for the client @@ -615,7 +658,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendMap() case SPS_ALL_SENT: /* All are sent, increase the sent_packets */ - if (this->savegame_packets != NULL) sent_packets *= 2; + if (has_packets) sent_packets *= 2; break; case SPS_PARTLY_SENT: diff --git a/src/network/network_server.h b/src/network/network_server.h index 0fc52adf38..b0a2ec461f 100644 --- a/src/network/network_server.h +++ b/src/network/network_server.h @@ -75,16 +75,13 @@ public: CommandQueue outgoing_queue; ///< The command-queue awaiting delivery int receive_limit; ///< Amount of bytes that we can receive at this moment - Packet *savegame_packets; ///< Packet queue of the savegame; send these "slowly" to the client. struct PacketWriter *savegame; ///< Writer used to write the savegame. - ThreadMutex *savegame_mutex; ///< Mutex for making threaded saving safe. NetworkAddress client_address; ///< IP-address of the client (so he can be banned) ServerNetworkGameSocketHandler(SOCKET s); ~ServerNetworkGameSocketHandler(); virtual Packet *ReceivePacket(); - virtual void SendPacket(Packet *packet); NetworkRecvStatus CloseConnection(NetworkRecvStatus status); void GetClientName(char *client_name, size_t size) const; diff --git a/src/strings.cpp b/src/strings.cpp index 5ea914035f..e092c24bcf 100644 --- a/src/strings.cpp +++ b/src/strings.cpp @@ -233,7 +233,10 @@ char *GetStringWithArgs(char *buffr, StringID string, StringParameters *args, co case 15: /* Old table for custom names. This is no longer used */ - error("Incorrect conversion of custom name string."); + if (!game_script) { + error("Incorrect conversion of custom name string."); + } + break; case GAME_TEXT_TAB: return FormatString(buffr, GetGameStringPtr(index), args, last, case_index, true); @@ -254,9 +257,6 @@ char *GetStringWithArgs(char *buffr, StringID string, StringParameters *args, co case 30: return FormatString(buffr, GetGRFStringPtr(index + 0x1000), args, last, case_index); - - case 31: - NOT_REACHED(); } if (index >= _langtab_num[tab]) { @@ -1478,7 +1478,7 @@ static char *FormatString(char *buff, const char *str_arg, StringParameters *arg StringID str; switch (v->type) { - default: NOT_REACHED(); + default: str = STR_INVALID_VEHICLE; break; case VEH_TRAIN: str = STR_SV_TRAIN_NAME; break; case VEH_ROAD: str = STR_SV_ROAD_VEHICLE_NAME; break; case VEH_SHIP: str = STR_SV_SHIP_NAME; break; diff --git a/src/video/cocoa/event.mm b/src/video/cocoa/event.mm index fc6b1bdea8..dae5253ad9 100644 --- a/src/video/cocoa/event.mm +++ b/src/video/cocoa/event.mm @@ -275,7 +275,7 @@ static void QZ_KeyEvent(unsigned short keycode, unsigned short unicode, BOOL dow if (down) { uint32 pressed_key = QZ_MapKey(keycode); /* Don't handle normal characters if an edit box has the focus. */ - if (!EditBoxInGlobalFocus() || (!IsInsideMM(pressed_key, 'A', 'Z' + 1) && !IsInsideMM(pressed_key, '0', '9' + 1))) { + if (!EditBoxInGlobalFocus() || ((pressed_key & ~WKC_SPECIAL_KEYS) <= WKC_TAB) || IsInsideMM(pressed_key & ~WKC_SPECIAL_KEYS, WKC_F1, WKC_PAUSE + 1)) { HandleKeypress(pressed_key, unicode); } DEBUG(driver, 2, "cocoa_v: QZ_KeyEvent: %x (%x), down, mapping: %x", keycode, unicode, pressed_key); diff --git a/src/video/win32_v.cpp b/src/video/win32_v.cpp index 8d50d812ad..d0f0acce8d 100644 --- a/src/video/win32_v.cpp +++ b/src/video/win32_v.cpp @@ -670,7 +670,7 @@ static LRESULT CALLBACK WndProcGdi(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lP case WM_KEYDOWN: { /* No matter the keyboard layout, we will map the '~' to the console. */ uint scancode = GB(lParam, 16, 8); - keycode = scancode == 41 ? WKC_BACKQUOTE : MapWindowsKey(wParam); + keycode = scancode == 41 ? (uint)WKC_BACKQUOTE : MapWindowsKey(wParam); /* Silently drop all messages handled by WM_CHAR. */ MSG msg;