Skip to content

fix(sandbox): harden managed-children registry and workload launcher - #5209

Open
EmilienM wants to merge 1 commit into
NVIDIA:mainfrom
EmilienM:fix/5186-managed-children-launcher/EmilienM
Open

EmilienM wants to merge 1 commit into
NVIDIA:mainfrom
EmilienM:fix/5186-managed-children-launcher/EmilienM

Conversation

@EmilienM

@EmilienM EmilienM commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Three silent-failure paths in the sandbox, all found by reading the code: a managed-children registration leaks after a poisoned registry lock, the orphan reaper goes inert on kernels without CONFIG_PROC_CHILDREN because every missing children file reads as a vanished task, and a panicking job kills the workload launcher thread while is_alive() keeps reporting healthy.

Related Issue

Closes #5186

Changes

  • managed_children::unregister recovers from a poisoned registry lock like its sibling accessors, so a registration can no longer leak after a waiter panics while holding the lock
  • direct_child_pids classifies a missing children file per task (vanished task vs missing support), only concludes missing support when no task in the scan listed one, and reports it once per process with a warning and an OCSF detection finding
  • The workload launcher runs each job under catch_unwind, like the network broker's handler panics: the panic is logged, the thread leaves its job loop, and is_alive() turns false so later spawns fail fast

Release Notes

When no task under /proc/self/task exposes a children file (kernel built without CONFIG_PROC_CHILDREN, or restricted procfs), the sandbox now reports the inert orphan reaper once instead of failing silently. A panicking workload-launcher job now stops the launcher with a clear error instead of leaving it reporting healthy.

Testing

  • Checks appropriate to the affected code and behavior pass

  • Unit tests added/updated (if applicable)

  • E2E tests added/updated (if applicable)

  • cargo test -p openshell-sandbox --lib: 293 passed, including regression tests for each path (poisoned-lock unregister, children-file classification and reporting, panicking launcher job; the suite also passes with RUST_BACKTRACE=1)

  • cargo clippy --workspace --all-targets -- -D warnings, the perf-harness variant, and cargo fmt --all -- --check pass

  • No local e2e run: this host has no docker daemon and the k8s lane needs a full image build. The changed paths are failure paths not reachable on healthy kernels, and the launcher's normal spawn path is covered by tests that spawn real children through the filtered thread. CI lanes cover the rest.

  • uv-based checks (python format, license headers) skipped locally due to a host uv config break (2026-11-03 RPM update); this diff touches no manifests or Python files

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

Recover the managed-children registry from a poisoned lock in unregister,
like its sibling accessors already do, so a PID whose waiter panicked
while holding the lock cannot stay registered forever and be skipped by
the orphan reaper after the PID is reused.

Classify a missing /proc/self/task/<tid>/children file instead of
treating every NotFound as a vanished task. When no task in a scan
exposes the file (kernel built without CONFIG_PROC_CHILDREN, or
restricted procfs), log a warning and emit an OCSF detection finding
once per process instead of silently leaving the orphan reaper inert
while zombies accumulate.

Contain a panicking job on the workload launcher thread, the way the
network broker contains handler panics: the panic is reported, the
thread leaves its job loop, and the health flag turns false so every
later spawn fails fast instead of hanging on a launcher that still
reports healthy.

Closes NVIDIA#5186

Signed-off-by: Emilien Macchi <emacchi@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

This branch has not been deployed

No deployments
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.

bug: managed_children and the workload launcher mishandle a poisoned lock, a missing CONFIG_PROC_CHILDREN and a panicking job

1 participant