Guard POSIX pipe cleanup when popen fails - #1523
Merged
bmehta001 merged 2 commits intoAug 28, 2026
Merged
Conversation
Shell-less environments can make popen return null, but shared_ptr still invokes its custom deleter for a null stored pointer. Guard the pclose call so telemetry initialization returns an empty fallback identifier instead of crashing. Files changed: - lib/pal/posix/sysinfo_sources.cpp: skip pclose for a null pipe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
Exec has sole ownership of the popen handle. unique_ptr expresses that lifetime directly and naturally skips pclose when popen returns null. Files changed:`n- lib/pal/posix/sysinfo_sources.cpp: own the pipe with a lambda-deleter unique_ptr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>`nCopilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
bmehta001
added a commit
to microsoft/onnxruntime
that referenced
this pull request
Aug 24, 2026
## Description Fixes #32173. ONNX Runtime 1.29.0 crashes during `OrtEnv` initialization in chiseled/distroless Linux images when POSIX telemetry is enabled. These images omit both `/etc/machine-id` and `/bin/sh`, so `cpp_client_telemetry` reaches its device-ID fallback and `popen()` fails. The SDK currently constructs `std::shared_ptr<FILE>(nullptr, pclose)`. A `shared_ptr` created with a custom deleter invokes that deleter even for a null pointer, so teardown calls `pclose(nullptr)` and segfaults in `_IO_new_fclose`. Own the `FILE*` with a lambda-deleter `unique_ptr`. `Exec()` has sole ownership of the pipe, and `unique_ptr` naturally skips its deleter when `popen()` returns null, allowing the empty-result fallback to work as intended when no shell is available. ## Validation - Reproduced the issue with the released `Microsoft.ML.OnnxRuntime` 1.29.0 package in the filesystem from `mcr.microsoft.com/dotnet/aspnet:10.0-noble-chiseled` under its default UID. - Telemetry enabled: deterministic SIGSEGV at `OrtEnv.Instance()`. - `ORT_DISABLE_TELEMETRY=1`: initialization succeeds. - Captured the failing native backtrace: `pclose(nullptr)` enters `_IO_new_fclose(fp=0x0)` immediately after the failed `popen()` fallback. - Built the telemetry-enabled Linux shared library with this unique-ownership implementation and warnings treated as errors. - Replaced the native library in the same managed repro and chiseled filesystem. - Telemetry enabled: initialization succeeds. - `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-id` is unavailable. ORT replaces the SDK-generated ID with its own persistent hashed device ID after `LogManager` initialization, 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
bmehta001
enabled auto-merge (squash)
August 28, 2026 21:41
Baiju Meswani (baijumeswani)
approved these changes
Aug 28, 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
2 participants
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
Own the POSIX
popenhandle withunique_ptrso a failed acquisition never callspclose(nullptr).std::shared_ptrinvokes a supplied deleter even when its stored pointer is null. On shell-less Linux images,popen()returns null; the existing deleter then passes that null pointer topclose(), which crashes during PAL/device-information initialization. The pipe has one owner, so a lambda-deleterunique_ptrmodels its lifetime directly and naturally skips cleanup for a null handle.This is the upstream fix for microsoft/onnxruntime#32173 and microsoft/onnxruntime#32226.
Validation
MATSDK_WARNINGS_AS_ERRORS=ON.UnitTestssuite.OrtEnv.Instance()as UID 1654 in the extractedmcr.microsoft.com/dotnet/aspnet:10.0-noble-chiseledfilesystem, which has neither/bin/shnor/etc/machine-id; initialization succeeds with telemetry enabled and disabled.