From f4216e6cbb00f6650b4f000d1a182122737ee239 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Aug 2026 10:50:37 +0200 Subject: [PATCH 1/2] test(http-server): guard the prompt stop after a kept-alive request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A kept-alive connection used to hold the accept loop until it idled out, so `stop()` spanned cpp-httplib's 5 s keep-alive timeout — on the path every consumer takes, potentially on the main thread while a document closes. The 0.47.0 bump (#653) fixed it upstream: the keep-alive wait now watches the listening socket, so closing it ends the wait. Nothing said so, so nothing would notice it coming back. This serves a request from a client that keeps the connection open, then times `stop()`: 12 ms here, against the 5.01 s the issue measured. `odr_test` links `httplib::httplib` for the client. Closes #641 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XDs5aK3ZGSZsEvqUUwBBXU --- test/CMakeLists.txt | 3 +++ test/src/http_server_test.cpp | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4e8b5b10a..52028f2fd 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -112,6 +112,9 @@ if (ODR_WITH_HTTP_SERVER) PRIVATE "src/http_server_test.cpp" ) + # a client to make a request with, so the server can be tested serving one + find_package(httplib REQUIRED) + target_link_libraries(odr_test PRIVATE httplib::httplib) endif () target_include_directories(odr_test PRIVATE diff --git a/test/src/http_server_test.cpp b/test/src/http_server_test.cpp index 25c99cd79..eed2819cd 100644 --- a/test/src/http_server_test.cpp +++ b/test/src/http_server_test.cpp @@ -1,6 +1,8 @@ #include #include +#include + #include #include @@ -150,3 +152,33 @@ TEST(HttpServer, listen_after_stop_returns) { // to serve, but nothing went wrong either EXPECT_NO_THROW(server.listen()); } + +/// A kept-alive connection used to hold the accept loop until it idled out, so +/// stop() spanned cpp-httplib's 5 s keep-alive timeout - the whole of it, on +/// the path every consumer takes (#641). Fixed upstream in 0.47.0, where the +/// keep-alive wait watches the listening socket; this is what says so. +TEST(HttpServer, stop_is_prompt_after_serving_a_kept_alive_request) { + const HttpServer server; + + const std::uint32_t port = server.bind("127.0.0.1", 0); + std::thread thread{[&server] { server.listen(); }}; + wait_until_running(server); + + // alive across the stop() below, so the connection it holds is open there + httplib::Client client{"127.0.0.1", static_cast(port)}; + client.set_keep_alive(true); + const httplib::Result response = client.Get("/"); + ASSERT_TRUE(response); + EXPECT_EQ(response->status, 200); + + const auto before = std::chrono::steady_clock::now(); + server.stop(); + const auto elapsed = std::chrono::steady_clock::now() - before; + + thread.join(); + + EXPECT_LT(elapsed, 2s) + << "stop() took " + << std::chrono::duration_cast(elapsed).count() + << " ms"; +} From abdf28807cb3bdaf56890d7d42422d77e1784837 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Aug 2026 11:56:21 +0200 Subject: [PATCH 2/2] test(http-server): stop and join the listener however the test leaves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ASSERT_TRUE(response)` returns from the test with the listen thread still joinable, and `~std::thread` then calls `std::terminate` — a transient socket error would abort the whole binary instead of reporting one failing test. A guard now stops the server and joins on every path; a second `stop()` is harmless, as the test above it says. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XDs5aK3ZGSZsEvqUUwBBXU --- test/src/http_server_test.cpp | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/test/src/http_server_test.cpp b/test/src/http_server_test.cpp index eed2819cd..9e3e985ec 100644 --- a/test/src/http_server_test.cpp +++ b/test/src/http_server_test.cpp @@ -22,6 +22,31 @@ void wait_until_running(const HttpServer &server) { } } +/// Stops the server and joins the thread however the test leaves - a fatal +/// assertion returns, and a joinable `std::thread` destructor would take the +/// whole test binary down with `std::terminate` instead of reporting it. +class ListenGuard { +public: + ListenGuard(const HttpServer &server, std::thread thread) + : m_server{&server}, m_thread{std::move(thread)} {} + + ~ListenGuard() { + m_server->stop(); + if (m_thread.joinable()) { + m_thread.join(); + } + } + + ListenGuard(const ListenGuard &) = delete; + ListenGuard &operator=(const ListenGuard &) = delete; + ListenGuard(ListenGuard &&) = delete; + ListenGuard &operator=(ListenGuard &&) = delete; + +private: + const HttpServer *m_server; + std::thread m_thread; +}; + } // namespace TEST(HttpServer, bind_reports_the_port_it_got) { @@ -161,7 +186,8 @@ TEST(HttpServer, stop_is_prompt_after_serving_a_kept_alive_request) { const HttpServer server; const std::uint32_t port = server.bind("127.0.0.1", 0); - std::thread thread{[&server] { server.listen(); }}; + // stops and joins whatever happens below, including a fatal assertion + const ListenGuard guard{server, std::thread{[&server] { server.listen(); }}}; wait_until_running(server); // alive across the stop() below, so the connection it holds is open there @@ -175,8 +201,6 @@ TEST(HttpServer, stop_is_prompt_after_serving_a_kept_alive_request) { server.stop(); const auto elapsed = std::chrono::steady_clock::now() - before; - thread.join(); - EXPECT_LT(elapsed, 2s) << "stop() took " << std::chrono::duration_cast(elapsed).count()