[Core] Stop simulations before shutting down their runtimes

[StafaH]

Description

Standalone scripts can leave a live SimulationContext when their launch_simulation() scope exits. Closing Kit first can terminate the process before physics, rendering, visualizer, and stage resources are released.

Release contexts created inside the launch scope before closing its runtimes. SimulationContext.clear_instance() now stops the simulation before releasing resources, replacing the separate stop in build_simulation_context(). Cleanup runs in finally before runtime shutdown. The existing exception handlers report body or cleanup errors and assign exit statuses; no extra catch or suppression is added. If both the body and cleanup fail, Python chains the original error behind the cleanup error. An existing caller-owned context is preserved.

Motivated by the ARL demo's Destroying busy TaskGroup! shutdown abort in #8171: https://github.com/isaac-sim/IsaacLab/actions/runs/36968778710/job/110721645994. No sleeps, retries, or assertion suppression are added. No test files are changed; the added test scaffolding was removed in favor of existing coverage and standalone validation.

The cleanup-order defect was reproduced with temporary regression checks, but the intermittent native abort has not been reproduced locally. Its native task-group owner remains unidentified. This is a draft pending validation against the failing CI image.

Validation

• Existing launcher tests: 11 passed.
• Existing Kit simulation-context tests: 26 passed, with two existing material deprecation warnings.
• Simplified implementation: headless ARL PhysX demo completed 10 steps and released its context before exit. The initial implementation also completed five consecutive runs.
• Real Kit exit checks after simplification: ValueError, KeyboardInterrupt, and SystemExit(7) released their contexts and exited with 1, 130, and 7 respectively. A registered resource whose cleanup intentionally fails also cleared the context and exited with 1.
• Formatting/lint hooks, changelog validation against the upstream base, and git diff --check passed.
• Earlier additional visualizer validation: 36 passed, one failed because a test reaches carb without a loaded Kit runtime in the local setup. No visualizer code is changed.

Local simulation checks used the available source-built Isaac Sim runtime, not the internal CI image digest from the original failure.

Type of change

• Bug fix

Release backport

• Backport this pull request to the active release branch after it merges into develop

Checklist

• I have read the contribution guidelines.
• I have run the pre-commit checks with uv run isaaclab --format.
• I have updated the affected API docstrings.
• I have validated the changed behavior with focused coverage and applied the test-audit criteria.
• I have added a changelog fragment for the touched package.
• My name already exists in CONTRIBUTORS.md.
• CI validation against the failing Kit image is complete.

[ooctipus]

run-ci

[StafaH]

run-ci

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Changes simulation shutdown order and resource cleanup.

The PR appears safe to merge, though the new cleanup-order and ownership behavior should have a regression test.

Findings

1.  Shutdown order lacks regression coverage ▶

Summary

The PR moves simulation-context cleanup ahead of runtime shutdown and makes clear_instance() stop the simulation before releasing its resources.

• A launch scope clears a context created within it while preserving one that existed on entry.
• The context builder now relies on clear_instance() for stopping and teardown.

Diagram

sequenceDiagram
    participant C as Caller
    participant L as launch_simulation
    participant S as SimulationContext
    participant R as Runtime launcher
    C->>L: Enter scope
    L->>R: Start runtime
    L->>L: Record existing context
    L-->>C: Yield physics configuration
    C-->>L: Exit scope
    alt Context created inside scope
        L->>S: clear_instance()
        S->>S: stop(), then release resources
    else Existing caller-owned context
        L->>L: Preserve context
    end
    L->>R: close(exit_code)

Loading

_{Reviews (1) · Last reviewed commit: "Reuse existing exception handling for si..."}

[greptile-apps]

Shutdown order lacks regression coverage The new cleanup path is not tested for its key behavior: stopping and clearing a context created inside the scope before the launcher closes, while preserving a context that existed on entry. The existing launcher test checks only the failure exit code, and the context test checks STOP dispatch separately. A regression in the ordering or ownership check could therefore pass the current tests.

Knowledge Base Used:

• Core simulation framework
• Simulation, rendering, and sensors

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[isaaclab-review-bot]

Isaac Lab Review Bot

The shutdown ordering is coherently centralized: SimulationContext.clear_instance() now stops producers before releasing physics, rendering, visualizer, backend, and stage resources, while launch_simulation() clears only a context created within its scope before closing runtimes. No blocking issue was substantiated; validation against the originally failing CI image remains a non-blocking residual risk.

• Design and architecture: The ownership split is appropriate. SimulationContext.clear_instance() owns complete context teardown, while launch_simulation() compares singleton identity to preserve a caller-owned context and uses an inner finally to release a scope-created context before launcher shutdown.
• API: The affected public surfaces were checked. launch_simulation() documents its new scope-cleanup behavior, and SimulationContext.clear_instance() explicitly documents that clearing now stops the simulation. No symbols, signatures, exports, configuration schemas, or stubs were added or removed.
• Compatibility and deprecation: Breaking changes: none identified. The prior headless-only stop in build_simulation_context() was replaced by the intentional stop-before-cleanup contract in clear_instance(), and that behavior is recorded in both the updated API docstring and package changelog. The previous has_gui condition alone does not establish a supported promise that GUI contexts must avoid STOP during final teardown.
• Implementation: The relevant control-flow paths were traced. Context cleanup runs before reversed launcher closure; cleanup failures continue through the existing teardown-error collector; body, KeyboardInterrupt, and SystemExit paths retain the established exit-code handling; and the deferred simulation-context import occurs after runtime launch, avoiding premature simulator dependency loading.
• Style consistency: The new comments explain the non-obvious teardown-order constraints rather than merely narrating syntax, the local import is consistent with deferred simulator dependencies, the public docstrings reflect the changed behavior, and the required package changelog fragment is present.
• Test quality: No test files were changed, so there were no new cases, fixtures, parameter matrices, or duplicated owner-boundary tests to audit. The PR reports focused launcher and simulation-context coverage plus standalone exception-path checks; reproducing the intermittent native abort on the original CI image remains outstanding validation rather than a demonstrated code defect.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

[StafaH]

run-ci