[Telemetry] Prevent crashes in shell-less containers - #32226
Merged
Merged
Conversation
cpp_client_telemetry constructs a shared_ptr with pclose as its deleter even when popen fails. Chiseled images have no /bin/sh, so telemetry initialization calls pclose(nullptr) and terminates the process. Use unique_ptr ownership so a null pipe is not passed to pclose and telemetry can degrade gracefully. Files changed: - cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch: make the POSIX pipe owner null-safe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents telemetry initialization crashes in shell-less Linux containers by making failed popen() ownership null-safe.
Changes:
- Replaces
shared_ptr<FILE>withunique_ptr<FILE>and apclosedeleter. - Preserves the existing empty-result fallback when
popen()fails.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keep the existing shared ownership shape while making its custom deleter null-safe. This matches cpp_client_telemetry#1523 and avoids the function-pointer warning that occurred under the Linux warnings-as-errors build. Files changed: - cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch: guard pclose in the shared_ptr deleter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
The pipe returned by popen has one owner. Matching the upstream unique_ptr fix makes that lifetime explicit and naturally avoids pclose for a null handle. Files changed:`n- cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch: use the upstream lambda-deleter unique_ptr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>`nCopilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
Edward Chen (edgchen1)
approved these changes
Aug 24, 2026
bmehta001
added a commit
to microsoft/onnxruntime-genai
that referenced
this pull request
Sep 17, 2026
…2563) ## Summary Ports the relevant telemetry changes from microsoft/onnxruntime#29872, microsoft/onnxruntime#32226, and microsoft/onnxruntime#32425 into GenAI: - upgrades `cpp_client_telemetry` to 3.10.240.1, which includes the null-safe POSIX `popen()` cleanup from microsoft/cpp_client_telemetry#1523 - moves routine model, generation, adapter, and runtime-error events to deterministic 1% client-side sampling while keeping `ProcessInfo` unsampled - disables SDK tracing at runtime and compiles internal 1DS logging out of FetchContent builds - adds bounded container, VM, WSL, cloud-guest, and Android-emulator classification to `ProcessInfo` without transmitting raw host evidence - uses the new SDK's canonical CMake options while retaining GenAI's separate process-exit teardown fixes GenAI already contained the relevant microsoft/onnxruntime#29872 behavior for default-on supported builds, explicit opt-out, minimized common context, network-context cleanup, and persistent generated device IDs. ORT-only work from microsoft/onnxruntime#32425—session/EP lifecycle consolidation, RuntimePerf/SystemMetrics changes, and EP device census—does not map to GenAI's event model and is intentionally excluded. ## Validation - Repository `lintrunner`: clean - Linux/WSL Release telemetry build: - `onnxruntime-genai` - `telemetry_helpers_tests` - `telemetry_device_info_tests` - Linux/WSL focused CTest: 2/2 passed - Visual Studio 2026 telemetry configure: passed - Visual Studio 2026 compile could not run locally because the installed toolchain lacks Spectre-mitigated libraries (`MSB8040`), including for GoogleTest and vendored dependencies - Verified the pinned 3.10.240.1 archive SHA-1 and applied the compatibility patch against a clean extracted archive --------- Copilot-Session: b72147d1-3937-4b13-940c-33299589af1b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #32173.
ONNX Runtime 1.29.0 crashes during
OrtEnvinitialization in chiseled/distroless Linux images when POSIX telemetry is enabled. These images omit both/etc/machine-idand/bin/sh, socpp_client_telemetryreaches its device-ID fallback andpopen()fails.The SDK currently constructs
std::shared_ptr<FILE>(nullptr, pclose). Ashared_ptrcreated with a custom deleter invokes that deleter even for a null pointer, so teardown callspclose(nullptr)and segfaults in_IO_new_fclose.Own the
FILE*with a lambda-deleterunique_ptr.Exec()has sole ownership of the pipe, andunique_ptrnaturally skips its deleter whenpopen()returns null, allowing the empty-result fallback to work as intended when no shell is available.Validation
Microsoft.ML.OnnxRuntime1.29.0 package in the filesystem frommcr.microsoft.com/dotnet/aspnet:10.0-noble-chiseledunder its default UID.OrtEnv.Instance().ORT_DISABLE_TELEMETRY=1: initialization succeeds.pclose(nullptr)enters_IO_new_fclose(fp=0x0)immediately after the failedpopen()fallback.ORT_DISABLE_TELEMETRY=1: initialization succeeds.Upstream
The SDK fix is proposed in microsoft/cpp_client_telemetry#1523. ORT should retain this compatibility patch until it pins an SDK release containing that change.
Exec()is used only by the non-Apple legacy device-ID fallback when/etc/machine-idis unavailable. ORT replaces the SDK-generated ID with its own persistent hashed device ID afterLogManagerinitialization, but the SDK probe currently runs before that override. A future SDK option to suppress automatic device-ID discovery would let ORT avoid this unnecessary shell probe entirely; the null-safe cleanup is still required for existing SDK consumers and versions.