From cc98074739d5e29d6c1449699f8ef3e8c1e213c6 Mon Sep 17 00:00:00 2001 From: chsami <23292806+chsami@users.noreply.github.com> Date: Sun, 11 Oct 2026 08:32:20 +0200 Subject: [PATCH 1/2] fix(statemachine): re-initialise StateMachineScript on restart A stopped script kept initialized=true, so the next start skipped initialize(): it never re-registered in the registry and resumed from its previous state with stale counters and trace. Stopping now resets the machine and only unregisters the entry it owns. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV --- .../statemachine/StateMachineScript.java | 12 ++- .../StateMachineScriptRestartTest.java | 87 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java index 1fc93089c49..09fe8d3adc1 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java @@ -73,7 +73,7 @@ public static Map> getRegistry() { private long loopCount; private long transitionCount; private List> transitions; - private boolean initialized; + private volatile boolean initialized; /** Bounded ring buffer of recent transitions for the trace log. */ private static final int MAX_TRACE_SIZE = 50; @@ -215,6 +215,12 @@ public final boolean step() { private void initialize() { currentState = initialState(); + previousState = null; + lastTransitionReason = null; + lastTransitionAt = null; + loopCount = 0; + transitionCount = 0; + traceBuffer.clear(); stateEnteredAt = Instant.now(); transitions = Collections.unmodifiableList(defineTransitions()); initialized = true; @@ -287,7 +293,9 @@ public void forceState(S newState, String reason) { @Override public void shutdown() { - REGISTRY.remove(getScriptName()); + initialized = false; + REGISTRY.remove(getScriptName(), this); + snapshot.set(null); super.shutdown(); } diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java new file mode 100644 index 00000000000..b2e17f322e4 --- /dev/null +++ b/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java @@ -0,0 +1,87 @@ +package net.runelite.client.plugins.microbot.statemachine; + +import java.util.ArrayList; +import java.util.List; +import net.runelite.client.plugins.microbot.Microbot; +import org.junit.Test; +import org.mockito.MockedStatic; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +public class StateMachineScriptRestartTest { + private static final String NAME = Machine.class.getSimpleName(); + + @Test public void restartReRegistersAndBeginsFromInitialState() { + Machine machine = new Machine(); + try (MockedStatic ignored = mockStatic(Microbot.class)) { + machine.advance = true; + assertTrue(machine.step()); + assertEquals(Machine.State.WORKING, machine.getCurrentState()); + assertSame(machine, StateMachineScript.getRegistry().get(NAME)); + + machine.shutdown(); + assertFalse(StateMachineScript.getRegistry().containsKey(NAME)); + assertNull(machine.getSnapshot()); + + machine.advance = false; + assertTrue(machine.step()); + assertEquals(Machine.State.IDLE, machine.getCurrentState()); + assertSame(machine, StateMachineScript.getRegistry().get(NAME)); + StateSnapshot snapshot = machine.getSnapshot(); + assertEquals(Machine.State.IDLE, snapshot.currentState()); + assertNull(snapshot.previousState()); + assertEquals(1, snapshot.loopCount()); + assertEquals(0, snapshot.transitionCount()); + assertTrue(snapshot.recentTrace().isEmpty()); + assertEquals(List.of(Machine.State.WORKING, Machine.State.IDLE), machine.visited); + + machine.shutdown(); + assertFalse(StateMachineScript.getRegistry().containsKey(NAME)); + } finally { + machine.shutdown(); + machine.dispose(); + } + } + + @Test public void staleInstanceStopDoesNotUnregisterItsReplacement() { + Machine first = new Machine(); + Machine second = new Machine(); + try (MockedStatic ignored = mockStatic(Microbot.class)) { + first.step(); + second.step(); + first.shutdown(); + assertSame(second, StateMachineScript.getRegistry().get(NAME)); + second.shutdown(); + assertFalse(StateMachineScript.getRegistry().containsKey(NAME)); + } finally { + first.shutdown(); + second.shutdown(); + first.dispose(); + second.dispose(); + } + } + + private static final class Machine extends StateMachineScript { + enum State { IDLE, WORKING } + + boolean advance; + final List visited = new ArrayList<>(); + + @Override protected State initialState() { + return State.IDLE; + } + + @Override protected List> defineTransitions() { + return List.of(Transition.from(State.IDLE).when(() -> advance, "advance") + .because("advance").goTo(State.WORKING)); + } + + @Override protected void onState(State state) { + visited.add(state); + } + + void dispose() { + scheduledExecutorService.shutdownNow(); + } + } +} From a193266676604e66ec381deecaea723d9b6db8a0 Mon Sep 17 00:00:00 2001 From: chsami <23292806+chsami@users.noreply.github.com> Date: Sun, 11 Oct 2026 08:39:38 +0200 Subject: [PATCH 2/2] fix(statemachine): keep an in-flight step from re-registering after stop Stop cancels the loop before resetting, initialisation skips an interrupted (cancelled) step, and snapshot publication and reset share a short lifecycle lock so a step finishing after stop cannot leave a registry entry or stale snapshot behind. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV --- .../statemachine/StateMachineScript.java | 52 ++++++++++++------- .../StateMachineScriptRestartTest.java | 41 +++++++++++++++ 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java index 09fe8d3adc1..c9b9ca5b2ec 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScript.java @@ -74,6 +74,7 @@ public static Map> getRegistry() { private long transitionCount; private List> transitions; private volatile boolean initialized; + private final Object lifecycleLock = new Object(); /** Bounded ring buffer of recent transitions for the trace log. */ private static final int MAX_TRACE_SIZE = 50; @@ -136,8 +137,8 @@ protected S onError(S state, Exception e) { * @return false if the base guard paused execution, true otherwise */ public final boolean step() { - if (!initialized) { - initialize(); + if (!initialized && !initialize()) { + return false; } // Check the base Script guard (heartbeat, blocking events, pause, etc.) @@ -213,19 +214,25 @@ public final boolean step() { return true; } - private void initialize() { - currentState = initialState(); - previousState = null; - lastTransitionReason = null; - lastTransitionAt = null; - loopCount = 0; - transitionCount = 0; - traceBuffer.clear(); - stateEnteredAt = Instant.now(); - transitions = Collections.unmodifiableList(defineTransitions()); - initialized = true; - REGISTRY.put(getScriptName(), this); + private boolean initialize() { + synchronized (lifecycleLock) { + if (Thread.currentThread().isInterrupted()) { + return false; + } + currentState = initialState(); + previousState = null; + lastTransitionReason = null; + lastTransitionAt = null; + loopCount = 0; + transitionCount = 0; + traceBuffer.clear(); + stateEnteredAt = Instant.now(); + transitions = Collections.unmodifiableList(defineTransitions()); + initialized = true; + REGISTRY.put(getScriptName(), this); + } log.info("[{}] State machine initialized in state {}", getScriptName(), currentState); + return true; } /** @@ -251,13 +258,18 @@ private void publishSnapshot(boolean allGuardsFalse) { } } - snapshot.set(new StateSnapshot<>( + StateSnapshot next = new StateSnapshot<>( currentState, previousState, lastTransitionReason, lastTransitionAt, stateEnteredAt, loopCount, transitionCount, pending, new ArrayList<>(traceBuffer) - )); + ); + synchronized (lifecycleLock) { + if (initialized) { + snapshot.set(next); + } + } } /** @@ -293,10 +305,12 @@ public void forceState(S newState, String reason) { @Override public void shutdown() { - initialized = false; - REGISTRY.remove(getScriptName(), this); - snapshot.set(null); super.shutdown(); + synchronized (lifecycleLock) { + initialized = false; + REGISTRY.remove(getScriptName(), this); + snapshot.set(null); + } } private String getScriptName() { diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java index b2e17f322e4..47c7bf506b7 100644 --- a/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java +++ b/runelite-client/src/test/java/net/runelite/client/plugins/microbot/statemachine/StateMachineScriptRestartTest.java @@ -61,12 +61,49 @@ public class StateMachineScriptRestartTest { } } + @Test public void cancelledStepAfterStopDoesNotReRegister() { + Machine machine = new Machine(); + try (MockedStatic ignored = mockStatic(Microbot.class)) { + machine.step(); + machine.shutdown(); + Thread.currentThread().interrupt(); + try { + assertFalse(machine.step()); + } finally { + Thread.interrupted(); + } + assertFalse(StateMachineScript.getRegistry().containsKey(NAME)); + assertNull(machine.getSnapshot()); + } finally { + machine.shutdown(); + machine.dispose(); + } + } + + @Test public void stopDuringStateActionDoesNotRepublishSnapshot() { + Machine machine = new Machine(); + try (MockedStatic ignored = mockStatic(Microbot.class)) { + machine.stopInAction = true; + machine.step(); + assertFalse(StateMachineScript.getRegistry().containsKey(NAME)); + assertNull(machine.getSnapshot()); + } finally { + machine.shutdown(); + machine.dispose(); + } + } + private static final class Machine extends StateMachineScript { enum State { IDLE, WORKING } boolean advance; + boolean stopInAction; final List visited = new ArrayList<>(); + @Override protected State onError(State state, Exception e) { + return State.WORKING; + } + @Override protected State initialState() { return State.IDLE; } @@ -78,6 +115,10 @@ enum State { IDLE, WORKING } @Override protected void onState(State state) { visited.add(state); + if (stopInAction) { + shutdown(); + throw new IllegalStateException("stopped"); + } } void dispose() {