Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
16 changes: 14 additions & 2 deletions source/isaaclab/isaaclab/app/sim_launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -629,12 +632,21 @@ 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
# 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()
Comment on lines +645 to +649

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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:

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!

except KeyboardInterrupt:
exit_code = 130
raise
Expand Down
8 changes: 4 additions & 4 deletions source/isaaclab/isaaclab/sim/simulation_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []
Expand All @@ -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
Expand Down Expand Up @@ -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()
24 changes: 18 additions & 6 deletions source/isaaclab/test/sim/test_physics_manager_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -307,19 +316,21 @@ 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:
SimulationContext.clear_instance()

SimulationContext.clear_instance()
assert events == [
"stop",
*([] if fail_cleanup else ["visualizer_failed_stop", "visualizer_last_stop"]),
"physics",
"writers",
"renderer_failed",
Expand Down Expand Up @@ -356,7 +367,8 @@ def close(self):
pass

class Context:
pass
def stop(self):
pass

context = Context()
context.physics_manager = Manager
Expand Down
Loading