From eafad1164d70bb86a4926d6113ad80636e12c3d3 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:57:10 +0000 Subject: [PATCH] [TEST] Time only the quit in ElegantQuitQuick, and bound it where the two outcomes are The case exists to catch a destruction that lets the polling background thread sleep out scheduled_delay_milliseconds_ instead of waking it. Measured on an idle host, that quit costs 0.1 to 0.5 ms; measured with the wakeup removed, it costs 256.5 to 256.9 ms, and there is nothing in between. The bound sat at 20 ms, which is close enough to the fast outcome that a host adding scheduling delay to a sub millisecond operation trips it. The 21 ms failure reported on #4265 is that, and the 247 ms one is a quit that really did wait the poll out. 100 ms partitions the same two sets: over 24 runs of each arm, no measurement landed between 20 and 100 ms, so the wider bound gives up no detection. The window now covers the quit alone. The request is finished first through the file's own waitForRequests(), which also removes the fixed 10 ms sleep that was standing in for it. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- ext/test/http/curl_http_test.cc | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 90142962d6..9a5f584139 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -919,8 +919,12 @@ TEST_F(BasicCurlHttpTests, FinishInAsyncCallback) } } +// Destroying the client wakes the polling background thread instead of letting it sleep out +// scheduled_delay_milliseconds_. A missed wakeup is slow rather than wrong, so the bound is the +// assertion: measured, the quit is under a millisecond and a slept out poll is 256 ms. TEST_F(BasicCurlHttpTests, ElegantQuitQuick) { + received_requests_.clear(); auto http_client = std::make_shared()->Create(); std::static_pointer_cast(http_client)->MaybeSpawnBackgroundThread(); // start background first, then test it could wakeup @@ -929,19 +933,22 @@ TEST_F(BasicCurlHttpTests, ElegantQuitQuick) request->SetUri("get/"); auto handler = std::make_shared(); session->SendRequest(handler); - std::this_thread::sleep_for(std::chrono::milliseconds{10}); // let it enter poll state + + // Sending is not what is timed, so a slow request must not read as a slow quit. + ASSERT_TRUE(waitForRequests(30, 1)); + session->FinishSession(); + ASSERT_TRUE(handler->is_called_.load(std::memory_order_acquire)); + ASSERT_TRUE(handler->got_response_.load(std::memory_order_acquire)); + auto beg = std::chrono::system_clock::now(); http_client->FinishAllSessions(); http_client.reset(); - // when background_thread_wait_for_ is used, it should have no side effect on elegant quit - // wait should be less than scheduled_delay_milliseconds_ - // Due to load on CI hosts (some take 10ms), we assert it is less than 20ms auto cost = std::chrono::system_clock::now() - beg; - ASSERT_TRUE(cost < std::chrono::milliseconds{20}) + + // background_thread_wait_for_ keeps the thread alive here and must not delay the quit. + ASSERT_TRUE(cost < std::chrono::milliseconds{100}) << "cost ms: " << std::chrono::duration_cast(cost).count() << " libcurl version: 0x" << std::hex << LIBCURL_VERSION_NUM; - ASSERT_TRUE(handler->is_called_); - ASSERT_TRUE(handler->got_response_); } TEST_F(BasicCurlHttpTests, BackgroundThreadWaitMore)