Fix AppLauncher exception exit status by nblauch · Pull Request #6634 · isaac-sim/IsaacLab

nblauch · 2026-07-20T19:05:58Z

Description

AppLauncher registers an atexit callback that closes SimulationApp. With
Kit fast shutdown enabled, calling close() with its default zero exit code can
replace Python's pending nonzero status after an unhandled exception.

This change detects the unhandled exception state exposed by the interpreter
and passes exit code 1 to SimulationApp.close(). Normal interpreter shutdown
continues to pass exit code 0.

The new integration test launches a real headless CPU AppLauncher in a child
process, raises an unhandled RuntimeError, and verifies both the traceback and
exit status 1.

Fixes #6573

Type of change

Bug fix (non-breaking change which fixes an issue)

Testing

Before the fix, the regression test failed because the child returned 0.
After the fix, running the trigger script directly reports exit status 1.
python -m pytest source/isaaclab/test/app/test_app_launcher_exit_code.py source/isaaclab/test/app/test_kwarg_launch.py::test_launch_simulation_preserves_failure_exit_code -q (2 passed)
./isaaclab.sh -f (all hooks passed)

Screenshots

Not applicable.

Checklist

I have read and understood the contribution guidelines.
I have run the pre-commit checks with ./isaaclab.sh --format.
Documentation is not required for this internal shutdown correction.
My changes generate no new warnings.
I have added tests that prove my fix is effective.
I have added a changelog fragment under source/isaaclab/changelog.d/.
My name is already present in CONTRIBUTORS.md.

Pass a nonzero code to SimulationApp.close() during interpreter shutdown when Python reports an unhandled exception. Add an integration test that reproduces the zero-status regression from isaac-sim#6573.

greptile-apps · 2026-07-20T19:10:03Z

Greptile Summary

This PR fixes AppLauncher's atexit callback masking unhandled-exception exit status. When Kit's fast shutdown is enabled, SimulationApp.close() with the default exit_code=0 replaces Python's pending nonzero status; the fix detects this state via sys.last_type and passes exit_code=1 instead.

app_launcher.py: The _atexit_close callback now reads sys.last_type (set by CPython before atexit runs, only for non-SystemExit unhandled exceptions) to decide whether to pass exit_code=1 to app.close().
test_app_launcher_exit_code.py: New integration test that forks a child process, raises an unhandled RuntimeError, and asserts both the traceback and exit code 1 are preserved.

Confidence Score: 5/5

Safe to merge — the change is a small, well-reasoned atexit callback adjustment with targeted tests covering both the subprocess integration path and the unit-level close() argument.

The fix correctly exploits the CPython-guaranteed ordering (sys.last_type is populated by PyErr_PrintEx before atexit runs), and the getattr fallback keeps it safe across Python versions. The one known gap — sys.exit(N) with N != 0 still gets overridden by Kit because SystemExit never sets sys.last_type — is a pre-existing limitation outside this PR's scope. Tests cover the targeted regression cleanly.

No files require special attention.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/app/app_launcher.py	Targeted 5-line change in _atexit_close: reads sys.last_type to detect unhandled exceptions and passes exit_code=1 to app.close(); logic is correct and well-commented.
source/isaaclab/test/app/test_app_launcher_exit_code.py	New integration test that forks a child process with a trigger flag, raises an intentional unhandled RuntimeError, and verifies returncode==1 and traceback in stderr; well-structured self-contained script pattern.
source/isaaclab/changelog.d/nblauch-fix-applauncher-exit-code.rst	Changelog fragment correctly describes the bug fix with an appropriate RST Fixed entry.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Script as Python Script
    participant Interp as CPython Interpreter
    participant Atexit as atexit._atexit_close
    participant Kit as SimulationApp.close()

    Script->>Interp: Unhandled exception propagates
    Interp->>Interp: "PyErr_PrintEx(set_sys_last_vars=1)"
    Note over Interp: Sets sys.last_type = ExcType
    Interp->>Interp: Print traceback via sys.excepthook
    Interp->>Atexit: Run atexit callbacks
    Atexit->>Atexit: getattr(sys, last_type, None) → ExcType (not None)
    Atexit->>Kit: "app.close(exit_code=1)"
    Kit->>Interp: os._exit(1)
    Note over Interp: Process exits with code 1

    Script->>Interp: Normal return / sys.exit(0)
    Interp->>Atexit: Run atexit callbacks
    Atexit->>Atexit: getattr(sys, last_type, None) → None
    Atexit->>Kit: "app.close(exit_code=0)"
    Kit->>Interp: os._exit(0)
    Note over Interp: Process exits with code 0

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Script as Python Script
    participant Interp as CPython Interpreter
    participant Atexit as atexit._atexit_close
    participant Kit as SimulationApp.close()

    Script->>Interp: Unhandled exception propagates
    Interp->>Interp: "PyErr_PrintEx(set_sys_last_vars=1)"
    Note over Interp: Sets sys.last_type = ExcType
    Interp->>Interp: Print traceback via sys.excepthook
    Interp->>Atexit: Run atexit callbacks
    Atexit->>Atexit: getattr(sys, last_type, None) → ExcType (not None)
    Atexit->>Kit: "app.close(exit_code=1)"
    Kit->>Interp: os._exit(1)
    Note over Interp: Process exits with code 1

    Script->>Interp: Normal return / sys.exit(0)
    Interp->>Atexit: Run atexit callbacks
    Atexit->>Atexit: getattr(sys, last_type, None) → None
    Atexit->>Kit: "app.close(exit_code=0)"
    Kit->>Interp: os._exit(0)
    Note over Interp: Process exits with code 0

_{Reviews (1): Last reviewed commit: "Preserve AppLauncher failure exit status" | Re-trigger Greptile}

IsaacLab requires Python 3.12, where sys.last_exc is the recommended way to inspect the unhandled exception recorded by the interpreter.

Extend the re-entrancy fix into a complete teardown-correctness change based on a combined review with the atexit exit-code fix (isaac-sim#6634): - Report killed-by-signal status: the handler previously ran a graceful close whose Kit fast-shutdown path terminated the process with exit code 0, so a SIGTERM-ed worker was recorded as successful and distributed launchers misattributed the failure. The handler now disables fast shutdown so close() performs the full teardown and returns, then re-raises the signal with the default action so the process exits with the conventional 128+signum status. The override is marked WORKAROUND(isaac-sim) for removal once SimulationApp can propagate a nonzero exit status through its fast-shutdown path. - Arm the re-entrancy guard in the atexit close (extracted to the testable _close_app_at_exit method) so a signal arriving during a normal shutdown takes the guarded path instead of starting a nested teardown of a half-closed app. - Stop intercepting SIGSEGV: a Python handler never runs for a synchronous main-thread segfault (the process spins on signal delivery), reports success for worker-thread segfaults, and replaces the carb crash reporter's handler, suppressing minidumps. - Restore Python's default SIGINT handler over SimulationApp's, which exits 0 before user finally blocks or KeyboardInterrupt handlers can run. Marked WORKAROUND(isaac-sim) for removal once the upstream handler preserves exception semantics and a nonzero exit status. - Remove the unregistered, dead _interrupt_signal_handle_callback. Tests cover the single-close-plus-reraise contract, the re-entrant signal path, and the atexit guard arming; all three fail against the previous behavior.

Gather the startup announcements (CI marker, Kit version diagnostics) and the entire exit-path policy (atexit close, signal handlers, exit codes) into a nested AppLauncher._SimulationAppLifecycle class. The pieces coordinate through one guard flag and exist for one reason -- report the process state truthfully to whatever supervises it -- so a single class with the policy table as its docstring replaces logic previously spread across __init__ and three private methods. Absorb the atexit exit-code fix from PR isaac-sim#6634 (nblauch) into the lifecycle class: the atexit close passes a nonzero exit code when an unhandled exception is pending, so Kit fast shutdown does not replace the failure status with 0. Includes that PR's integration test and a kitless unit test for the exit-code selection. SystemExit is documented as not yet detected. Behavior is otherwise unchanged; all kitless unit tests and both real-Kit integration tests (SIGTERM status, exception exit code) pass.

AntoineRichard

Should we enable this for debug only?

AntoineRichard · 2026-07-21T12:57:31Z

Do we need this test?

thanks for review @AntoineRichard

re: needing for debug only - why? shouldn't users be correctly informed when their code fails by the exit status of the script?

re: the test, i defer judgment on this to code owners

kellyguo11 · 2026-07-23T01:16:24Z

superseded by #6636

@nblauch

…U NCCL workaround (#6636) # Summary - Consolidates `AppLauncher` process-lifecycle handling into one nested `_SimulationAppLifecycle` class: startup announcements (CI marker, Kit version diagnostics) plus the entire exit-path policy, with the policy table as the class docstring. - Fixes every exit path that misreported failure as success or destroyed its own diagnostics (full failure-case table below). Absorbs the atexit exit-code fix from #6634. - Documents the NCCL cuMem workaround for multi-GPU RTX training on NUMA-spanning GPU allocations (`NCCL_CUMEM_HOST_ENABLE=0` first, `NCCL_CUMEM_ENABLE=0` as fallback). - The upstream `SimulationApp` exit-status fix (public reference: isaac-sim/IsaacSim#717) has been **merged** (`isaacsim.simulation_app` >= 2.18.5): the signal handler now simply passes `close(exit_code=128 + signum)` — full teardown + truthful status on fixed builds, truthful status on older builds. The one remaining `WORKAROUND(isaac-sim)` is the SIGINT re-registration (upstream handler still exits 0 before user code unwinds); it fails loudly if drift disables it. - Fixes #6573 (absorbed atexit exit-code fix, originally #6634 by @nblauch). - Fixes #6530: the SIGTERM handler no longer returns to the interrupted execution path — the worker exits through `close(exit_code=128 + signum)` (full Kit teardown on `isaacsim.simulation_app` >= 2.18.5) or dies by the re-raised signal, so distributed workers terminate with a truthful status instead of surviving and spamming TCPStore `Broken pipe` errors. # Failure cases and how this PR addresses them Root mechanism: Kit fast shutdown terminated the process with exit code 0 from inside `SimulationApp.close()`, so any death funneled through an unqualified `close()` was reported as success; additionally, the abort-signal handler was unguarded against re-entrancy. | How the process ends | Before this PR | After this PR | |---|---|---| | Unhandled Python exception | atexit close overwrote the pending failure with **exit 0** (CI false-green) | exits 1 (`sys.last_exc` detected; absorbed from #6634; `SystemExit` documented as not yet covered) | | Single SIGTERM (torchrun teardown, SLURM preemption, `kill`) | graceful close → **exit 0**; launcher marks the killed rank SUCCEEDED; surviving ranks hang until the NCCL watchdog | `close(exit_code=128 + signum)`: full teardown + truthful status on `isaacsim.simulation_app` >= 2.18.5; truthful status on older builds; dies by the signal if `close()` returns | | Second signal while a close is running (repeated SIGTERM; fault inside the replicator stop/wait) | handler re-entered `close()` → **infinite recursion** → SIGKILL-only shutdown, spurious SIGSEGV, logs flooded (~975 recursion frames/job observed on OSMO pods) | guard: re-entrant signal falls back to `SIG_DFL` | | Signal racing the normal atexit close | nested full second teardown of a half-closed app | atexit arms the same guard | | `kill -ABRT` | graceful close → **exit 0** | same truthful-close path as SIGTERM | | Real SIGSEGV, main thread | Python handler can never run → process **spins forever** at 100% CPU, crash reporter clobbered (no minidump) | SIGSEGV no longer intercepted → default action, minidumps restored | | Real SIGSEGV, worker thread | handler ran on the main thread → **exit 0** for a crashed process | same: default action, truthful signal death | | Ctrl-C | SimulationApp's handler exits **0** before user `finally`/`KeyboardInterrupt` code runs | Python default handler restored: `KeyboardInterrupt` unwinds user code, nonzero exit | Not addressed here (tracked elsewhere): `sys.exit(N)`/`SystemExit` still exits 0 (gap inside the #6634 mechanism, documented at the detection site). # Implementation notes 1. All exit-path logic lives in `AppLauncher._SimulationAppLifecycle`; the class docstring is the policy table, and each decision carries its rationale in place. 2. The signal handler passes the killed-by-signal status through `close(exit_code=128 + signum)`. With the merged upstream fix (`isaacsim.simulation_app` >= 2.18.5) the app performs its full teardown and exits with that status; on older builds the status is preserved without the teardown; if `close()` returns (fast shutdown disabled), the handler re-raises with the default action. A `TypeError` fallback warns loudly if a future `SimulationApp` drops the parameter. 3. Docs: distributed camera training fails deterministically when the allocated GPUs span NUMA nodes and passes on a single-switch set; disabling NCCL cuMem host allocations rescues the failing shape in paired same-node experiments. Added to the multi-GPU NCCL troubleshooting section. **Testing.** Six kitless unit tests (`test_simulation_app_lifecycle.py`: killed-by-signal status, re-entrancy both directions, exit-code selection, drift fallbacks on both paths) plus two real-Kit integration tests (`test_app_launcher_exit_status.py`: SIGTERM → truthful termination status with no handler recursion; unhandled exception → exits 1). The integration tests pass against both the pre-fix and the fixed (>= 2.18.5) `SimulationApp` builds; on develop's original behavior the SIGTERM test observes exit code 0 (the bug). ## Type of change - Bug fix (non-breaking change which fixes an issue) - Documentation update ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there

@nblauch

…U NCCL workaround (isaac-sim#6636) # Summary - Consolidates `AppLauncher` process-lifecycle handling into one nested `_SimulationAppLifecycle` class: startup announcements (CI marker, Kit version diagnostics) plus the entire exit-path policy, with the policy table as the class docstring. - Fixes every exit path that misreported failure as success or destroyed its own diagnostics (full failure-case table below). Absorbs the atexit exit-code fix from isaac-sim#6634. - Documents the NCCL cuMem workaround for multi-GPU RTX training on NUMA-spanning GPU allocations (`NCCL_CUMEM_HOST_ENABLE=0` first, `NCCL_CUMEM_ENABLE=0` as fallback). - The upstream `SimulationApp` exit-status fix (public reference: isaac-sim/IsaacSim#717) has been **merged** (`isaacsim.simulation_app` >= 2.18.5): the signal handler now simply passes `close(exit_code=128 + signum)` — full teardown + truthful status on fixed builds, truthful status on older builds. The one remaining `WORKAROUND(isaac-sim)` is the SIGINT re-registration (upstream handler still exits 0 before user code unwinds); it fails loudly if drift disables it. - Fixes isaac-sim#6573 (absorbed atexit exit-code fix, originally isaac-sim#6634 by @nblauch). - Fixes isaac-sim#6530: the SIGTERM handler no longer returns to the interrupted execution path — the worker exits through `close(exit_code=128 + signum)` (full Kit teardown on `isaacsim.simulation_app` >= 2.18.5) or dies by the re-raised signal, so distributed workers terminate with a truthful status instead of surviving and spamming TCPStore `Broken pipe` errors. # Failure cases and how this PR addresses them Root mechanism: Kit fast shutdown terminated the process with exit code 0 from inside `SimulationApp.close()`, so any death funneled through an unqualified `close()` was reported as success; additionally, the abort-signal handler was unguarded against re-entrancy. | How the process ends | Before this PR | After this PR | |---|---|---| | Unhandled Python exception | atexit close overwrote the pending failure with **exit 0** (CI false-green) | exits 1 (`sys.last_exc` detected; absorbed from isaac-sim#6634; `SystemExit` documented as not yet covered) | | Single SIGTERM (torchrun teardown, SLURM preemption, `kill`) | graceful close → **exit 0**; launcher marks the killed rank SUCCEEDED; surviving ranks hang until the NCCL watchdog | `close(exit_code=128 + signum)`: full teardown + truthful status on `isaacsim.simulation_app` >= 2.18.5; truthful status on older builds; dies by the signal if `close()` returns | | Second signal while a close is running (repeated SIGTERM; fault inside the replicator stop/wait) | handler re-entered `close()` → **infinite recursion** → SIGKILL-only shutdown, spurious SIGSEGV, logs flooded (~975 recursion frames/job observed on OSMO pods) | guard: re-entrant signal falls back to `SIG_DFL` | | Signal racing the normal atexit close | nested full second teardown of a half-closed app | atexit arms the same guard | | `kill -ABRT` | graceful close → **exit 0** | same truthful-close path as SIGTERM | | Real SIGSEGV, main thread | Python handler can never run → process **spins forever** at 100% CPU, crash reporter clobbered (no minidump) | SIGSEGV no longer intercepted → default action, minidumps restored | | Real SIGSEGV, worker thread | handler ran on the main thread → **exit 0** for a crashed process | same: default action, truthful signal death | | Ctrl-C | SimulationApp's handler exits **0** before user `finally`/`KeyboardInterrupt` code runs | Python default handler restored: `KeyboardInterrupt` unwinds user code, nonzero exit | Not addressed here (tracked elsewhere): `sys.exit(N)`/`SystemExit` still exits 0 (gap inside the isaac-sim#6634 mechanism, documented at the detection site). # Implementation notes 1. All exit-path logic lives in `AppLauncher._SimulationAppLifecycle`; the class docstring is the policy table, and each decision carries its rationale in place. 2. The signal handler passes the killed-by-signal status through `close(exit_code=128 + signum)`. With the merged upstream fix (`isaacsim.simulation_app` >= 2.18.5) the app performs its full teardown and exits with that status; on older builds the status is preserved without the teardown; if `close()` returns (fast shutdown disabled), the handler re-raises with the default action. A `TypeError` fallback warns loudly if a future `SimulationApp` drops the parameter. 3. Docs: distributed camera training fails deterministically when the allocated GPUs span NUMA nodes and passes on a single-switch set; disabling NCCL cuMem host allocations rescues the failing shape in paired same-node experiments. Added to the multi-GPU NCCL troubleshooting section. **Testing.** Six kitless unit tests (`test_simulation_app_lifecycle.py`: killed-by-signal status, re-entrancy both directions, exit-code selection, drift fallbacks on both paths) plus two real-Kit integration tests (`test_app_launcher_exit_status.py`: SIGTERM → truthful termination status with no handler recursion; unhandled exception → exits 1). The integration tests pass against both the pre-fix and the fixed (>= 2.18.5) `SimulationApp` builds; on develop's original behavior the SIGTERM test observes exit code 0 (the bug). ## Type of change - Bug fix (non-breaking change which fixes an issue) - Documentation update ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there

Preserve AppLauncher failure exit status

deebb23

Pass a nonzero code to SimulationApp.close() during interpreter shutdown when Python reports an unhandled exception. Add an integration test that reproduces the zero-status regression from isaac-sim#6573.

nblauch requested a review from a team July 20, 2026 19:05

nblauch requested review from AntoineRichard, kellyguo11, ooctipus, pbarejko and pv-nvidia as code owners July 20, 2026 19:05

github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Jul 20, 2026

Use last_exc for shutdown detection

7e0c5ec

IsaacLab requires Python 3.12, where sys.last_exc is the recommended way to inspect the unhandled exception recorded by the interpreter.

hujc7 mentioned this pull request Jul 20, 2026

Make AppLauncher teardown truthful and re-entrancy-safe; document mGPU NCCL workaround #6636

Merged

7 tasks

AntoineRichard reviewed Jul 21, 2026

View reviewed changes

kellyguo11 closed this Jul 23, 2026

Navigation Menu

Uh oh!

Fix AppLauncher exception exit status - #6634

Fix AppLauncher exception exit status#6634
nblauch wants to merge 2 commits into
isaac-sim:developfrom
nblauch:nblauch/fix-applauncher-exit-code

nblauch commented Jul 20, 2026

Uh oh!

greptile-apps Bot commented Jul 20, 2026

Uh oh!

AntoineRichard left a comment

Uh oh!

AntoineRichard Jul 21, 2026

Uh oh!

nblauch Jul 22, 2026

Uh oh!

kellyguo11 commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nblauch commented Jul 20, 2026

Description

Type of change

Testing

Screenshots

Checklist

Uh oh!

greptile-apps Bot commented Jul 20, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

AntoineRichard left a comment

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

nblauch Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

kellyguo11 commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants