Skip to content

Guard POSIX pipe cleanup when popen fails - #1523

Open
bmehta001 wants to merge 2 commits into
microsoft:mainfrom
bmehta001:bhamehta/null-safe-resource-cleanup
Open

Guard POSIX pipe cleanup when popen fails#1523
bmehta001 wants to merge 2 commits into
microsoft:mainfrom
bmehta001:bhamehta/null-safe-resource-cleanup

Conversation

@bmehta001

@bmehta001 bmehta001 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Description

Own the POSIX popen handle with unique_ptr so a failed acquisition never calls pclose(nullptr).

std::shared_ptr invokes 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 to pclose(), which crashes during PAL/device-information initialization. The pipe has one owner, so a lambda-deleter unique_ptr models 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

  • Built the Linux SDK and unit tests with MATSDK_WARNINGS_AS_ERRORS=ON.
  • Ran the full Linux UnitTests suite.
  • Rebuilt telemetry-enabled ONNX Runtime with this exact unique-ownership implementation.
  • Ran OrtEnv.Instance() as UID 1654 in the extracted mcr.microsoft.com/dotnet/aspnet:10.0-noble-chiseled filesystem, which has neither /bin/sh nor /etc/machine-id; initialization succeeds with telemetry enabled and disabled.

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 bmehta001 self-assigned this Aug 26, 2026
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.

1 participant