fix: prevent EDEADLK self-join in ~CurlHttpOperation on async-thread destruction

[bmehta001]

Problem

Linux and macOS module tests abort with:

terminate called after throwing an instance of 'std::system_error'
  what(): Resource deadlock avoided

Root cause

CurlHttpOperation::SendAsync used std::async and stored its std::future in the operation. The completion callback can release the request's last reference to the operation on the async worker itself. The operation destructor then destroys the future on that same worker; libstdc++ tries to join the async thread from itself, throws EDEADLK, and terminates through the destructor's noexcept boundary.

Fix

Replace the joining future with one worker thread owned by CurlHttpOperation:

• normal destruction joins the worker, preserving the original lifetime barrier;
• destruction from the worker callback detaches instead of self-joining;
• a small start gate ensures a fast worker cannot destroy the operation before its std::thread has been published to the member;
• OnDestroy is delivered exactly once before the completion callback, while the IHttpResponseCallback is still valid;
• worker exceptions are contained because an exception escaping a raw std::thread would terminate the process;
• callable-copy or thread-creation failures report CURLE_FAILED_INIT through the normal completion callback instead of escaping;
• a permanent send-attempt flag keeps operations single-use even when worker construction fails.

The request body remains referenced rather than copied: CurlHttpRequest destroys its operation member before inherited request-body storage, and cross-thread operation destruction joins the worker before that storage is released.

This removes the detached self-keepalive, shared client state, in-flight tracker, shutdown timeout/abandonment path, body copy, and synchronous fallback introduced by earlier revisions. There is no public SDK API change.

Verification

• Current-main full unit suite: 536/536 passed.
• Current-main curl and response-cap groups: 15/15 passed.
• Current-main self-join regression stress: 200 repeated runs passed.
• Current-main callable-copy/thread-start failure stress: 200 repeated runs passed.
• AddressSanitizer full suite before the conflict-free main merge: 527/527 passed; subsequent targeted AddressSanitizer curl and stress runs also passed.
• No AddressSanitizer findings.
• Final material review: no significant findings.

The focused regression drops the operation's last owner from inside its worker callback, reproducing the original destruction path directly.

[Copilot]

Pull request overview

This PR addresses a Linux/macOS crash caused by std::future’s implicit join throwing on self-join when ~CurlHttpOperation runs on the same async thread created by std::async.

Changes:

• Adds tracking of the async task’s thread id to detect destructor execution on the async thread.
• Updates ~CurlHttpOperation to avoid self-join by deferring std::future destruction to a detached helper thread.
• Updates comments in the curl HTTP client to document the lifetime/join behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/http/HttpClient_Curl.hpp	Adds async thread id tracking and modifies destructor logic to avoid std::future self-join aborts.
lib/http/HttpClient_Curl.cpp	Updates documentation comment describing the lifetime guarantees of CurlHttpOperation across async send.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

lib/http/HttpClient_Curl.hpp:352

• SendAsync() assigns m_worker = std::thread(...) without guarding thread construction. std::thread construction can throw (e.g., std::system_error on resource exhaustion, or std::bad_alloc while allocating/copying the callable), and this will currently escape SendAsync(); HttpClient_Curl::SendRequestAsync() does not catch, so this can terminate the process and/or violate the expectation that async sends always eventually invoke the completion path. Consider wrapping thread creation in a try/catch and falling back to running Send() + invoking the completion callback synchronously (or reporting a failure result and invoking the callback), so nothing escapes from SendAsync().

        m_worker = std::thread([this, callback]() {
            {
                std::lock_guard<std::mutex> startGuard(m_workerStartMtx);
            }

lib/http/HttpClient_Curl.hpp:200

• The destructor now suppresses DispatchEvent(OnDestroy) once m_completed is true. This means the OnDestroy state event is no longer guaranteed to be delivered for successfully completed operations, which contradicts the HTTP state machine documented in lib/include/public/IHttpClient.hpp ("destroy -> [OnDestroy]"). If OnDestroy must be suppressed to avoid calling into a callback that may have been deleted by the completion path, consider either (1) updating the documented contract, or (2) redesigning state-event delivery so OnDestroy can still be reported without dereferencing a potentially-freed IHttpResponseCallback (e.g., route state events to a stable sink that outlives the request/callback).

        // The completion callback may destroy m_callback. SendAsync marks completion
        // before invoking it, so do not dispatch through that pointer afterward.
        if (!m_completed.load(std::memory_order_acquire))
        {
            DispatchEvent(OnDestroy);

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/unittests/HttpClientCurlTests.cpp:22

• This test file uses std::function (later in SendAsync_CallbackCopyFailureStillCompletes) but does not include <functional>, relying on transitive includes. Adding the direct include makes the test more robust to future header/include changes.

#include <future>
#include <chrono>
#include <memory>
#include <atomic>
#include <cstdlib>
#include <stdexcept>
#include <utility>

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.