Skip to content

Commit bd49de4

Browse files
bmehta001Copilot
andcommitted
Hard-stop if a detached curl worker refuses to drain before teardown
These directly-constructed operations are not tracked by HttpClient_Curl::m_activeOps, so nothing else bounds the race between a lingering worker's curl_easy_cleanup and the fixture's curl_global_cleanup. DrainOperationOrDie now aborts a stuck worker and, if the operation is still alive afterward (a genuine keepalive/abort regression), records a failure and std::abort()s rather than returning into fixture teardown with an in-flight curl worker. In practice the .invalid host fails DNS in milliseconds so the operation is always gone immediately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8650ce1 commit bd49de4

1 file changed

Lines changed: 18 additions & 10 deletions

File tree

‎tests/unittests/HttpClientCurlTests.cpp‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <memory>
1818
#include <atomic>
1919
#include <thread>
20+
#include <cstdlib>
2021

2122
using namespace testing;
2223
using namespace MAT;
@@ -30,11 +31,14 @@ class HttpClientCurlTests : public ::testing::Test
3031
const std::vector<uint8_t> m_body;
3132
};
3233

33-
// Ensure a detached async operation is fully destroyed before the test returns, so the
34-
// worker's curl_easy_cleanup cannot race fixture teardown (m_client -> curl_global_cleanup).
35-
// The .invalid host fails DNS in milliseconds, so this normally completes immediately; a
36-
// stuck worker is aborted as a fallback. Returns whether the operation was destroyed.
37-
static bool DrainOperation(const std::weak_ptr<CurlHttpOperation>& weakOp)
34+
// Wait for a detached async operation to be fully destroyed before the test returns, so
35+
// the worker's curl_easy_cleanup cannot race fixture teardown (m_client ->
36+
// curl_global_cleanup). These operations are not tracked by HttpClient_Curl::m_activeOps,
37+
// so nothing else bounds that race. The .invalid host fails DNS in milliseconds, so this
38+
// normally completes immediately; a stuck worker is aborted as a fallback. If the
39+
// operation is STILL alive after that (a genuine keepalive/abort regression), hard-stop
40+
// the process rather than proceed into curl_global_cleanup with an in-flight curl worker.
41+
static void DrainOperationOrDie(const std::weak_ptr<CurlHttpOperation>& weakOp)
3842
{
3943
for (int i = 0; i < 500 && !weakOp.expired(); ++i)
4044
std::this_thread::sleep_for(std::chrono::milliseconds(10));
@@ -45,7 +49,12 @@ static bool DrainOperation(const std::weak_ptr<CurlHttpOperation>& weakOp)
4549
for (int i = 0; i < 500 && !weakOp.expired(); ++i)
4650
std::this_thread::sleep_for(std::chrono::milliseconds(10));
4751
}
48-
return weakOp.expired();
52+
if (!weakOp.expired())
53+
{
54+
ADD_FAILURE() << "detached curl worker did not terminate after abort; hard-stopping "
55+
"so curl_global_cleanup cannot run concurrently with an in-flight worker";
56+
std::abort();
57+
}
4958
}
5059

5160
// --- SetSslVerification wiring ---
@@ -200,8 +209,7 @@ TEST_F(HttpClientCurlTests, SendAsync_DestroyOnWorkerThread_NoSelfJoin)
200209
// whether the send completed: the callback sets the promise while the detached
201210
// worker still holds its self-reference, so the worker (and its curl_easy_cleanup)
202211
// can outlive this frame and race fixture teardown (m_client -> curl_global_cleanup).
203-
// Drain (with an abort fallback) so the operation is gone first.
204-
ASSERT_TRUE(DrainOperation(weakOp)) << "operation still alive after abort; worker may outlive teardown";
212+
DrainOperationOrDie(weakOp);
205213
EXPECT_TRUE(completed) << "SendAsync did not complete within 15s";
206214
}
207215

@@ -271,8 +279,8 @@ TEST_F(HttpClientCurlTests, SendAsync_NoOnDestroyDispatchAfterCompletion)
271279

272280
// Ensure the operation is destroyed before this test returns so the worker cannot
273281
// outlive fixture teardown (m_client -> curl_global_cleanup); cb is heap-owned and
274-
// captured by the worker, so it stays alive on its own. Abort a stuck worker.
275-
ASSERT_TRUE(DrainOperation(weakOp)) << "operation still alive after abort; worker may outlive teardown";
282+
// captured by the worker, so it stays alive on its own.
283+
DrainOperationOrDie(weakOp);
276284
ASSERT_TRUE(completed) << "SendAsync did not complete within 15s";
277285
// Let the destructor body finish so a missing OnDestroy guard (which would increment
278286
// the counter inside ~CurlHttpOperation) is observed rather than raced past.

0 commit comments

Comments
 (0)