fix(core): wait for in-flight save before shutting down systems

[soloturn]

Fixes #704 - events getting sent to ComponentSystems after their shutdown() has already run, causing NPEs from internal state the system's own shutdown() had already cleared (original report: SignalSystem.producerRemoved crashing on a null map during host shutdown).

Root cause

StateIngame.dispose() calls storageManager.waitForCompletionOfPreviousSaveAndStartSaving(), which starts a SaveTransaction on GameScheduler.parallel() - a genuine background thread, per ReadWriteStorageManager.startSaving()'s Mono.fromRunnable(...).subscribeOn(GameScheduler.parallel()).subscribe(). That transaction's run() calls privateEntityManager.deactivateForStorage(entityRef) while serializing entities, which sends deactivation events to whatever ComponentSystems are still registered to handle them.

dispose() went on to call componentSystemManager.shutdown() while that save was still running in the background, and only blocked on it much later via storageManager.finishSavingAndShutdown() - after componentSystemManager.shutdown(), PhysicsEngine.dispose(), and entityManager.clear() had already run. Any deactivation event the save dispatched in that window landed on a system whose own shutdown() may have already nulled out the state its event handler needed - the exact NPE in #704's stack trace, just relocated from the 2013-era synchronous save path to the current async one.

Fix

Move the finishSavingAndShutdown() blocking call to immediately before componentSystemManager.shutdown(), so the save (and every event it dispatches) is guaranteed complete before any system, physics, or entity teardown begins. No new logic - two existing calls reordered.

Verification

:engine:compileJava clean. Ran the full engine-tests:integrationTest suite (18 MTE test classes, 34 tests) - StateIngame.dispose() is exactly the method every test's teardown goes through, so this is a meaningful regression check for the reordering. All 34 pass.

This is inherently hard to verify beyond that: reproducing #704 needs an actual multiplayer host mid-save at the moment of shutdown, which isn't something I can drive here. The integration suite exercising this exact dispose() path on every test teardown, 34/34 green, is the best available signal that the reorder doesn't break normal shutdown.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b7890ae-ba0c-4f16-93b9-f1d8c13febc3

📥 Commits

Reviewing files that changed from the base of the PR and between 338d7dd and eabb792.

📒 Files selected for processing (1)

• engine/src/main/java/org/terasology/engine/core/modes/StateIngame.java

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved shutdown handling to ensure background saves finish before storage and game systems are closed.
  • Prevented potential data loss or incomplete saves during application exit.

Walkthrough

StateIngame.dispose now completes background saves and shuts down storage before it disposes component systems, physics, and entities.

Changes

In-game shutdown

Layer / File(s)	Summary
Finalize saves before system teardown engine/src/main/java/org/terasology/engine/core/modes/StateIngame.java	The disposal sequence finalizes storage saves before dependent systems are shut down. The previous post-teardown finalization block was removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to eabb7

The shutdown ordering now waits for in-flight saves before tearing down systems, preventing late save events from reaching cleared state. The change is localized and verified by the reported checks; no actionable merge-blocking risk remains after normal review.

Poem

A rabbit checks the save with care,
Before the systems leave the lair.
Storage rests, then worlds depart,
No late events disturb the start.
Hop, hop—the shutdown flows just right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes waiting for in-flight saves before shutting down systems.
Description check	✅ Passed	The description explains the shutdown race, the reordered calls, and the reported verification results.
Linked Issues check	✅ Passed	The change addresses issue #704 by completing asynchronous saves before component systems shut down.
Out of Scope Changes check	✅ Passed	The changes are limited to reordering shutdown operations in StateIngame.dispose() and match the linked issue objective.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch soloturn-fix-events-after-shutdown

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}