Fix interrupted tutorial shutdown - #529
Conversation
Close the native render window before scene teardown, release interrupted tutorial frames before destroying borrowed native resources, and safely discard partially constructed managers. Add focused regression coverage for normal, interrupted, and failed initialization paths.
Greptile SummaryThe PR makes interrupted atomic-action tutorial shutdown unwind Python frames before native teardown and closes the rendering window before scene resources are released.
Confidence Score: 4/5The implementation appears safe to merge, but the lifecycle-sensitive interrupt and teardown changes should have the regression tests claimed by the PR committed with them. No blocking failure was established in the cleanup implementation; the remaining concern is that the new exceptional-control-flow and native teardown branches lack changes to automated regression coverage. Files Needing Attention: scripts/tutorials/atomic_action/tutorial_utils.py; embodichain/lab/sim/sim_manager.py
|
| Filename | Overview |
|---|---|
| embodichain/lab/sim/sim_manager.py | Adds construction-state tracking and moves render-window closure ahead of native scene teardown; no concrete functional defect was established. |
| scripts/tutorials/atomic_action/tutorial_utils.py | Handles Ctrl+C inside the tutorial frame, distinguishes partial construction during cleanup, and exits with status 130, but the claimed regression tests are absent from the changeset. |
Sequence Diagram
sequenceDiagram
participant User
participant Tutorial as run_tutorial
participant Sim as SimulationManager
participant Window as Render window
participant Native as Scene / Env / World
User->>Tutorial: Ctrl+C
Tutorial->>Tutorial: Catch KeyboardInterrupt and unwind main frame
alt Construction completed
Tutorial->>Sim: "destroy(exit_process=False)"
Sim->>Window: close_window()
Sim->>Native: clean materials and environment
Sim->>Native: quit world
Tutorial->>Sim: flush_cleanup_queue()
else Partial construction
Tutorial->>Sim: reset(instance_id)
end
Tutorial-->>User: SystemExit(130)
Prompt To Fix All With AI
### Issue 1
scripts/tutorials/atomic_action/tutorial_utils.py:243-247
**Regression tests absent from changeset**
The new interrupt, partial-construction, and native teardown branches have no accompanying test changes, so later regressions in this lifecycle-sensitive behavior will not be detected by the repository's automated coverage despite the PR description claiming new regression tests.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "update" | Re-trigger Greptile
| finally: | ||
| if SimulationManager.is_instantiated(): | ||
| sim = SimulationManager.get_instance() | ||
| if sim.is_window_recording(): | ||
| sim.stop_window_record() | ||
| sim.wait_window_record_saves() | ||
| sim.destroy(exit_process=False) | ||
| SimulationManager.flush_cleanup_queue() | ||
| if not getattr(sim, "_is_constructed", False): | ||
| SimulationManager.reset(getattr(sim, "instance_id", 0)) |
There was a problem hiding this comment.
Regression tests absent from changeset
The new interrupt, partial-construction, and native teardown branches have no accompanying test changes, so later regressions in this lifecycle-sensitive behavior will not be detected by the repository's automated coverage despite the PR description claiming new regression tests.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/tutorials/atomic_action/tutorial_utils.py
Line: 243-247
Comment:
**Regression tests absent from changeset**
The new interrupt, partial-construction, and native teardown branches have no accompanying test changes, so later regressions in this lifecycle-sensitive behavior will not be detected by the repository's automated coverage despite the PR description claiming new regression tests.
**Context Used:** AGENTS.md ([source](https://github.com/dexforce/embodichain/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
Description
This PR makes atomic-action tutorial shutdown safe when execution is interrupted with Ctrl+C.
SimulationManager, so traceback-held Python wrappers cannot outlive their nativeWorldSimulationManagerconstruction without touching unavailable native stateThe original failure was reproducible as a Vulkan acquire-semaphore validation error followed by a segmentation fault during native material destruction. A normally completed tutorial was unaffected.
No public API or dependency changes are included.
Type of change
Screenshots
Not applicable.
Validation
black .— 681 files left unchangedplace.py --auto_playCtrl+C regression — clean exit, no Vulkan validation error, no segmentation faultThe focused pytest run used a process-local compatibility shim because the installed local DexSim build predates
dexsim.types.DenoiserType; no compatibility code is part of this PR.Checklist