Skip to content

Preserve nonzero exit status through fast-shutdown close#717

Closed
hujc7 wants to merge 1 commit into
isaac-sim:mainfrom
hujc7:jichuanh/preserve-close-exit-status
Closed

Preserve nonzero exit status through fast-shutdown close#717
hujc7 wants to merge 1 commit into
isaac-sim:mainfrom
hujc7:jichuanh/preserve-close-exit-status

Conversation

@hujc7

@hujc7 hujc7 commented Jul 21, 2026

Copy link
Copy Markdown

Description

SimulationApp.close(exit_code) currently behaves differently depending on the exit code: a nonzero code exits immediately via os._exit before any cleanup runs, while the graceful path always terminates the process with status 0 from inside app.shutdown() under fast shutdown. Callers therefore have to choose between cleanup and a truthful exit status. In practice, a worker that closes gracefully after a failure or a termination signal is reported as exit 0, which distributed launchers (torchrun) and CI record as a successful run — the surviving ranks then block until the NCCL watchdog fires, and the root cause is misattributed. (Downstream context: isaac-sim/IsaacLab#6636 currently works around this by disabling fast shutdown at signal time.)

This change makes exit_code orthogonal to teardown:

  • every close() path performs its normal cleanup regardless of the exit code;
  • a nonzero status is applied with os._exit(exit_code) exactly at the point where fast shutdown would otherwise terminate the process with 0 (before _arm_shutdown_watchdog(), since that path never reaches app.shutdown());
  • _flush_stdio() additionally flushes the C stdio streams (fflush(NULL) via ctypes): native output such as IApp.print_and_log is buffered on the C/C++ side, which os._exit abandons, so the tail of the shutdown log — including the Simulation App Shutting Down marker — was previously lost on every _exit path.

Verification

On a Linux source build (6.0.0 GA base, change rebased onto main with the shutdown-watchdog resolution described above), with a real headless app:

  • SimulationApp({"headless": True}).close(exit_code=5) → process exits 5, full cleanup runs, and Simulation App Shutting Down is present in the output (previously: either exit 5 with no cleanup, or cleanup with exit 0).
  • close() with the default exit code → unchanged: exits 0 with full cleanup.
  • Isaac Lab's AppLauncher exit-status integration tests (SIGTERM status, unhandled-exception status) pass unchanged against the patched build.

SimulationApp.close(exit_code) previously behaved differently by exit
code: a nonzero code exited immediately before any cleanup, while the
graceful path always terminated with status 0 from inside
app.shutdown() under fast shutdown. Callers therefore had to choose
between cleanup and a truthful exit status -- a SIGTERM-ed or failed
process that closed gracefully was reported as successful, which
distributed launchers and CI record as a passing run.

Make exit_code orthogonal to teardown: every close() path performs its
normal cleanup, and a nonzero status is applied exactly where fast
shutdown would otherwise terminate the process with 0.

Also flush the C stdio streams in _flush_stdio(): native output such as
IApp.print_and_log goes through the C/C++ buffers, which os._exit
abandons, so the tail of the shutdown log (including the "Simulation
App Shutting Down" marker) was lost on every _exit path.

Validated with a real headless app: close(exit_code=5) exits 5 with the
full cleanup log present; close() still exits 0; IsaacLab's AppLauncher
exit-status integration tests pass unchanged against this build.
hujc7 added a commit to hujc7/IsaacLab that referenced this pull request Jul 21, 2026
The two WORKAROUND(isaac-sim) blocks were wrapped in blanket exception
suppression, so an upstream SimulationApp change could silently disable
them and quietly reintroduce the exit-status masking. Print a clear
warning when the fast-shutdown override does not take, and fall back to
a plain close() with a warning if close() stops accepting exit_code.

Reference the upstream fix (isaac-sim/IsaacSim#717) in the workaround
note; once it lands, both blocks can be deleted.
@hujc7 hujc7 closed this Jul 21, 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