Skip to content

Commit 01cc6da

Browse files
committed
[TEST] Hold what a client without a multi handle does with a request
Reporting the curl_multi_init failure does not by itself stop the client taking requests, so the case now holds what taking one leads to rather than only that the failure was reported. Raised by @lalitb in review. Measured before choosing an assertion. A client built on a null multi handle answers normally: curl_multi_perform rejects the handle, the IO loop resets it, the pending session moves to the new one and the request completes, three runs out of three with the internal log confirming the handle really was null. Gate resetMultiHandle out and the same case fails on the new assertion and then hangs, which is the outcome the assertion exists to catch. So the case asserts the caller reaches a terminal outcome and the session does not stay active, without pinning which outcome. A build where the allocation is still failing when the reset runs reports a failure instead of a response, and both satisfy the property that matters. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 1fbf309 commit 01cc6da

1 file changed

Lines changed: 71 additions & 0 deletions

File tree

‎ext/test/http/curl_http_test.cc‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,35 @@ struct FailCurlCalloc
285285
FailCurlCalloc &operator=(FailCurlCalloc &&) = delete;
286286
};
287287

288+
// Counts terminal outcomes without caring which one, since a client whose multi handle could
289+
// not be created may still recover and answer, and the case below is about the caller being
290+
// told either way rather than about which answer it gets.
291+
class MultiHandleOutcomeHandler : public http_client::EventHandler
292+
{
293+
public:
294+
void OnResponse(http_client::Response & /* response */) noexcept override
295+
{
296+
terminal_.fetch_add(1, std::memory_order_release);
297+
}
298+
299+
void OnEvent(http_client::SessionState state, nostd::string_view /* reason */) noexcept override
300+
{
301+
switch (state)
302+
{
303+
case http_client::SessionState::CreateFailed:
304+
case http_client::SessionState::ConnectFailed:
305+
case http_client::SessionState::SendFailed:
306+
case http_client::SessionState::Cancelled:
307+
terminal_.fetch_add(1, std::memory_order_release);
308+
break;
309+
default:
310+
break;
311+
}
312+
}
313+
314+
std::atomic<int> terminal_{0};
315+
};
316+
288317
class CapturingLogHandler : public opentelemetry::sdk::common::internal_log::LogHandler
289318
{
290319
public:
@@ -965,6 +994,48 @@ TEST_F(BasicCurlHttpTests, AFailedMultiHandleAllocationIsReported)
965994
<< "a multi handle that could not be created was not reported, captured: " << text;
966995
}
967996

997+
// Reporting the failure does not by itself stop the client taking requests, so this holds what
998+
// taking one leads to. The IO loop resets the multi handle whenever curl_multi_perform rejects
999+
// it, so a client built on a null handle repairs itself and answers; if the allocation is still
1000+
// failing by then it reports a failure instead. What must not happen is neither.
1001+
TEST_F(BasicCurlHttpTests, AClientWithoutAMultiHandleStillAnswers)
1002+
{
1003+
ASSERT_TRUE(g_curl_hooks_installed);
1004+
received_requests_.clear();
1005+
1006+
{
1007+
auto warmup = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();
1008+
ASSERT_TRUE(warmup != nullptr);
1009+
warmup->FinishAllSessions();
1010+
}
1011+
1012+
std::shared_ptr<http_client::HttpClient> client;
1013+
{
1014+
FailCurlCalloc fail;
1015+
client = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();
1016+
}
1017+
ASSERT_TRUE(client != nullptr);
1018+
1019+
auto session = client->CreateSession("http://127.0.0.1:19000");
1020+
auto request = session->CreateRequest();
1021+
request->SetUri("get/");
1022+
1023+
auto handler = std::make_shared<MultiHandleOutcomeHandler>();
1024+
session->SendRequest(handler);
1025+
1026+
for (int i = 0; i < 300 && 0 == handler->terminal_.load(std::memory_order_acquire); ++i)
1027+
{
1028+
std::this_thread::sleep_for(std::chrono::milliseconds(100));
1029+
}
1030+
1031+
EXPECT_GE(handler->terminal_.load(std::memory_order_acquire), 1)
1032+
<< "the request was accepted and never reached an outcome";
1033+
EXPECT_FALSE(session->IsSessionActive()) << "the session stayed active with nothing running it";
1034+
1035+
session->FinishSession();
1036+
client->FinishAllSessions();
1037+
}
1038+
9681039
TEST_F(BasicCurlHttpTests, SendGetRequestSync)
9691040
{
9701041
received_requests_.clear();

0 commit comments

Comments
 (0)