Skip to content

Commit 6e68e25

Browse files
committed
Debug infrastructure: managed mode, hit-count dedup, and comprehensive tests
- Enable managed execution mode in CommandHelperPlugin so the debug session survives script completion on embedded (Minecraft) servers - Fix hit-count breakpoint deduplication: multiple AST nodes on the same source line no longer increment the hit counter more than once per visit. Uses column-based caching in ThreadDebugState to distinguish "same line, different node" from "new loop iteration, same first node" - Add evaluateBreakpointCondition() to DebugContext with per-thread cache-aware hit-count and condition evaluation - Add Breakpoint.getHitCount() getter - Add MSDebugServer managed mode support: setManagedExecution(), startedOnHostMainThread capture, resumeOnHostMainThread() for resuming on the server main thread, dynamic scripting mode flag in evaluate handler - Add 21 new DAP integration tests (39 total) covering: managed mode step-over, thread events, disconnect; variables/scopes; CArray expansion (indexed, associative, nested); evaluate expressions; exception breakpoints; conditional and hit-count breakpoints; step-in, step-out, step-in targets; disconnect resumes execution
1 parent fb7d440 commit 6e68e25

File tree

6 files changed

+726
-19
lines changed

6 files changed

+726
-19
lines changed

src/main/java/com/laytonsmith/commandhelper/CommandHelperPlugin.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ private void startDebugServer() {
729729
: null;
730730

731731
debugServer = new MSDebugServer();
732+
debugServer.setManagedExecution(true);
732733
try {
733734
GlobalEnv gEnv = new GlobalEnv(
734735
MethodScriptFileLocations.getDefault().getConfigDirectory(),

src/main/java/com/laytonsmith/core/environments/Breakpoint.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,13 @@ public int incrementHitCount() {
159159
return ++hitCount;
160160
}
161161

162+
/**
163+
* Returns the current hit count without incrementing.
164+
*/
165+
public int getHitCount() {
166+
return hitCount;
167+
}
168+
162169
@Override
163170
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
164171
public boolean equals(Object obj) {

src/main/java/com/laytonsmith/core/environments/DebugContext.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ public boolean shouldPause(Target source, int userCallDepth, Environment env) {
385385
Breakpoint bp = getBreakpoint(source.file(), source.line());
386386
if(bp != null) {
387387
if(bp.isConditional()) {
388-
shouldStop = evaluateBreakpointCondition(bp, env);
388+
shouldStop = evaluateBreakpointCondition(bp, source, state, env);
389389
} else {
390390
shouldStop = true;
391391
}
@@ -484,10 +484,22 @@ && sameSourceLine(source, state.getStepReferenceTarget())
484484
* Evaluates a conditional breakpoint's condition and hit count.
485485
* Returns true if the breakpoint should cause a pause.
486486
*/
487-
private boolean evaluateBreakpointCondition(Breakpoint bp, Environment env) {
488-
// Check hit count first (cheap)
487+
private boolean evaluateBreakpointCondition(Breakpoint bp, Target source,
488+
ThreadDebugState state, Environment env) {
489+
// Check hit count first (cheap). Deduplicate per source line visit:
490+
// multiple AST nodes on the same line should only increment the hit
491+
// count once. We use the column to distinguish "same visit, different
492+
// node" (col differs → cache hit) from "new visit, same first node"
493+
// (col matches → cache miss, re-evaluate).
489494
if(bp.hitCountThreshold() > 0) {
490-
if(bp.incrementHitCount() < bp.hitCountThreshold()) {
495+
if(!state.hasCachedBreakpointResult(source.file(), source.line(), source.col())) {
496+
int count = bp.incrementHitCount();
497+
boolean hitReached = count >= bp.hitCountThreshold();
498+
state.cacheBreakpointResult(source.file(), source.line(), source.col(), hitReached);
499+
if(!hitReached) {
500+
return false;
501+
}
502+
} else if(!state.getCachedBreakpointResult()) {
491503
return false;
492504
}
493505
}

src/main/java/com/laytonsmith/core/environments/ThreadDebugState.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.laytonsmith.core.constructs.Target;
44

5+
import java.io.File;
56
import java.util.concurrent.CountDownLatch;
67

78
/**
@@ -20,6 +21,16 @@ public class ThreadDebugState {
2021
private volatile boolean skippingResume = false;
2122
private volatile CountDownLatch pauseLatch;
2223

24+
// Breakpoint hit-count deduplication: when shouldPause checks a
25+
// hit-count breakpoint, we cache the file+line+col+result so that
26+
// subsequent AST nodes on the same source line reuse the cached result
27+
// (and don't re-increment the hit count). A "new visit" to the same
28+
// line is detected when the same first-node column fires again.
29+
private volatile File lastBpFile;
30+
private volatile int lastBpLine = -1;
31+
private volatile int lastBpCol = -1;
32+
private volatile boolean lastBpResult;
33+
2334
public DebugContext.StepMode getStepMode() {
2435
return stepMode;
2536
}
@@ -95,4 +106,44 @@ public void resume() {
95106
latch.countDown();
96107
}
97108
}
109+
110+
/**
111+
* Returns true if we already evaluated a hit-count breakpoint for this
112+
* file+line on this thread and can reuse the cached result. A cache hit
113+
* occurs when file+line match but col differs (a subsequent AST node on
114+
* the same source line). When file+line+col all match, it means the first
115+
* node is firing again (new loop iteration), so we return false to force
116+
* a fresh evaluation.
117+
*/
118+
public boolean hasCachedBreakpointResult(File file, int line, int col) {
119+
return lastBpLine == line && file != null && file.equals(lastBpFile)
120+
&& lastBpCol != col;
121+
}
122+
123+
/**
124+
* Returns the cached breakpoint evaluation result.
125+
*/
126+
public boolean getCachedBreakpointResult() {
127+
return lastBpResult;
128+
}
129+
130+
/**
131+
* Caches the breakpoint evaluation result for the given file+line+col.
132+
*/
133+
public void cacheBreakpointResult(File file, int line, int col, boolean result) {
134+
this.lastBpFile = file;
135+
this.lastBpLine = line;
136+
this.lastBpCol = col;
137+
this.lastBpResult = result;
138+
}
139+
140+
/**
141+
* Clears the breakpoint evaluation cache. Called when execution moves
142+
* to a different source line.
143+
*/
144+
public void clearBreakpointCache() {
145+
this.lastBpFile = null;
146+
this.lastBpLine = -1;
147+
this.lastBpCol = -1;
148+
}
98149
}

src/main/java/com/laytonsmith/tools/debugger/MSDebugServer.java

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@
9191
import org.eclipse.lsp4j.debug.ThreadEventArguments;
9292
import org.eclipse.lsp4j.debug.ThreadEventArgumentsReason;
9393

94+
import com.laytonsmith.abstraction.StaticLayer;
95+
9496
/**
9597
* A Debug Adapter Protocol (DAP) server for MethodScript. This server can be started
9698
* alongside any MethodScript execution mode (cmdline, eval, embedded) to enable
@@ -134,6 +136,7 @@ public class MSDebugServer implements IDebugProtocolServer {
134136
private boolean suspendOnStart = false;
135137
private boolean managedExecution = false;
136138
private volatile boolean scriptCompleted = false;
139+
private boolean startedOnHostMainThread = false;
137140
private PrintStream originalOut;
138141
private PrintStream originalErr;
139142
private DebugSecurity securityMode = DebugSecurity.KEYPAIR;
@@ -142,6 +145,16 @@ public class MSDebugServer implements IDebugProtocolServer {
142145
private final ConcurrentHashMap<Integer, Mixed> compoundVariableRefs = new ConcurrentHashMap<>();
143146
private int nextCompoundRef = COMPOUND_REF_START;
144147

148+
/**
149+
* Sets whether this server is in managed execution mode. In managed mode,
150+
* the server does not own the process lifecycle - the host (e.g. Minecraft)
151+
* manages it. This affects thread registration, completion events, and
152+
* how execution resumes after breakpoints.
153+
*/
154+
public void setManagedExecution(boolean managed) {
155+
this.managedExecution = managed;
156+
}
157+
145158
/**
146159
* Starts a DAP TCP listener on the given port with an explicit threading mode.
147160
*
@@ -162,7 +175,6 @@ public Environment startListening(int port, String bindAddress, Environment envi
162175
DebugSecurity security, File authorizedKeysFile) throws IOException {
163176
this.env = environment;
164177
this.suspendOnStart = suspend;
165-
this.managedExecution = true;
166178
this.securityMode = security;
167179

168180
if(security == DebugSecurity.KEYPAIR) {
@@ -328,6 +340,7 @@ public void awaitCompletion() throws InterruptedException {
328340
*/
329341
public void runScript(ParseTree tree, Environment env, List<com.laytonsmith.core.constructs.Variable> vars) {
330342
this.env = env;
343+
this.startedOnHostMainThread = managedExecution && StaticLayer.IsMainThread();
331344
wireUpDaemonManager(env);
332345
installOutputRedirect();
333346
spawnExecutionThread(() -> MethodScriptCompiler.execute(tree, env, null, null, vars));
@@ -940,6 +953,8 @@ public CompletableFuture<EvaluateResponse> evaluate(EvaluateArguments args) {
940953
Environment evalEnv = inspected.getEnvironment();
941954
GlobalEnv gEnv = evalEnv.getEnv(GlobalEnv.class);
942955
gEnv.SetFlag(GlobalEnv.FLAG_NO_CHECK_UNDEFINED, true);
956+
boolean oldDynamic = gEnv.GetDynamicScriptingMode();
957+
gEnv.SetDynamicScriptingMode(true);
943958
try {
944959
Set<Class<? extends Environment.EnvironmentImpl>> envClasses
945960
= evalEnv.getEnvClasses();
@@ -952,6 +967,7 @@ public CompletableFuture<EvaluateResponse> evaluate(EvaluateArguments args) {
952967
response.setResult(result.val());
953968
response.setType(result.typeof(evalEnv).val());
954969
} finally {
970+
gEnv.SetDynamicScriptingMode(oldDynamic);
955971
gEnv.ClearFlag(GlobalEnv.FLAG_NO_CHECK_UNDEFINED);
956972
}
957973
} catch(Exception e) {
@@ -994,16 +1010,42 @@ private void resumeExecution(int threadId) {
9941010
pausedStates.remove(threadId);
9951011
if(debugCtx.getThreadingMode() == DebugContext.ThreadingMode.SYNCHRONOUS
9961012
|| threadId != DebugContext.MAIN_THREAD_DAP_ID) {
997-
// Sync mode or background thread: thread is blocked on the latch release it
1013+
// Sync mode or background thread: thread is blocked on the latch - release it
9981014
asyncSnapshots.remove(threadId);
9991015
debugCtx.resume(threadId);
1016+
} else if(startedOnHostMainThread) {
1017+
// Async mode, managed execution started on host main thread:
1018+
// resume on that thread so the host's main-thread contract is honored
1019+
Script.DebugSnapshot snapshot = asyncSnapshots.remove(threadId);
1020+
resumeOnHostMainThread(snapshot);
10001021
} else {
10011022
// Async mode, main thread: spawn a new thread to resume from snapshot
10021023
Script.DebugSnapshot snapshot = asyncSnapshots.remove(threadId);
10031024
spawnExecutionThread(() -> Script.resumeEval(snapshot));
10041025
}
10051026
}
10061027

1028+
/**
1029+
* Resumes script execution on the host's main thread. Used in managed/embedded
1030+
* mode when the script was originally running on the host's main thread (e.g.
1031+
* Minecraft server thread). The host's convertor schedules the work via
1032+
* {@link com.laytonsmith.abstraction.AbstractConvertor#runOnMainThreadLater}.
1033+
*/
1034+
private void resumeOnHostMainThread(Script.DebugSnapshot snapshot) {
1035+
StaticLayer.GetConvertor().runOnMainThreadLater(null, () -> {
1036+
try {
1037+
Mixed result = Script.resumeEval(snapshot);
1038+
if(!Script.isDebuggerPaused(result)) {
1039+
mainThreadFinished();
1040+
executionCompleted();
1041+
}
1042+
} catch(Exception e) {
1043+
sendOutput("error", "Script error: " + e.getMessage());
1044+
executionCompleted();
1045+
}
1046+
});
1047+
}
1048+
10071049
/**
10081050
* Registers the DebugContext with the environment's DaemonManager so that
10091051
* background threads (e.g. x_new_thread) are automatically tracked and

0 commit comments

Comments
 (0)