Fix NetworkState submit/cleanup TOCTOU races

[jhugard]

Fixes #999

Summary

NetworkState did not serialize network-operation submission against the start of cleanup.
Calling HCHttpCallPerformAsync / HCWebSocketConnectAsync on one thread while another thread
runs HCCleanup / HCCleanupAsync could crash on a moved-from m_networkState, or orphan a
request that lands in the cleanup tracking sets after they were snapshotted. See the linked issue
for the full analysis.

This PR closes both facets. The public API surface is unchanged, and there is no new shared_ptr
or per-call allocation on the submission path.

Code review also found a cleanup wake-up edge case in the admission-gate implementation: a
rejected HTTP perform still runs its XAsyncOp::Cleanup provider path, and if that rejected
context is treated like a tracked request it can consume cleanup's one schedule slot or double-run
provider cleanup. The final fix only lets contexts that were actually present in
m_activeHttpRequests wake cleanup.

Bug addressed

Two facets of one root cause — submission is not serialized against cleanup-begin:

• Race A (late insert). A perform/connect whose Begin op acquires m_mutex after
  CleanupAsyncProvider::Begin took its snapshot was inserted into m_activeHttpRequests /
  m_connectingWebSockets anyway, orphaning it past cleanup: never canceled or awaited, and able
  to run against a torn-down provider.
• Race B (moved-from m_networkState). HCHttpCallPerformAsync dereferences
  httpSingleton->m_networkState while http_singleton::CleanupAsyncProvider::Begin
std::moves it out. get_http_singleton() does not increment the singleton use count, so an
  in-flight caller holding a strong reference is unsynchronized against the move — a null
  dereference / data race.

The PR also includes a code-review hardening fix for the rejection path itself: an HTTP perform
that is refused after cleanup begins is never inserted into m_activeHttpRequests, so its later
provider cleanup must reclaim only its local context and must not wake or reschedule cleanup.

What changed

Race A — admission gate (NetworkState)

• Added m_cleanupStarted, set under m_mutex in CleanupAsyncProvider::Begin alongside the
  existing snapshot.
• HttpCallPerformAsyncProvider::Begin and WebSocketConnectAsyncProvider::Begin now check
  m_cleanupStarted inside the same m_mutex critical section as the tracking-set insert. If
  cleanup has begun, the submission is refused with E_HC_NOT_INITIALISED instead of being
  inserted. Because insert and snapshot share m_mutex and the flag is set under that lock, a
  submission either lands before the snapshot (and is captured/canceled) or is cleanly refused —
  no gap.
• HttpCallPerformAsyncProvider::Cleanup now wakes cleanup only when
  m_activeHttpRequests.erase(performContext) actually removed a tracked request. Rejected
  performs still reclaim their async context, but cannot consume cleanup's schedule slot or cause
  provider cleanup to run twice because they were never admitted to the active set.

Race B — keep NetworkState owned by the singleton

• NetworkState::CleanupAsync now takes a non-owning NetworkState* and no longer transfers
  ownership; global.cpp passes singleton->m_networkState.get() instead of std::move.
• Cleanup no longer destroys NetworkState (HttpProviderCleanupComplete and the DoWork
  failure path no longer reclaim it). The instance stays owned by the http_singleton and is
  destroyed with the singleton, after the singleton's existing use_count gate confirms no other
  references remain. An in-flight caller holding a strong singleton reference keeps use_count
  above 1, so it can never observe a moved-from or destroyed m_networkState.
• Documented that gate in global.cpp, including the supporting invariant that no code may hold a
  get_http_singleton() reference across an async wait.

Tests

• TestCleanupKeepsNetworkStateForInFlightSingletonRef (Race B) — a caller holding a live
  get_http_singleton() reference asserts m_networkState stays non-null across cleanup; fails
  pre-fix when it is moved out from under that reference.
• TestHttpPerformRejectedAfterCleanupStarted (Race A) — a perform whose Begin runs after
  cleanup has begun must be refused rather than inserted into m_activeHttpRequests; fails pre-fix
  when it is orphaned past the snapshot. The test was strengthened during code review with a real
  unscheduled cleanup async block, so it also fails if a rejected perform consumes cleanup's one
  allowed schedule.
• The original reproductions are deterministic (they rely on XAsyncBegin dispatching Begin
  synchronously) and were committed first as failing tests, then made to pass by the fixes. The
  code-review cleanup wake-up regression was also committed red, then fixed green.
• Adds a TestSetCleanupStarted HC_UNITTEST_API seam used by the Race A test; the seam can also
  attach a cleanup async block so the rejection path is tested against realistic cleanup state.

Design rationale

• Why the admission gate under m_mutex? The insert and the cleanup snapshot already
  serialize on m_mutex. Setting/reading the flag in that same critical section makes the
  admission decision atomic with the insert/snapshot, closing the window without a new lock.
• m_cleanupStarted is a plain bool, not atomic. Every read and write occurs under
  m_mutex, which provides the required mutual exclusion and ordering; an atomic would be
  redundant and would misleadingly imply lockless access. The correctness of the guard depends on
  the read and the insert being in the same critical section as cleanup's write+snapshot, which
  the mutex — not an atomic — provides.
• Why gate cleanup wake-up on erase? A rejected HTTP perform still owns an async context and
  still receives XAsyncOp::Cleanup, but it was never part of the cleanup tracking set. Only an
  admitted request can be work that cleanup is waiting for, so the cleanup wake-up is tied to the
  active-set erase actually removing an entry. This prevents a refused submit from stealing the
  cleanup async block's schedule slot or invoking provider cleanup a second time.
• Why keep NetworkState owned by the singleton instead of a shared_ptr? The singleton
  already has a use_count gate that waits for all outstanding references before destruction.
  Reusing it (by not moving NetworkState out early) fixes Race B with no new shared_ptr, no
  new atomics, and no per-call cost — versus adding an in-flight submitter counter that would
  re-implement use_count, touch every public entry point, and add hot-path overhead. The
  trade-off is that NetworkState (and its already-cleaned providers) lives until the singleton is
  destroyed — a few milliseconds longer under the gate — and a documented invariant that callers
  must not hold a singleton reference across an async wait.

Public API surface

• No public HC* signatures added, removed, or changed.
• HCHttpCallPerformAsync / HCWebSocketConnectAsync behavior is unchanged in the normal case;
  when they race a concurrent cleanup they now fail deterministically with E_HC_NOT_INITIALISED
  rather than crashing or orphaning the request.

Validation

• Windows x64 Debug, libHttpClient.UnitTest.TE:
  • The two race reproductions: red on their reproduction commits → green after the fix commits.
  • Code-review regression: TestHttpPerformRejectedAfterCleanupStarted red with E_UNEXPECTED
    when the rejected perform consumed cleanup's schedule slot → green after
    Skip cleanup wakeup for rejected HTTP performs.
  • The full GlobalTests class passes 7/7, including the cleanup-focused cases
    (TestAsyncCleanup, TestAsyncCleanupWithHttpCall, TestAsyncCleanupWithHttpCallPendingRetry,
    TestHttpCallCompletionCallbackIsNotCleanupCancelable,
    TestCleanupKeepsNetworkStateForInFlightSingletonRef,
    TestHttpPerformRejectedAfterCleanupStarted).
  • Full TE suite passes on a clean rebuild (102–103/103 across runs). The only stragglers are
    load-induced global XTaskQueue teardown flakes (VerifyGlobalQueueTermination, and
    TaskQueueTests::TestCleanup XTaskQueueUninitialize(250)) that are unrelated to this change,
    pass in isolation on both main and this branch, and clear on re-run.

Scope

• No public API additions or removals.
• Change is scoped to NetworkState admission control and singleton↔NetworkState ownership; the
  cancel/await handshake for already-tracked requests is unchanged.
• No backend/provider-specific changes; all platforms pick up the fix through shared code.

Review focus

• The m_cleanupStarted guard sharing the m_mutex critical section with the tracking-set
  insert/snapshot (the core Race A fix), and the E_HC_NOT_INITIALISED rejection semantics.
• The rejected HTTP perform cleanup path: only requests actually erased from
  m_activeHttpRequests may wake cleanup.
• The ownership change: NetworkState is no longer moved out or destroyed by cleanup, and is now
  destroyed with the singleton after the use_count gate (the core Race B fix).
• The documented invariant that no code may hold a get_http_singleton() reference across an async
  wait, and whether it holds for all current callers.

[jhugard]

Identified another potential shutdown race not covered by these fixes. Closing this PR until investigation completes & will resubmit once resolved one way or the other.

[jhugard]

We feel comfortable that the other shutdown heisenbug we are seeing has a different root cause, so reopening this PR.