From 1b6d705c63cb4545eece0aa53de0645a8e5086d6 Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Wed, 11 Feb 2026 11:07:41 +0100 Subject: [PATCH 1/3] Add a test to verify shutting down a TCPServer works correctly A connected client should be aware that the server went away of the server object is destroyed. --- tests/test_tcp_server.cpp | 48 +++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/tests/test_tcp_server.cpp b/tests/test_tcp_server.cpp index 77fb916a9..45d701ee2 100644 --- a/tests/test_tcp_server.cpp +++ b/tests/test_tcp_server.cpp @@ -65,11 +65,13 @@ class TCPServerTest : public ::testing::Test size_t read_chars = 99; while (read_chars > 0) { - TCPSocket::read((uint8_t*)&character, 1, read_chars); - result << character; - if (character == '\n') + if (TCPSocket::read((uint8_t*)&character, 1, read_chars)) { - break; + result << character; + if (character == '\n') + { + break; + } } } return result.str(); @@ -333,6 +335,44 @@ TEST_F(TCPServerTest, check_address_already_in_use) EXPECT_THROW(comm::TCPServer test_server(12321, 2, std::chrono::milliseconds(500)), std::system_error); } +TEST_F(TCPServerTest, check_shutting_down_server_while_listening) +{ + auto server = std::make_unique(port_); + server->start(); + + // Use a client with a read timeout so we don't hang forever if the server doesn't shut down + // properly. + Client client(port_); + timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 100000; // 100 ms + client.setReceiveTimeout(tv); + + // Start reading data with the client in a separate thread. This will have a blocking recv() call + // that should be interrupted when the server is shut down. In that case the + bool read_success = true; + std::thread read_data_thread([&client, &read_success]() { + while (read_success) + { + // As we aren't sending any data, this will block until the server is shut down and the + // connection is closed. At that point, recv() should return an empty string, which we + // interpret as a successful shutdown of the server. + read_success = (client.recv() != ""); + } + }); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + server.reset(); + + if (read_data_thread.joinable()) + { + read_data_thread.join(); + } + EXPECT_FALSE(read_success); + EXPECT_EQ(client.getState(), comm::SocketState::Disconnected); +} + int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); From c7e40a2e2ec81cf4dbd98ebba700bba01c612401 Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Wed, 11 Feb 2026 11:11:48 +0100 Subject: [PATCH 2/3] TCPServer: Close all filedescriptors on shutdown This fixes a bug where clients would not have been aware that the server went away, as we just closed the listen filedescriptor, but not the one attached to a client. --- src/comm/tcp_server.cpp | 8 +++++++- tests/test_tcp_server.cpp | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/comm/tcp_server.cpp b/src/comm/tcp_server.cpp index cb58252f7..8b5797746 100644 --- a/src/comm/tcp_server.cpp +++ b/src/comm/tcp_server.cpp @@ -58,7 +58,6 @@ TCPServer::~TCPServer() { URCL_LOG_DEBUG("Destroying TCPServer object."); shutdown(); - ur_close(listen_fd_); } void TCPServer::init() @@ -115,6 +114,13 @@ void TCPServer::shutdown() worker_thread_.join(); URCL_LOG_DEBUG("Worker thread joined."); } + + for (const auto& client_fd : client_fds_) + { + ur_close(client_fd); + } + ur_close(shutdown_socket); + ur_close(listen_fd_); } void TCPServer::bind(const size_t max_num_tries, const std::chrono::milliseconds reconnection_time) diff --git a/tests/test_tcp_server.cpp b/tests/test_tcp_server.cpp index 45d701ee2..6abfab38d 100644 --- a/tests/test_tcp_server.cpp +++ b/tests/test_tcp_server.cpp @@ -370,6 +370,7 @@ TEST_F(TCPServerTest, check_shutting_down_server_while_listening) read_data_thread.join(); } EXPECT_FALSE(read_success); + // If the read just would have timeouted, the client state would still be connected. EXPECT_EQ(client.getState(), comm::SocketState::Disconnected); } From 67a414596a1ba39e9f7ffa7e736460f5f3d39b9d Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Wed, 11 Feb 2026 12:23:44 +0100 Subject: [PATCH 3/3] Increase timeout in test The timeout of 100ms was a bit optimistic depending on the system that we run on. Since it's only the worst-case scenario, so the test will only take that much time if it actually fails, having this quite high should be alright. --- tests/test_tcp_server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_tcp_server.cpp b/tests/test_tcp_server.cpp index 6abfab38d..561131418 100644 --- a/tests/test_tcp_server.cpp +++ b/tests/test_tcp_server.cpp @@ -344,8 +344,8 @@ TEST_F(TCPServerTest, check_shutting_down_server_while_listening) // properly. Client client(port_); timeval tv; - tv.tv_sec = 0; - tv.tv_usec = 100000; // 100 ms + tv.tv_sec = 10; + tv.tv_usec = 0; // 100 ms client.setReceiveTimeout(tv); // Start reading data with the client in a separate thread. This will have a blocking recv() call