Decide thread-safety strategy for SimulationController (EDT enforcement)

[bedaHovorka]

Context

desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/SimulationController.kt mutates Swing components (controlPanel.updateStatus / setStopEnabled) from the caller thread of start() / stop(). Today the contract is documented in KDoc but not enforced at this layer — production correctness relies on Frame.startSimulation / Frame.stopSimulation having require(SwingUtilities.isEventDispatchThread()) guards one level up.

Originally raised by Copilot review (PR #481, comment 3143794605):

SimulationController mutates Swing components on the caller thread. ControlPanel's contract says these must be called on the EDT, and start() / stop() can currently be invoked from non-EDT (e.g. unit tests), which can lead to Swing thread-safety issues and flaky behavior.

Punted from PR #481 — kept current behaviour (option 4 below) so the headless SimulationControllerTest keeps working without a Swing harness. Tracked here to decide deliberately rather than by drift.

Options

1. Hard EDT enforcement

require(SwingUtilities.isEventDispatchThread()) at the top of start() and stop().

• ✅ Same loud failure mode as Frame; consistent style across the GUI layer.
• ❌ Breaks headless SimulationControllerTest — tests have to wrap every call in SwingUtilities.invokeAndWait { ... }.

2. Marshal to EDT via invokeLater

Wrap each controlPanel.* call in SwingUtilities.invokeLater { ... }; runner start/stop stays synchronous.

• ✅ Callable from any thread (production EDT, tests off-EDT).
• ❌ State changes become async — setStopEnabled(true) happens after start() returns, so observation-after-call patterns need latches; ordering of stop+start can race against the queue.

3. Marshal via invokeAndWait

Synchronous variant of #2.

• ✅ Preserves current call-and-observe semantics.
• ❌ Deadlocks if called from the EDT (which is the production path) — needs if (isEventDispatchThread) run else invokeAndWait switch. Extra machinery for marginal gain.

4. Document only — current state ✅

Keep Frame as the EDT enforcement boundary; SimulationController KDoc explains the contract.

• ✅ Zero churn; tests stay simple; production already covered by Frame's require guards.
• ❌ internal visibility is the only thing protecting the contract at this layer.

5. Split the API (principled refactor)

Make SimulationController UI-agnostic — it owns the runner + lifecycle state and exposes callbacks (onStateChanged: (SimulationStatus) -> Unit); Frame wires those callbacks to controlPanel on the EDT.

• ✅ Clean separation of concerns; SimulationController stops touching Swing entirely; trivially testable; aligns with Phase 2 Kalasim migration where simulation lifecycle should be UI-independent anyway.
• ❌ Larger refactor than Feature/sonar #1–Feature/kotlin corrected #4; changes the public-ish (internal) shape.

Recommendation

Defer #5 until a follow-up that already touches this area (e.g. Goal 7 Phase 1.3 speed control, or the Kalasim migration). For now #4 (status quo) is fine; revisit if a real off-EDT bug surfaces.

Acceptance Criteria

• Pick one of the options above (or define a Non gui classes has dependency to java.awt #6).
• If Feature/sonar #1 chosen: update SimulationControllerTest to use invokeAndWait and confirm headless CI still passes.
• If Cosmetic code review recommendations #5 chosen: open a design doc / sub-issue with the new callback shape before implementation.
• If status quo (Feature/kotlin corrected #4) confirmed: tighten the KDoc to explicitly state the rationale (test-callability) so future contributors don't second-guess it.

References

• PR Phase 1.2: Wire SimulationRunner into GUI lifecycle (simgui mode + Stop button) #481 review thread 4177162654
• Frame EDT guards: desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/Frame.kt:168, 201, 237, 294, 331, 352
• Current SimulationController KDoc: desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/SimulationController.kt:32-39

[bedaHovorka]

preferred is option 5, and 2 only as plan B. Other options are not acceptable