From 29ca6bbd3488aa8ffa574bd61dfeea412c970cbf Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Sun, 19 Nov 2023 06:59:04 -0600 Subject: [PATCH 1/3] RemoteServer: reset fail counter after success should close #4040 --- library/RemoteServer.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/library/RemoteServer.cpp b/library/RemoteServer.cpp index 72f2a29b5..58a6f52ba 100644 --- a/library/RemoteServer.cpp +++ b/library/RemoteServer.cpp @@ -80,7 +80,7 @@ namespace { struct BlockedException : std::exception { const char* what() const noexcept override { - return "Core has blocked all connection. This should have been catched."; + return "Core has blocked all connection. This should have been caught."; } }; } @@ -478,23 +478,26 @@ void ServerMainImpl::threadFn(std::promise promise, int port) CActiveSocket *client = nullptr; try { - for (int acceptFail = 0 ; server.socket.IsSocketValid() && acceptFail < 5 ; acceptFail++) + for (int acceptFail = 0; server.socket.IsSocketValid() && acceptFail < 5; acceptFail++) { if ((client = server.socket.Accept()) != NULL) { BlockGuard lock; ServerConnection::Accepted(client); client = nullptr; + acceptFail = 0; } else { WARN(socket).print("Connection failure: %s (%d of %d)\n", server.socket.DescribeError(), acceptFail + 1, 5); } } - } catch(BlockedException &) { + } + catch (BlockedException&) { if (client) client->Close(); delete client; + WARN(socket).print("Connection failure: unexpectedly blocked"); } if (server.socket.IsSocketValid()) From 77ce84ca0a6c4129f1c85f9bdff056c95da02cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= Date: Sun, 19 Nov 2023 14:46:14 +0100 Subject: [PATCH 2/3] Improve Accept error handling in RemoteServer Also makes sure the socket is blocking to avoid EWOULDBLOCK errors. --- library/RemoteServer.cpp | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/library/RemoteServer.cpp b/library/RemoteServer.cpp index 72f2a29b5..192ead947 100644 --- a/library/RemoteServer.cpp +++ b/library/RemoteServer.cpp @@ -475,35 +475,28 @@ void ServerMainImpl::threadFn(std::promise promise, int port) { ServerMainImpl server{std::move(promise), port}; - CActiveSocket *client = nullptr; - + server.socket.SetBlocking(); try { - for (int acceptFail = 0 ; server.socket.IsSocketValid() && acceptFail < 5 ; acceptFail++) - { - if ((client = server.socket.Accept()) != NULL) - { + while (server.socket.IsSocketValid()) { + if (std::unique_ptr client{server.socket.Accept()}) { BlockGuard lock; - ServerConnection::Accepted(client); - client = nullptr; + ServerConnection::Accepted(client.release()); } - else - { - WARN(socket).print("Connection failure: %s (%d of %d)\n", server.socket.DescribeError(), acceptFail + 1, 5); + else switch (server.socket.GetSocketError()) { + case CSimpleSocket::SocketInvalidSocket: + WARN(socket).print("Listening socket invalid, shutting down RemoteServer\n"); + server.socket.Close(); + break; + case CSimpleSocket::SocketFirewallError: + case CSimpleSocket::SocketProtocolError: + WARN(socket).print("Connection failed: %s\n", server.socket.DescribeError()); + break; + default: + break; } } - } catch(BlockedException &) { - if (client) - client->Close(); - delete client; - } - - if (server.socket.IsSocketValid()) - { - WARN(socket).print("Too many failed accepts, shutting down RemoteServer\n"); } - else - { - WARN(socket).print("Listening socket invalid, shutting down RemoteServer\n"); + catch(BlockedException &) { } } From 9a62a63f89dae56045e2e9da67b8394d23846f19 Mon Sep 17 00:00:00 2001 From: Myk Taylor Date: Mon, 20 Nov 2023 00:09:30 -0800 Subject: [PATCH 3/3] update changelog --- docs/changelog.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.txt b/docs/changelog.txt index b3c73f3b7..1d19d3f74 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -57,6 +57,7 @@ Template for new versions: ## Fixes - `buildingplan`: fix choosing the wrong mechanism (or something that isn't a mechanism) when linking a lever and manually choosing a mechanism, but then canceling the selection +- RemoteServer: don't shut down the socket prematurely, allowing continuing connections from, for example, dfhack-run ## Misc Improvements - `buildingplan`: save magma safe mechanisms for when magma safety is requested when linking levers and pressure plates to targets