Skip to content

Commit 34301f8

Browse files
CopilotbedaHovorka
authored andcommitted
SP0.10: AgentLoopDriver — paced sense-decide-act loop via SimulationController (#739)
* SP0.10: Add AgentLoopDriver — paced sense-decide-act loop via SimulationController * SP0.10: fix review — off-thread-safe snapshot + contract KDoc Address Important + Minor findings from PR #739 review (#732 SP0.10). Issue 2 — DefaultNetworkPerceptionPort.snapshot() was not off-thread-safe: it read plain non-volatile mutable sim fields and iterated the live approwedTrains list, so an off-thread driver call could throw a ConcurrentModificationException. Fix follows the spec's capture-on-sim-thread model (never blocks/interrupts the kDisco kernel — no sim-thread lock): - Add @volatile latestCaptured cache; captureSnapshot() does the fresh on-thread read and publishes; snapshot() returns latestCaptured or SimulationSnapshot.EMPTY — a pure cached read that never touches live state, never throws, never blocks the kernel. - Add SimulationSnapshot.EMPTY companion (default before first capture). - Migrate ShuntingLoop's two on-thread snapshot() calls (admission + path-advancement) to captureSnapshot() — behavior-preserving (still fresh reads, plus the cache publish side-effect). - AgentLoopDriver keeps calling snapshot(), now genuinely off-thread-safe. Issue 1 — contract drift: the :core KDoc on Dispatcher, SimulationController and NetworkPerceptionPort still claimed single-kDisco-thread exclusivity, contradicting the SP0.5 spec which runs the drive loop on the driver's own thread. Updated to reflect that the kDisco kernel runs on its own single thread while outside callers (the SP0.10 driver, future LLM) run in their own threads; implementations must not rely on kDisco-thread exclusivity. Minor 4 — rewrote the "mirrors DefaultSimulationContext" comment in AgentLoopDriver.runCycle() without exact numbers and without the misleading call-order claim (DefaultSimulationContext does throttle→awaitIfPaused; the driver does awaitIfPaused→throttle); kept an accurate initial-delta convention reference. Tests: added captureSnapshot() to the two SimulationSnapshotTest stubs; migrated the six fresh-read assertions in DefaultNetworkPerceptionPortTest to captureSnapshot() and added two cached-snapshot() tests (returns EMPTY before first capture; returns last capture and stays frozen through later mutations). AgentLoopDriverTest unchanged (driver uses the mocked snapshot()).
1 parent 61188f4 commit 34301f8

11 files changed

Lines changed: 744 additions & 67 deletions

File tree

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/context/SimulationController.kt

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,23 @@ package cz.vutbr.fit.interlockSim.context
1818
*
1919
* ## Threading Contract
2020
*
21-
* - [awaitIfPaused] is called from the simulation coroutine (kDisco dispatcher thread).
22-
* It suspends (not blocks) so the coroutine yields while paused.
23-
* - [throttle] is called from the simulation coroutine to apply wall-clock pacing.
24-
* - [isPaused] is called from the simulation coroutine to check pause state.
25-
* - [pollStepEvent] is called from the simulation coroutine; returns `true` once if
26-
* a step-event was requested, then resets.
27-
* - [pollStepTime] is called from the simulation coroutine; returns the pending
28-
* time-delta once, then resets.
21+
* The kDisco simulation kernel runs on its own single thread. The methods below
22+
* are invoked from one of several outside callers, which may run concurrently
23+
* with each other and with the kernel:
24+
* - the **simulation coroutine** — the controlled event loop in
25+
* `DefaultSimulationContext` (GUI/animated runs) — calls [awaitIfPaused],
26+
* [throttle], [isPaused], [pollStepEvent], [pollStepTime];
27+
* - the **external driver coroutine** — the SP0.10 drive-loop driver (#732), which
28+
* paces its own sense→decide→act cycle independently of the kernel — calls
29+
* [awaitIfPaused] and [throttle];
30+
* - [requestPause] is called by external agents (e.g. the collision detection
31+
* service) from arbitrary threads.
2932
*
30-
* The GUI/control thread sets pause state and enqueues step requests via its own
31-
* implementation (e.g., `SimulationRunner`). The simulation coroutine polls these
32-
* values through this interface.
33+
* Implementations must be thread-safe across all of these callers: the
34+
* GUI/control thread sets pause state and enqueues step requests, while the
35+
* simulation coroutine and the external driver coroutine poll. The interface
36+
* itself stays KMP-pure (see below); platform-specific synchronisation lives in
37+
* implementations such as `SimulationRunner` / `NoOpSimulationController`.
3338
*
3439
* ## KMP Purity
3540
*
@@ -44,18 +49,19 @@ interface SimulationController {
4449
/**
4550
* Suspends the simulation coroutine while the controller is in paused state.
4651
*
47-
* Called from the simulation coroutine at each iteration of the controlled loop.
48-
* When paused, this function suspends until the controller resumes or a step
49-
* is requested. Uses `suspend` (not blocking) so it cooperates with kDisco's
50-
* coroutine-based dispatcher.
52+
* Called from the simulation coroutine or the external driver coroutine at each
53+
* iteration of the controlled loop. When paused, this function suspends until
54+
* the controller resumes or a step is requested. Uses `suspend` (not blocking)
55+
* so it cooperates with kDisco's coroutine-based dispatcher.
5156
*/
5257
suspend fun awaitIfPaused()
5358

5459
/**
5560
* Applies wall-clock throttling to pace the simulation relative to real time.
5661
*
57-
* Called from the simulation coroutine after advancing the simulation clock.
58-
* The implementation may delay execution to match the desired speed multiplier.
62+
* Called from the simulation coroutine or the external driver coroutine after
63+
* advancing the simulation clock. The implementation may delay execution to
64+
* match the desired speed multiplier.
5965
*
6066
* @param simDeltaSeconds the simulation time that has just elapsed (in seconds)
6167
*/
@@ -64,8 +70,8 @@ interface SimulationController {
6470
/**
6571
* Returns the current pause state.
6672
*
67-
* Called from the simulation coroutine to determine whether to enter the
68-
* pause-wait loop.
73+
* Called from the simulation coroutine or the external driver coroutine to
74+
* determine whether to enter the pause-wait loop.
6975
*
7076
* @return `true` if the simulation is currently paused
7177
*/

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/ports/DefaultNetworkPerceptionPort.kt

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,17 @@ import cz.vutbr.fit.interlockSim.util.DynamicWrapperUtils
5151
*
5252
* ## Thread-safety
5353
*
54-
* This class is **not thread-safe**. Both the [SimulationEnvironment] state and the
55-
* [activeTrains] list are mutated by the kDisco simulation thread; all queries must be
56-
* issued from the same thread.
54+
* The kDisco simulation kernel runs on its own single thread; both the
55+
* [SimulationEnvironment] state and the [activeTrains] list are mutated by that
56+
* thread without synchronisation. [captureSnapshot] and the `allXxx()` /
57+
* single-query methods read that live state and **must** be called on the kDisco
58+
* thread.
59+
*
60+
* [snapshot] is the exception: it reads only a `@Volatile` reference to the most
61+
* recent snapshot published by [captureSnapshot], so it is safe to call from any
62+
* thread (including the SP0.10 drive-loop driver) — it never blocks or interrupts
63+
* the kDisco kernel and never throws. Before the first [captureSnapshot] it returns
64+
* [SimulationSnapshot.EMPTY].
5765
*
5866
* @param env The simulation environment to read semaphore and block state from.
5967
* @param activeTrains A supplier that returns the list of currently active (approved
@@ -231,29 +239,58 @@ class DefaultNetworkPerceptionPort(
231239
// ── Full snapshot ─────────────────────────────────────────────────────
232240

233241
/**
234-
* Captures a frozen [SimulationSnapshot] of the complete observable network state.
242+
* The most recent snapshot published by [captureSnapshot], or `null` before the
243+
* first on-thread capture.
244+
*
245+
* `@Volatile` so an off-thread [snapshot] caller (the SP0.10 drive-loop driver)
246+
* observes the latest published reference without locking; the published
247+
* [SimulationSnapshot] is itself immutable, so once a reference is seen it is safe
248+
* to read concurrently. Written only from [captureSnapshot] on the kDisco thread.
249+
*/
250+
@kotlin.concurrent.Volatile
251+
private var latestCaptured: SimulationSnapshot? = null
252+
253+
/**
254+
* Returns the most recent snapshot captured on the kDisco thread, safe to call
255+
* from any thread (see [NetworkPerceptionPort.snapshot]).
256+
*
257+
* This is the **off-thread-safe** accessor: it reads only [latestCaptured] and
258+
* never touches live simulation state, so it cannot block or interrupt the kDisco
259+
* kernel and cannot throw. Before the first [captureSnapshot] it returns
260+
* [SimulationSnapshot.EMPTY].
261+
*/
262+
override fun snapshot(): SimulationSnapshot = latestCaptured ?: SimulationSnapshot.EMPTY
263+
264+
/**
265+
* Captures a fresh, consistent [SimulationSnapshot] of the complete observable
266+
* network state on the kDisco thread and publishes it to [latestCaptured].
235267
*
236268
* Calls each `allXxx()` bulk query in sequence. Because kDisco runs on a single
237269
* simulation thread, all four calls observe the same simulation state — no events
238-
* interleave between them during a normal tick.
270+
* interleave between them during a normal tick. The result is published via
271+
* [latestCaptured] so off-thread [snapshot] callers see a consistent picture.
239272
*
240273
* [SimulationSnapshot.simTime] is set from [Process.time]; if called outside an
241274
* active kDisco simulation (e.g. in unit tests), the resulting [DiscoException]
242275
* is caught and `simTime` falls back to `0.0`. Any other exception propagates.
243276
*/
244-
override fun snapshot(): SimulationSnapshot =
245-
SimulationSnapshot(
246-
simTime =
247-
try {
248-
Process.time()
249-
} catch (_: DiscoException) {
250-
0.0
251-
},
252-
semaphores = allSignalAspects(),
253-
blocks = allBlockOccupancies(),
254-
trainPositions = allTrainPositions(),
255-
timetables = allTrainTimetables()
256-
)
277+
override fun captureSnapshot(): SimulationSnapshot {
278+
val captured =
279+
SimulationSnapshot(
280+
simTime =
281+
try {
282+
Process.time()
283+
} catch (_: DiscoException) {
284+
0.0
285+
},
286+
semaphores = allSignalAspects(),
287+
blocks = allBlockOccupancies(),
288+
trainPositions = allTrainPositions(),
289+
timetables = allTrainTimetables()
290+
)
291+
latestCaptured = captured
292+
return captured
293+
}
257294

258295
// ── Grid scan ─────────────────────────────────────────────────────────
259296

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/ports/NetworkPerceptionPort.kt

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ import cz.vutbr.fit.interlockSim.objects.cells.Signal
4040
* - **Core-only**: this interface lives in `:core` with no dependency on Koog, Ollama,
4141
* or any agent framework. The agent framework wiring happens in `:dispatcher-agent`
4242
* (SP1 Issues #546–#551).
43-
* - **Thread-safety**: implementations are expected to be called from the single kDisco
44-
* simulation thread. No external synchronisation is required.
43+
* - **Thread-safety**: the kDisco simulation kernel runs on its own single thread;
44+
* callers outside the simulation (e.g. the SP0.10 drive-loop driver) run on their
45+
* own threads. [snapshot] is safe to call from any thread — it returns the most
46+
* recent snapshot captured on the kDisco thread (off-thread, possibly stale, and
47+
* it never blocks the kernel). [captureSnapshot] and the individual `allXxx()` /
48+
* single-query methods read live simulation state and must be called on the
49+
* kDisco thread.
4550
*
4651
* @see DefaultNetworkPerceptionPort
4752
* @since Issue #541 (SP0.2 — Goal 10 sensor ports)
@@ -132,27 +137,52 @@ interface NetworkPerceptionPort {
132137
// ── Full snapshot ─────────────────────────────────────────────────────
133138

134139
/**
135-
* Capture a frozen [SimulationSnapshot] of the complete observable network state.
140+
* Return the most recent snapshot captured on the kDisco thread, safe to call from
141+
* any thread including the external driver thread (SP0.10).
142+
*
143+
* This is the **off-thread-safe** sense call: it does **not** read live simulation
144+
* state — it returns the snapshot most recently published by [captureSnapshot],
145+
* so it never blocks or interrupts the kDisco kernel and never throws. The
146+
* returned value is a frozen, immutable [SimulationSnapshot] but may be stale
147+
* relative to the current simulation time; before the first [captureSnapshot] an
148+
* empty snapshot (simulation time at the start baseline, empty lists) is returned.
149+
* Callers that hold it across `hold()` boundaries must treat it as potentially
150+
* stale.
151+
*
152+
* For a fresh, consistent, on-thread snapshot, call [captureSnapshot] (kDisco
153+
* thread only).
154+
*
155+
* @return An immutable [SimulationSnapshot] containing all observable state.
156+
*
157+
* @since Issue #543 (SP0.4 — Goal 10 observable simulation state); made
158+
* off-thread-safe (cached) in Issue #732 (SP0.10 — Goal 10)
159+
*/
160+
fun snapshot(): SimulationSnapshot
161+
162+
/**
163+
* Capture a fresh, consistent [SimulationSnapshot] by reading live network state,
164+
* and publish it as the latest snapshot returned by [snapshot].
136165
*
137166
* Calls all `allXxx()` bulk queries in sequence and bundles the results together
138-
* with the current simulation time into a single immutable value. Use this method
139-
* when you need a **consistent, frozen picture** of the entire network at one
140-
* moment — for example, to hand to an LLM dispatcher, to log the full state for
141-
* debugging, or to compare two tick states.
167+
* with the current simulation time into a single immutable value, then publishes
168+
* it so an off-thread [snapshot] caller (e.g. the SP0.10 drive-loop driver) sees
169+
* a consistent picture without reading live state itself. Use this method when you
170+
* need a **consistent, frozen picture** of the entire network at one moment — for
171+
* example, to log the full state for debugging, or to compare two tick states.
172+
*
173+
* Must be called on the kDisco simulation thread (it reads mutable simulation state
174+
* that is not synchronised for off-thread access). Returns the same freshly-captured
175+
* snapshot it published, so on-thread callers (e.g. `ShuntingLoop`) get a
176+
* consistent, frozen picture identical to the previous single-threaded `snapshot`
177+
* behaviour.
142178
*
143179
* For one-off queries (e.g. "is block k1 free right now?") prefer the individual
144180
* methods on this interface.
145181
*
146-
* ## Thread-safety
147-
*
148-
* As with all methods on this interface, implementations are expected to be called
149-
* from the single kDisco simulation thread. The snapshot is consistent within
150-
* a single-threaded call (no interleaved simulation events), but callers that hold
151-
* the snapshot across `hold()` boundaries must treat it as potentially stale.
182+
* @return An immutable [SimulationSnapshot] capturing the network state at the
183+
* current simulation time.
152184
*
153-
* @return An immutable [SimulationSnapshot] containing all observable state.
154-
*
155-
* @since Issue #543 (SP0.4 — Goal 10 observable simulation state)
185+
* @since Issue #732 (SP0.10 — Goal 10 off-thread-safe perception)
156186
*/
157-
fun snapshot(): SimulationSnapshot
187+
fun captureSnapshot(): SimulationSnapshot
158188
}

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/ports/SimulationSnapshot.kt

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,23 @@ data class SimulationSnapshot(
6565
val blocks: List<BlockOccupancyReading>,
6666
val trainPositions: List<TrainPositionReading>,
6767
val timetables: List<TimetableReading>
68-
)
68+
) {
69+
companion object {
70+
/**
71+
* Empty snapshot returned by [NetworkPerceptionPort.snapshot] before the first
72+
* on-thread [NetworkPerceptionPort.captureSnapshot] has published a real one.
73+
*
74+
* Safe to hand to a dispatcher from any thread: it carries no live state and
75+
* carries no active trains or signal changes, so a well-behaved dispatcher
76+
* responds with a no-op decision.
77+
*/
78+
val EMPTY: SimulationSnapshot =
79+
SimulationSnapshot(
80+
simTime = 0.0,
81+
semaphores = emptyList(),
82+
blocks = emptyList(),
83+
trainPositions = emptyList(),
84+
timetables = emptyList()
85+
)
86+
}
87+
}

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/Dispatcher.kt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,14 @@ package cz.vutbr.fit.interlockSim.sim
3737
* pre-hold state.
3838
*
3939
* ## Thread-safety
40-
* [decide] is always called from the single kDisco dispatcher thread — no
41-
* synchronisation is required inside an implementation.
40+
*
41+
* The kDisco simulation kernel runs on its own single thread, but [decide] is
42+
* invoked from **outside** the kernel — by the external drive-loop driver (SP0.10,
43+
* #732) on its own thread/coroutine, not the kDisco thread. Implementations must
44+
* therefore not rely on kDisco-thread exclusivity: keep them pure and stateless
45+
* ([DispatchObservation] already carries everything the policy needs — invariant 4
46+
* of the SP0.5 design spec, `docs/specs/2026-07-08-544-sp05-drive-loop-design.md`),
47+
* or synchronise any mutable state held across calls.
4248
*
4349
* @see RuleBasedDispatcher
4450
* @see DispatchObservation

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ class ShuntingLoop(
382382
*/
383383
private fun buildAdmissionObservation(): DispatchObservation =
384384
DispatchObservation(
385-
snapshot = perceptionPort.snapshot(),
385+
snapshot = perceptionPort.captureSnapshot(),
386386
unapprovedTrains =
387387
unapprowedTrains.map { train ->
388388
QueuedTrainObservation(train.name, train.timetableDestinationName)
@@ -403,7 +403,7 @@ class ShuntingLoop(
403403
*/
404404
private fun buildPathAdvancementObservation(): DispatchObservation =
405405
DispatchObservation(
406-
snapshot = perceptionPort.snapshot(),
406+
snapshot = perceptionPort.captureSnapshot(),
407407
unapprovedTrains = emptyList(),
408408
innerBlockInputs =
409409
innerTrackBlocks.flatMap { block ->

core/src/commonTest/kotlin/cz/vutbr/fit/interlockSim/ports/SimulationSnapshotTest.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,9 @@ class SimulationSnapshotTest {
163163
trainPositions = allTrainPositions(),
164164
timetables = allTrainTimetables()
165165
)
166+
167+
// Stub is not a caching port; both accessors return the same fixed snapshot.
168+
override fun captureSnapshot(): SimulationSnapshot = snapshot()
166169
}
167170

168171
@Test
@@ -207,6 +210,9 @@ class SimulationSnapshotTest {
207210
trainPositions = allTrainPositions(),
208211
timetables = allTrainTimetables()
209212
)
213+
214+
// Stub is not a caching port; both accessors return the same fixed snapshot.
215+
override fun captureSnapshot(): SimulationSnapshot = snapshot()
210216
}
211217
val snap = emptyPort.snapshot()
212218

0 commit comments

Comments
 (0)