From 79cbd31b791fc7cdbddc9831a83744d4bfbc70f8 Mon Sep 17 00:00:00 2001 From: Mustafa Haiderbhai Date: Thu, 1 Oct 2026 23:26:21 -0700 Subject: [PATCH 1/4] Stop simulations before shutting down their runtimes --- .../orderly-runtime-shutdown.fixed.rst | 2 + source/isaaclab/isaaclab/app/sim_launcher.py | 24 ++++++- .../isaaclab/sim/simulation_context.py | 8 +-- .../app/test_launch_simulation_require_kit.py | 72 +++++++++++++++++++ 4 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 source/isaaclab/changelog.d/orderly-runtime-shutdown.fixed.rst diff --git a/source/isaaclab/changelog.d/orderly-runtime-shutdown.fixed.rst b/source/isaaclab/changelog.d/orderly-runtime-shutdown.fixed.rst new file mode 100644 index 000000000000..4f5c624f560c --- /dev/null +++ b/source/isaaclab/changelog.d/orderly-runtime-shutdown.fixed.rst @@ -0,0 +1,2 @@ +* Stopped simulations and released their resources before shutting down the enclosing runtime, + preserving failure exit codes and contexts owned by an outer launch scope. diff --git a/source/isaaclab/isaaclab/app/sim_launcher.py b/source/isaaclab/isaaclab/app/sim_launcher.py index 8cf8b8cfd8d8..075765c670f3 100644 --- a/source/isaaclab/isaaclab/app/sim_launcher.py +++ b/source/isaaclab/isaaclab/app/sim_launcher.py @@ -541,6 +541,9 @@ def launch_simulation( needed), then starts the launcher each required runtime's config names (closed on exit) or does nothing for kitless ones. Cameras are auto-enabled for Kit-renderer sensors. + On exit, stops and releases a simulation context created inside the scope before closing + its runtimes. An existing context owned by the caller is preserved. + The run's visualizers and device are decided here, once: they are written to the :class:`~isaaclab.sim.SimulationCfg` in *cfg* (``visualizer_cfgs`` and ``device``), which every later consumer reads. ``--visualizer`` selects the visualizers that run, none without it; @@ -629,6 +632,9 @@ def launch_simulation( settings.set("/isaaclab/visualizer/types", ",".join(args["visualizer"]) if sim_cfg is None else "") settings.set("/isaaclab/visualizer/max_visible_envs", -1 if max_visible_envs is None else int(max_visible_envs)) + from ..sim.simulation_context import SimulationContext + + previous_context = SimulationContext.instance() exit_code = 0 try: # With no selected profile this is a no-op; with one, it installs process-wide OmniClient @@ -647,5 +653,19 @@ def launch_simulation( traceback.print_exc() raise finally: - for launcher in reversed(launchers): - launcher.close(exit_code) + try: + context = SimulationContext.instance() + if context is not None and context is not previous_context: + try: + SimulationContext.clear_instance() + except Exception: + # Preserve the user's exception, but never report successful shutdown + # when releasing simulation resources failed. + if exit_code == 0: + exit_code = 1 + raise + logger.exception("Failed to clean up the simulation before runtime shutdown") + finally: + # Runtime shutdown may terminate the process; release its consumers first. + for launcher in reversed(launchers): + launcher.close(exit_code) diff --git a/source/isaaclab/isaaclab/sim/simulation_context.py b/source/isaaclab/isaaclab/sim/simulation_context.py index 0bd6f4ffa41b..e47ff9cf4b84 100644 --- a/source/isaaclab/isaaclab/sim/simulation_context.py +++ b/source/isaaclab/isaaclab/sim/simulation_context.py @@ -790,7 +790,7 @@ def close_backend(self, backend: Any) -> None: @classmethod def clear_instance(cls) -> None: - """Clean up resources and clear the singleton instance.""" + """Stop the simulation, clean up resources, and clear the singleton instance.""" instance = cls._instance if instance is not None: teardown_errors: list[Exception] = [] @@ -802,7 +802,9 @@ def run_cleanup(callback: Callable[[], Any]) -> None: teardown_errors.append(exc) try: - # Close physics manager FIRST to detach PhysX from the stage. + # Stop task producers and deliver STOP before releasing their resources. + run_cleanup(instance.stop) + # Detach PhysX before any stage-bound resource or prim is deleted. run_cleanup(instance.physics_manager.close) # Close camera renderers after STOP invalidates camera-owned render data and @@ -935,6 +937,4 @@ def build_simulation_context( raise finally: if sim is not None: - if not sim.has_gui: - sim.stop() sim.clear_instance() diff --git a/source/isaaclab/test/app/test_launch_simulation_require_kit.py b/source/isaaclab/test/app/test_launch_simulation_require_kit.py index 41063ec64cc3..678a8e4f11ee 100644 --- a/source/isaaclab/test/app/test_launch_simulation_require_kit.py +++ b/source/isaaclab/test/app/test_launch_simulation_require_kit.py @@ -26,6 +26,7 @@ from isaaclab.app import SimulationLauncher, launch_simulation from isaaclab.physics import PhysicsCfg from isaaclab.renderers import RendererCfg +from isaaclab.sim import BackendCfg, SimulationCfg, SimulationContext @pytest.fixture @@ -182,3 +183,74 @@ def test_require_kit_false_does_not_suppress_a_kit_config(kit_branch_taken): pass assert kit_branch_taken == [True] + + +@pytest.mark.parametrize( + "body_error, cleanup_fails, expected_status", + [ + (None, False, 0), + (ValueError, False, 1), + (KeyboardInterrupt, False, 130), + (SystemExit, False, 7), + (None, True, 1), + (ValueError, True, 1), + ], +) +def test_launch_releases_simulation_before_runtime_exit(monkeypatch, body_error, cleanup_fails, expected_status): + """Release real context resources before a runtime can exit, including failed runs and cleanup.""" + closed = [] + + class Runtime(SimulationLauncher): + def close(self, exit_code=0): + assert SimulationContext.instance() is None + assert closed == ["resource"] + closed.append(exit_code) + + class Resource: + def __init__(self, cfg): + pass + + def close(self): + assert sim.is_stopped() + closed.append("resource") + if cleanup_fails: + raise RuntimeError("cleanup failed") + + class RuntimeRendererCfg(RendererCfg): + launcher_type = "shutdown_runtime:Runtime" + + monkeypatch.setitem(sys.modules, "shutdown_runtime", types.SimpleNamespace(Runtime=Runtime)) + cfg = SimulationCfg(physics=sim_launcher.NewtonCfg(), device="cpu", visualizer_cfgs=[]) + launch_cfg = argparse.Namespace(sim=cfg, renderer=RuntimeRendererCfg()) + expected_error = body_error or (RuntimeError if cleanup_fails else None) + + try: + with launch_simulation(launch_cfg): + sim = SimulationContext(cfg) + sim.get_or_create_backend(BackendCfg(class_type=Resource)) + sim.play() + if body_error is not None: + raise body_error(7 if body_error is SystemExit else "body failed") + except BaseException as exc: + assert expected_error is not None + assert type(exc) is expected_error + assert str(exc) == ("7" if body_error is SystemExit else "body failed" if body_error else "cleanup failed") + else: + assert expected_error is None + finally: + SimulationContext.clear_instance() + + assert closed == ["resource", expected_status] + + +def test_nested_launch_preserves_outer_simulation(): + """An inner launch must not stop or release a context owned by its caller.""" + cfg = SimulationCfg(physics=sim_launcher.NewtonCfg(), device="cpu", visualizer_cfgs=[]) + with launch_simulation(cfg): + sim = SimulationContext(cfg) + sim.play() + with launch_simulation(cfg): + pass + assert SimulationContext.instance() is sim + assert sim.is_playing() + assert SimulationContext.instance() is None From d9405c1993a28cc1cfb44e4ce06660f3340879a8 Mon Sep 17 00:00:00 2001 From: Mustafa Haiderbhai Date: Thu, 1 Oct 2026 23:31:12 -0700 Subject: [PATCH 2/4] Flatten shutdown cleanup and remove added test scaffolding --- source/isaaclab/isaaclab/app/sim_launcher.py | 15 ++-- .../app/test_launch_simulation_require_kit.py | 72 ------------------- 2 files changed, 6 insertions(+), 81 deletions(-) diff --git a/source/isaaclab/isaaclab/app/sim_launcher.py b/source/isaaclab/isaaclab/app/sim_launcher.py index 075765c670f3..63b521fd9a7d 100644 --- a/source/isaaclab/isaaclab/app/sim_launcher.py +++ b/source/isaaclab/isaaclab/app/sim_launcher.py @@ -656,15 +656,12 @@ def launch_simulation( try: context = SimulationContext.instance() if context is not None and context is not previous_context: - try: - SimulationContext.clear_instance() - except Exception: - # Preserve the user's exception, but never report successful shutdown - # when releasing simulation resources failed. - if exit_code == 0: - exit_code = 1 - raise - logger.exception("Failed to clean up the simulation before runtime shutdown") + SimulationContext.clear_instance() + except Exception: + if exit_code == 0: + exit_code = 1 + raise + logger.exception("Failed to clean up the simulation before runtime shutdown") finally: # Runtime shutdown may terminate the process; release its consumers first. for launcher in reversed(launchers): diff --git a/source/isaaclab/test/app/test_launch_simulation_require_kit.py b/source/isaaclab/test/app/test_launch_simulation_require_kit.py index 678a8e4f11ee..41063ec64cc3 100644 --- a/source/isaaclab/test/app/test_launch_simulation_require_kit.py +++ b/source/isaaclab/test/app/test_launch_simulation_require_kit.py @@ -26,7 +26,6 @@ from isaaclab.app import SimulationLauncher, launch_simulation from isaaclab.physics import PhysicsCfg from isaaclab.renderers import RendererCfg -from isaaclab.sim import BackendCfg, SimulationCfg, SimulationContext @pytest.fixture @@ -183,74 +182,3 @@ def test_require_kit_false_does_not_suppress_a_kit_config(kit_branch_taken): pass assert kit_branch_taken == [True] - - -@pytest.mark.parametrize( - "body_error, cleanup_fails, expected_status", - [ - (None, False, 0), - (ValueError, False, 1), - (KeyboardInterrupt, False, 130), - (SystemExit, False, 7), - (None, True, 1), - (ValueError, True, 1), - ], -) -def test_launch_releases_simulation_before_runtime_exit(monkeypatch, body_error, cleanup_fails, expected_status): - """Release real context resources before a runtime can exit, including failed runs and cleanup.""" - closed = [] - - class Runtime(SimulationLauncher): - def close(self, exit_code=0): - assert SimulationContext.instance() is None - assert closed == ["resource"] - closed.append(exit_code) - - class Resource: - def __init__(self, cfg): - pass - - def close(self): - assert sim.is_stopped() - closed.append("resource") - if cleanup_fails: - raise RuntimeError("cleanup failed") - - class RuntimeRendererCfg(RendererCfg): - launcher_type = "shutdown_runtime:Runtime" - - monkeypatch.setitem(sys.modules, "shutdown_runtime", types.SimpleNamespace(Runtime=Runtime)) - cfg = SimulationCfg(physics=sim_launcher.NewtonCfg(), device="cpu", visualizer_cfgs=[]) - launch_cfg = argparse.Namespace(sim=cfg, renderer=RuntimeRendererCfg()) - expected_error = body_error or (RuntimeError if cleanup_fails else None) - - try: - with launch_simulation(launch_cfg): - sim = SimulationContext(cfg) - sim.get_or_create_backend(BackendCfg(class_type=Resource)) - sim.play() - if body_error is not None: - raise body_error(7 if body_error is SystemExit else "body failed") - except BaseException as exc: - assert expected_error is not None - assert type(exc) is expected_error - assert str(exc) == ("7" if body_error is SystemExit else "body failed" if body_error else "cleanup failed") - else: - assert expected_error is None - finally: - SimulationContext.clear_instance() - - assert closed == ["resource", expected_status] - - -def test_nested_launch_preserves_outer_simulation(): - """An inner launch must not stop or release a context owned by its caller.""" - cfg = SimulationCfg(physics=sim_launcher.NewtonCfg(), device="cpu", visualizer_cfgs=[]) - with launch_simulation(cfg): - sim = SimulationContext(cfg) - sim.play() - with launch_simulation(cfg): - pass - assert SimulationContext.instance() is sim - assert sim.is_playing() - assert SimulationContext.instance() is None From 5bef658b0e36292828f4143fa873ede1d7dfcc96 Mon Sep 17 00:00:00 2001 From: Mustafa Haiderbhai Date: Thu, 1 Oct 2026 23:36:07 -0700 Subject: [PATCH 3/4] Reuse existing exception handling for simulation cleanup --- source/isaaclab/isaaclab/app/sim_launcher.py | 25 ++++++++------------ 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/source/isaaclab/isaaclab/app/sim_launcher.py b/source/isaaclab/isaaclab/app/sim_launcher.py index 63b521fd9a7d..2ab983fb2d47 100644 --- a/source/isaaclab/isaaclab/app/sim_launcher.py +++ b/source/isaaclab/isaaclab/app/sim_launcher.py @@ -639,8 +639,14 @@ def launch_simulation( try: # With no selected profile this is a no-op; with one, it installs process-wide OmniClient # routing before user code runs. - configure_storage_profile() - yield physics_cfg + try: + configure_storage_profile() + yield physics_cfg + finally: + # Release runtime consumers before a launcher can terminate the process. + context = SimulationContext.instance() + if context is not None and context is not previous_context: + SimulationContext.clear_instance() except KeyboardInterrupt: exit_code = 130 raise @@ -653,16 +659,5 @@ def launch_simulation( traceback.print_exc() raise finally: - try: - context = SimulationContext.instance() - if context is not None and context is not previous_context: - SimulationContext.clear_instance() - except Exception: - if exit_code == 0: - exit_code = 1 - raise - logger.exception("Failed to clean up the simulation before runtime shutdown") - finally: - # Runtime shutdown may terminate the process; release its consumers first. - for launcher in reversed(launchers): - launcher.close(exit_code) + for launcher in reversed(launchers): + launcher.close(exit_code) From 7e84d8bd34537d385440c7c53b0b4b076a906827 Mon Sep 17 00:00:00 2001 From: Mustafa Haiderbhai Date: Fri, 2 Oct 2026 11:14:58 -0700 Subject: [PATCH 4/4] Update lifecycle test doubles for orderly simulation shutdown --- .../sim/test_physics_manager_lifecycle.py | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/source/isaaclab/test/sim/test_physics_manager_lifecycle.py b/source/isaaclab/test/sim/test_physics_manager_lifecycle.py index c112e0ff393d..8cf50376e6be 100644 --- a/source/isaaclab/test/sim/test_physics_manager_lifecycle.py +++ b/source/isaaclab/test/sim/test_physics_manager_lifecycle.py @@ -267,17 +267,26 @@ def test_clear_instance_closes_renderers_before_native_backends(monkeypatch, ren events = [] class Manager: + @classmethod + def stop(cls): + events.append("stop") + if fail_cleanup: + raise RuntimeError("STOP failed") + @classmethod def close(cls): events.append("physics") if fail_cleanup: - raise RuntimeError("STOP failed") + raise RuntimeError("physics close failed") class Resource: def __init__(self, name, error=None): self.name = name self.error = error + def stop(self): + events.append(f"{self.name}_stop") + def close(self): events.append(self.name) if fail_cleanup and self.error is not None: @@ -307,12 +316,12 @@ def close(self): monkeypatch.setattr(context_module.gc, "collect", lambda: events.append("gc")) if fail_cleanup: - with pytest.raises(RuntimeError, match=r"4 error\(s\) occurred during teardown") as exc_info: + with pytest.raises(RuntimeError, match=r"5 error\(s\) occurred during teardown") as exc_info: SimulationContext.clear_instance() assert str(exc_info.value) == ( - "SimulationContext.clear_instance(): 4 error(s) occurred during teardown: " - "RuntimeError: STOP failed; OSError: renderer failed; ValueError: visualizer failed; " - "LookupError: backend failed" + "SimulationContext.clear_instance(): 5 error(s) occurred during teardown: " + "RuntimeError: STOP failed; RuntimeError: physics close failed; OSError: renderer failed; " + "ValueError: visualizer failed; LookupError: backend failed" ) assert str(exc_info.value.__cause__) == "STOP failed" else: @@ -320,6 +329,8 @@ def close(self): SimulationContext.clear_instance() assert events == [ + "stop", + *([] if fail_cleanup else ["visualizer_failed_stop", "visualizer_last_stop"]), "physics", "writers", "renderer_failed", @@ -356,7 +367,8 @@ def close(self): pass class Context: - pass + def stop(self): + pass context = Context() context.physics_manager = Manager