Skip to content

Commit 08a10ef

Browse files
committed
[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>
1 parent 312acb6 commit 08a10ef

1 file changed

Lines changed: 14 additions & 7 deletions

File tree

‎ext/test/http/curl_http_test.cc‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -926,8 +926,12 @@ TEST_F(BasicCurlHttpTests, FinishInAsyncCallback)
926926
}
927927
}
928928

929+
// Destroying the client wakes the polling background thread instead of letting it sleep out
930+
// scheduled_delay_milliseconds_. A missed wakeup is slow rather than wrong, so the bound is the
931+
// assertion: measured, the quit is under a millisecond and a slept out poll is 256 ms.
929932
TEST_F(BasicCurlHttpTests, ElegantQuitQuick)
930933
{
934+
received_requests_.clear();
931935
auto http_client = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();
932936
std::static_pointer_cast<curl::HttpClient>(http_client)->MaybeSpawnBackgroundThread();
933937
// start background first, then test it could wakeup
@@ -936,19 +940,22 @@ TEST_F(BasicCurlHttpTests, ElegantQuitQuick)
936940
request->SetUri("get/");
937941
auto handler = std::make_shared<GetEventHandler>();
938942
session->SendRequest(handler);
939-
std::this_thread::sleep_for(std::chrono::milliseconds{10}); // let it enter poll state
943+
944+
// Sending is not what is timed, so a slow request must not read as a slow quit.
945+
ASSERT_TRUE(waitForRequests(30, 1));
946+
session->FinishSession();
947+
ASSERT_TRUE(handler->is_called_.load(std::memory_order_acquire));
948+
ASSERT_TRUE(handler->got_response_.load(std::memory_order_acquire));
949+
940950
auto beg = std::chrono::system_clock::now();
941951
http_client->FinishAllSessions();
942952
http_client.reset();
943-
// when background_thread_wait_for_ is used, it should have no side effect on elegant quit
944-
// wait should be less than scheduled_delay_milliseconds_
945-
// Due to load on CI hosts (some take 10ms), we assert it is less than 20ms
946953
auto cost = std::chrono::system_clock::now() - beg;
947-
ASSERT_TRUE(cost < std::chrono::milliseconds{20})
954+
955+
// background_thread_wait_for_ keeps the thread alive here and must not delay the quit.
956+
ASSERT_TRUE(cost < std::chrono::milliseconds{100})
948957
<< "cost ms: " << std::chrono::duration_cast<std::chrono::milliseconds>(cost).count()
949958
<< " libcurl version: 0x" << std::hex << LIBCURL_VERSION_NUM;
950-
ASSERT_TRUE(handler->is_called_);
951-
ASSERT_TRUE(handler->got_response_);
952959
}
953960

954961
TEST_F(BasicCurlHttpTests, BackgroundThreadWaitMore)

0 commit comments

Comments
 (0)