Skip to content

Fix 1DS telemetry deadlock during Windows process termination - #33291

Merged
Jiajia Qin (qjia7) merged 4 commits into
microsoft:mainfrom
qjia7:fix/1ds-windows-process-exit
Oct 17, 2026
Merged

Jiajia Qin (qjia7) merged 4 commits into
microsoft:mainfrom
qjia7:fix/1ds-windows-process-exit

Conversation

@qjia7

@qjia7 Jiajia Qin (qjia7) commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Description

Native inference can finish successfully, write its profiling output, and then hang indefinitely while Windows terminates the process. Skip 1DS uploader teardown in OneDsTelemetry::~OneDsTelemetry when Windows DLL shutdown is in progress, before acquiring telemetry locks.

The Windows helper resolves RtlDllShutdownInProgress from the already loaded ntdll.dll. This process-wide check also works for telemetry compiled into an EP plugin DLL. Normal object destruction and explicit FreeLibrary retain the existing cleanup path; non-Windows behavior is unchanged. Pending telemetry is not flushed during process termination.

Motivation and Context

Two captured native benchmark hangs had only the terminating thread remaining, with this shutdown stack:

ExitProcess -> LdrShutdownProcess -> WindowsEnv static destructor
  -> OneDsTelemetry::~OneDsTelemetry
  -> LogManagerImpl::FlushAndTeardown
  -> HttpClientManager::cancelAllRequests / cancelAllRequestsAsync
  -> HttpClient_WinHttp::CancelAllRequests -> condition-variable wait

Windows terminates the other threads before calling DLL process-detach destructors. The uploader can therefore wait forever for WinHTTP callbacks that can no longer run; a telemetry lock may also be held by a terminated thread. Checking shutdown state before taking locks avoids both waits. Resolved ORT/SDK frames were verified against identical machine code in a build with symbols.

Regression Coverage

Three committed Windows CTest cases load a fixture DLL linked to the production onnxruntime_common telemetry implementation. A test peer injects a mock log manager with network telemetry disabled; no model, GPU, or symbol-address patching is required.

  • onnxruntime_telemetry_process_exit_test: calls ExitProcess with a pending mock callback and verifies that the real DLL-owned telemetry destructor skips uploader cleanup.
  • onnxruntime_telemetry_normal_destruction_test: destroys the object before DLL unload and verifies one Flush, one FlushAndTeardown, and cleared shared state.
  • onnxruntime_telemetry_explicit_unload_test: verifies the same cleanup during FreeLibrary.

The parent requires both a successful child exit and an inherited event signaled only after teardown assertions pass. Each child has a 10-second timeout, with a 15-second CTest limit. The mock drain itself is bounded to two seconds so a regressed shutdown cannot strand a child inside DLL teardown.

Run from the CMake build directory after building onnxruntime_telemetry_shutdown_test:

ctest -C Release -R "^onnxruntime_telemetry_(process_exit|normal_destruction|explicit_unload)_test$" --output-on-failure

Validation

  • Windows x64/MSVC Release: configured a separate build and compiled the committed test target and its production dependencies; all three lifecycle tests passed.
  • Local mutation checks: removing the shutdown guard failed only the process-exit case; returning unconditionally from the destructor failed both normal destruction and explicit unload. Restoring the fix passed all three cases again.
  • Windows/MSVC runtime validation: recompiled the changed translation units and relinked isolated built-in WebGPU ORT, plugin ORT host, and WebGPU plugin binaries using compatible cached build objects.
  • Original local lifecycle harness with network telemetry suppressed and a mock pending uploader injected into the actual ORT DLL: baseline entered SDK draining and timed out; fixed runtime exited normally in 20/20 trials. Explicit DLL unload with an inactive uploader passed 3/3 trials for both baseline and fixed runtimes.
  • Native GenAI benchmark: built-in/plugin WebGPU × R32 FP16 LoRA on/off, 1,024 prompt tokens and 32 output tokens, batch 1, one warmup and two measured runs. All four configurations exited with code 0; generated text matched across providers for each adapter state. An additional built-in/LoRA-on run matched the original runtime's output and exited normally.
  • Changed-file lintrunner -a and git diff --check passed.

The committed lifecycle tests use a mock uploader; they do not cover real pending-upload unload stress. The benchmark executable, GenAI DLL, model graph, configuration, and adapter were unchanged during runtime validation.

Skip uploader teardown before acquiring telemetry locks when Windows is detaching DLLs for process termination. WinHTTP callbacks and worker threads are already gone at that point, so draining them can wait forever. Keep normal object destruction and explicit DLL unload cleanup.

Validated against captured native hang stacks and identical machine code at all resolved frames. A pending-uploader fault injection hangs the unchanged runtime and exits in 20 of 20 fixed trials. Both WebGPU modes completed 1024-prompt, 32-output-token native profiles with LoRA on and off; explicit unload also passed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The shutdown-specific deadlock fix lacks committed automated regression coverage.

1 open finding
What changed in this PR

Prevents Windows process termination from deadlocking during 1DS telemetry teardown.

Changes:

  • Detects Windows DLL shutdown via RtlDllShutdownInProgress.
  • Skips unsafe uploader teardown during process exit.
  • Preserves normal destruction and explicit DLL-unload cleanup.
File Description
onnxruntime/​core/​platform/​windows/​telemetry_1ds.cc Implements shutdown-state detection.
onnxruntime/​core/​platform/​telemetry_1ds.cc Bypasses teardown during Windows shutdown.
onnxruntime/​core/​platform/​telemetry_1ds_platform.h Declares the Windows helper.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread onnxruntime/core/platform/telemetry_1ds.cc

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The shutdown guard is narrowly scoped and covered across process exit, normal destruction, and explicit unload.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Comment thread cmake/onnxruntime_unittests.cmake Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants