[BUG] WinHttpRequest destructor blocks forever when the request is destroyed before `WinHttpSendRequest` binds the context

[dcristoloveanu]

Library name and version: Azure Core 1.16.3 (also present on main @ 426fb110f)

Describe the bug

WinHttpRequest::~WinHttpRequest() closes the request handle and then waits for
WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING before returning, so that no WinHTTP worker thread can
dereference the WinHttpAction after it is freed. That barrier is correct in principle, but it can
never be satisfied if the request is destroyed before WinHttpSendRequest() runs — and in that
case the calling thread blocks permanently.

The wait is unbounded and uncancellable, so the thread is lost for the lifetime of the process.

Root cause

Three pieces of sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp interact:

1. The status callback discards every notification that has no context:

void WinHttpAction::StatusCallback(
    HINTERNET hInternet, DWORD_PTR dwContext, DWORD internetStatus, ...)
{
  // If we're called before our context has been set (on Open and Close callbacks), ignore the
  // status callback.
  if (dwContext == 0)
  {
    return;
  }

2. The context is bound only by WinHttpSendRequest(), but the callback is registered in the
constructor — so there is a window in which the handle has a callback but dwContext == 0:

// WinHttpRequest::WinHttpRequest(), last statements of the constructor
m_httpAction = std::make_unique<_detail::WinHttpAction>(this);

if (!m_httpAction->RegisterWinHttpStatusCallback(m_requestHandle))
{
  GetErrorAndThrow("Error while setting up the status callback.");
}

// WinHttpRequest::SendRequest() - the only place the context is associated with the handle
WinHttpSendRequest(
    m_requestHandle.get(), ..., reinterpret_cast<DWORD_PTR>(m_httpAction.get()));

3. The destructor waits for a notification that step 1 will discard:

WinHttpRequest::~WinHttpRequest()
{
  if (!m_requestHandleClosed)
  {
    Log::Write(Logger::Level::Informational,
        "WinHttpRequest::~WinHttpRequest. Closing handle synchronously.");

    if (!m_httpAction->WaitForAction(
            [this]() { /* WinHttpCloseHandle(...) */ },
            WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING,
            Azure::Core::Context{}))       // no deadline, never cancelled

WaitForAction loops on WaitForSingleObject(..., pollDuration) and only leaves the loop through
context.ThrowIfCancelled(), which is a no-op for a default-constructed Context. So when
HANDLE_CLOSING arrives with dwContext == 0 and is dropped, CompleteAction() never runs, the
event is never signalled, and the loop spins forever.

The usual escape hatch is also closed: WinHTTP reports WINHTTP_CALLBACK_STATUS_REQUEST_ERROR /
ERROR_WINHTTP_OPERATION_CANCELLED when a handle with a pending operation is closed, but
CompleteActionWithError() deliberately does not signal while m_expectedStatus == WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING.

Anything that throws between the constructor completing and WinHttpSendRequest() being called
lands in this window. In SendRequest() that includes the header preparation
(GetHeadersAsString(), StringToWideString()) and request.GetBodyStream()->Length().
We hit it in production under memory pressure, where an allocation during request setup threw
std::bad_alloc and permanently leaked the calling thread.

Log signature

A healthy teardown always emits both lines within microseconds (this pairing is visible, for
example, in the CI logs attached to #5151):

DEBUG : WinHttpRequest::~WinHttpRequest. Closing handle synchronously.
INFO  : Status operation: 2048(WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING )
DEBUG : Closing handle; completing outstanding Close request
INFO  : WinHttpRequest::~WinHttpRequest. Handle closed.

A hung teardown emits only the first line, and the thread produces no further output ever again:

INFO  : WinHttpRequest::~WinHttpRequest. Closing handle synchronously.
<nothing - thread is blocked permanently>

To Reproduce

Deterministic, no network required, and reachable entirely through the public API.
SendRequest() calls request.GetBodyStream()->Length() before WinHttpSendRequest(), and
BodyStream::Length() is not noexcept, so a body stream that throws from Length() enters the
window exactly:

class ThrowingLengthBodyStream final : public Azure::Core::IO::BodyStream {
public:
  int64_t Length() const override { throw std::runtime_error("injected"); }
private:
  size_t OnRead(uint8_t*, size_t, Azure::Core::Context const&) override { return 0; }
};

Azure::Core::Http::WinHttpTransport transport;
ThrowingLengthBodyStream bodyStream;
Azure::Core::Http::Request request(
    Azure::Core::Http::HttpMethod::Put,
    Azure::Core::Url("https://localhost/"),
    &bodyStream);

Azure::Core::Context context;
transport.Send(request, context);   // <-- never returns

WinHttpOpen() / WinHttpConnect() / WinHttpOpenRequest() only allocate handles, so nothing is
ever put on the wire and the URL does not need to resolve.

Observed: the call never returns. A leak checker additionally reports the abandoned
WinHttpRequest allocated in WinHttpTransportImpl::CreateRequestHandle(), which is the object
whose destructor is stuck.

Expected behavior

Destroying a WinHttpRequest completes promptly regardless of how far the request progressed, and
Send() propagates the original failure.

Suggested fix

Associate the context with the handle in the constructor, using
WinHttpSetOption(WINHTTP_OPTION_CONTEXT_VALUE, ...), so HANDLE_CLOSING is always delivered with
a valid context. WinHttpSendRequest() then sets the same value idempotently and the existing
barrier keeps working in every state.

An alternative would be to track whether the context was ever bound and skip the wait when it was
not — safe for the same reason the bug exists (with no context, every callback is already dropped,
so there is nothing to synchronize against) — but it leaves HANDLE_CLOSING unobservable.

Note that simply adding a timeout to the destructor's wait would be unsafe: abandoning the wait
allows a WinHTTP worker thread to invoke the callback after WinHttpAction and WinHttpRequest
have been freed, turning a hang into a use-after-free.

Related

• Fixed WinHTTP failfast caused by premature cancellation #6637 fixed a FailFast in this same function for the closely related case of "a request is
  cancelled before the request is actually sent on the wire", and its comment already notes that
  WinHttpSendRequest is what establishes the context. This issue is the hang that remains in that
  window when the throw happens before WaitForAction is reached.
• TransportAdapterOptions.DisableCrlValidation test is failing intermittently with a timeout error, on Win2022_Win32Api_debug_tests_winhttp_x86 and x64 #5151 contains CI logs showing the healthy HANDLE_CLOSING sequence for contrast.

Setup

• OS: Windows Server 2022 / Windows 11
• Compiler: MSVC
• Transport: WinHTTP (BUILD_TRANSPORT_WINHTTP_ADAPTER)
• azure-core: 1.16.3 (vcpkg), and confirmed by inspection on main @ 426fb110f