Fix runtime data races, memory leak, and shutdown safety

[bmehta001]

Fixes runtime thread-safety, shutdown-safety, response-lifetime, and resource-leak issues that affect normal SDK operation. Split out from #1415 per reviewer request so runtime behavior changes stay separate from CI/build/test fixes.

Fix HTTP handle cleanup

HttpClient_WinInet.cpp

• Close the WinInet session handle under its own null check. Previously m_hWinInetSession was closed only inside the if (m_hWinInetRequest != nullptr) block, so if HttpOpenRequestA failed after InternetConnectA succeeded (session set, request null) the session handle leaked.

Fix HTTP response lifetime on abort/network-failure paths

HttpResponseDecoder.cpp

• Preserve ctx->httpResponse through requestAborted(ctx) and temporaryNetworkFailure(ctx) so downstream storage/statistics handlers can still read status and headers.
• Avoid leaking aborted/network-failure responses by keeping ownership with EventsUploadContext, whose clear() path deletes the response.

HttpResponseDecoderTests.cpp

• Add regression coverage that aborted and network-failure decode routes still receive the response object with result/status intact.

Fix WorkerThread shutdown safety, dropped-task handles, and task leak

TaskDispatcher.hpp

• Make scheduleTask(...) return a no-op DeferredCallbackHandle when the dispatcher synchronously drops/deletes the task during Queue() (for example on a shutdown-drop path), instead of returning a handle that dangles.
• Track scheduled task lifetime internally so callers only retain a cancellation pointer if the task survived Queue().

WorkerThread.cpp

• Add an explicit shutdown gate (m_shuttingDown): late Queue() calls are rejected (and the task deleted) once teardown starts.
• Enqueue the shutdown sentinel only once under the queue lock.
• Delete pending queued/timer tasks on shutdown instead of only logging that they exist (previously they leaked). On a normal Join() the owning thread deletes them; on the self-dispose path the worker drains and deletes its own remaining tasks before exiting, so neither path leaks.
• Make self-dispose safe when the last dispatcher reference is released on the worker thread itself: capture the worker id under lock, detach, defer final delete to the thread after its loop exits, and keep self-dispose detection correct even after a prior detach.
• Log join/detach failures instead of silently swallowing all exceptions.

TaskDispatcherCAPITests.cpp

• Add coverage that scheduleTask(...) returns a no-op handle when a dispatcher drops the task synchronously during Queue().

PalTests.cpp

• Add regression coverage that scheduling after Join() returns a no-op handle, and that releasing the last worker-thread reference from a task on that same worker thread does not use-after-free.

Fix Flush teardown deadlock

OfflineStorageHandler.cpp

• Flush() could early-return (when StartActivity() fails during teardown) without posting flush completion, so WaitForFlush() blocked forever. Signal completion on the early-return path so teardown cannot deadlock.
• Flush() also paired StartActivity()/EndActivity() manually (StartActivity() at the top, EndActivity() on the last line) with no exception safety in between. StoreRecords(), the optional checkpoint Flush(), and IOfflineStorageObserver::OnStorageRecordsSaved() are all real throw surfaces (disk I/O, a full/locked DB, or an observer implementation); if any of them threw, EndActivity() was skipped and the pause-activity count was permanently leaked, deadlocking every later FlushAndTeardown()'s PauseActivity()+WaitPause(). This reproduced as a live macOS deadlock on main. Added ActivityGuard, an RAII wrapper matching the existing safe pattern already used by PauseGuard (TransmissionPolicyManager.cpp) and ActiveLoggerCall (Logger.cpp): its destructor calls EndActivity() on every exit path, including exception unwinding.

BasicFuncTests.cpp

• Add a smoke test that tears down while an upload is in flight (CFG_INT_MAX_TEARDOWN_TIME = 0, large payloads against the slow endpoint) and asserts shutdown completes cleanly; it also asserts the /slow/ endpoint rewrite actually happened so the coverage can’t silently lapse.

Fix static-destruction-order crashes in the two process-wide singletons

LogManagerFactory.hpp / PAL.cpp

• LogManagerFactory::instance() and PAL::GetPAL() were ordinary function-local statics. Their destruction order relative to LogManagerProvider::Release() and PAL::shutdown() (both invoked during process teardown) is unspecified — PAL in particular is constructed lazily on first use rather than at a fixed point, so whether it outlives the teardown call that needs it depends on runtime timing, not source order.
• A downstream consumer (onnxruntime-genai, see microsoft/onnxruntime-genai#2363) hit this in production as intermittent EXC_BAD_ACCESS crashes on macOS-arm64 at process exit — LogManagerFactory's registries and PAL's ISystemInformation member were sometimes already destroyed by the time teardown code tried to use them — and worked around it in their vendored copy of this SDK by leaking both singletons.
• Applied the same fix upstream: static T& x = *new T(); deliberately never destroys the object, so it stays valid for the rest of the process regardless of teardown timing. Both objects are small and process-lifetime singletons (one instance ever), and PAL::shutdown() / Release() already perform the real resource teardown explicitly, so this only removes the destructor-ordering hazard, not a resource leak in the ordinary sense.

Make TransmissionPolicyManager scheduling consistently mutex-guarded

TransmissionPolicyManager.cpp / .hpp

• Guard m_isUploadScheduled, m_runningLatency, and m_scheduledUploadTime consistently with m_scheduledUploadMutex.
• Avoid holding m_scheduledUploadMutex across potentially blocking cancellation during stop/shutdown.
• Keep force/zero-delay no-wait cancellation under the scheduler mutex so a competing delayed schedule cannot suppress an immediate upload, and propagate the requested latency to the already-running task when a no-wait cancel fails because that task is already executing.
• Use an explicit std::chrono::milliseconds value in the bandwidth-controller reschedule path, and cast std::chrono counts to long long in the %lld LOG_TRACE calls (the rep is long on LP64, a -Wformat mismatch in logging-enabled builds).

TransmissionPolicyManagerTests.cpp

• Add regression coverage for the force/zero-delay scheduling race, including the running-cancel/latency-propagation case.

Fix Logger static-destruction-order crash

Logger.cpp

• Remove destructor logging from Logger::~Logger() because it can run after logging infrastructure has already been destroyed, causing crashes during static teardown.

---

Known parity gap (separate repo, follow-up): AIHttpResponseDecoder::handleDecode in the lib/modules submodule (lib/modules/azmon/AIHttpResponseDecoder.cpp:105,118) still sets ctx->httpResponse = nullptr; on the aborted/failure paths — the same response-lifetime leak fixed here in lib/http/HttpResponseDecoder.cpp. It lives in a different repository (the lib/modules submodule), so it must be fixed there and pulled in via a submodule bump; it is out of scope for this PR.

Add batched offline persistence and recovery hardening

OfflineStorage_SQLite.cpp / OfflineStorageHandler.cpp

• Persist flush batches in one SQLite transaction, filter invalid records before persistence, verify commit success, and roll back both database writes and the in-memory size estimate on failure.
• Requeue drained records only when the batch is not durably stored, avoiding both event loss and duplicate requeueing.
• Keep flush completion signaling and record recovery correct on rejected, failed, and exceptional flush paths.

OfflineStorageTests_SQLite.cpp

• Exercise the batched persistence path in the release performance test and cover rollback/recovery behavior.

Harden Apple cancellation and callback lifetime handling

HttpClient_Apple.mm / HttpClientManager.cpp

• Handle NSURLSession cancellation callbacks that have no HTTP response without dereferencing a null response.
• Keep Apple request tracking consistent through asynchronous cancellation callbacks so teardown can drain callback state safely.

Follow-up review fix

WorkerThread.cpp

• Use the captured worker thread identity for self-cancellation checks, which remains valid after the std::thread object is detached.

[Copilot]

Pull request overview

This PR targets runtime correctness in the SDK by addressing thread-safety issues, shutdown safety, and a per-request memory leak in HTTP response handling.

Changes:

• Tighten HTTP request cancellation scoping and fix torn reads in request/callback tracking loops.
• Fix a SimpleHttpResponse leak on aborted/network-failure decode paths.
• Rework worker-thread shutdown behavior to avoid unsafe queue cleanup after detach() and improve error logging.
• Refactor TransmissionPolicyManager scheduling state synchronization (mutex + new cancelUploadTaskLocked() helper).

Reviewed changes

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

Show a summary per file

File	Description
lib/tpm/TransmissionPolicyManager.hpp	Changes upload-scheduling state fields and adds a locked cancellation helper declaration.
lib/tpm/TransmissionPolicyManager.cpp	Moves upload-scheduling state access under a mutex and adjusts cancellation/scheduling flow.
lib/pal/WorkerThread.cpp	Makes shutdown/join behavior safer and improves exception handling/logging during join/detach.
lib/http/HttpResponseDecoder.cpp	Deletes ctx->httpResponse on Abort/RetryNetwork paths to prevent leaks.
lib/http/HttpClient_Apple.mm	Limits cancellation to the instance’s task and fixes a torn read in the shutdown wait loop.
lib/http/HttpClientManager.cpp	Fixes a torn read in the shutdown wait loop by locking around empty-check.
lib/api/Logger.cpp	Removes destructor logging to avoid iOS static-destruction-order crash.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (1)

tests/unittests/OfflineStorageTests.cpp:13

• This test file now uses std::find and std::vector (NoopTaskDispatcher) but does not include or . Relying on transitive includes is brittle and can break builds when headers change; add the direct includes here.

[Copilot]

Pull request overview

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

Suppressed comments (2)

lib/offline/OfflineStorageHandler.cpp:380

• LOG_INFO uses printf-style varargs; for a "%p" format the argument must be a void*. Passing a MAT::Task* here is undefined behavior. Cast the pointer to void* so the format/type match.

                        LOG_INFO("Requested Flush (%p)", m_flushHandle.GetTask());

lib/offline/OfflineStorageHandler.cpp:121

• LOG_INFO uses printf-style varargs; for a "%p" format the argument must be a void*. Passing a MAT::Task* here is undefined behavior. Cast the pointer to void* so the format/type match consistently (similar to the WorkerThread logging fixes).

This issue also appears on line 380 of the same file.

        LOG_INFO("Waiting for pending Flush (%p) to complete...", m_flushHandle.GetTask());

[Copilot]

Pull request overview

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

Suppressed comments (1)

lib/offline/OfflineStorage_SQLite.cpp:371

• In the batched StoreRecords() path, insertRecordUnsafe() increments m_DbSizeEstimate before the transaction is committed. If an exception occurs during the loop, the transaction is marked for rollback and rethrown, but m_DbSizeEstimate is not reverted, leaving the size estimate permanently inflated (which can trigger incorrect storage-full notifications/resizes).

            catch (...)
            {
#ifdef ENABLE_LOCKING
                // DbTransaction commits on destruction by default for legacy
                // callers. An exception during a batch must explicitly roll