diff --git a/README.md b/README.md
index 2652cf8..3124a22 100644
--- a/README.md
+++ b/README.md
@@ -94,7 +94,24 @@ serializations of the same instrumentation underneath.
script calls into, for free, wherever the platform already wraps the JVM (e.g. debugging
an inline Maven script).
-Full details, screenshots and known limitations: [`plugin/README.md`](plugin/README.md).
+Full details and known limitations: [`plugin/README.md`](plugin/README.md).
+
+### Screenshots
+
+
+
+Click a thumbnail to open it full-size:
+
+
+
+
+
+
+
+
+
+
+
## Editors: VS Code, Neovim, Eclipse
diff --git a/agent/CHANGELOG.md b/agent/CHANGELOG.md
index 747bd7a..b120526 100644
--- a/agent/CHANGELOG.md
+++ b/agent/CHANGELOG.md
@@ -8,3 +8,12 @@ VS Code, Neovim and Eclipse DAP transports. The copy of this jar bundled inside
IDEA plugin has its own release cycle -- see [`../plugin/CHANGELOG.md`](../plugin/CHANGELOG.md).
## [Unreleased]
+
+## [0.2.0] - 2026-08-04
+
+### Added
+
+- `SCOPES` now reports a `Block`/`Closure` level for each `for`/`if` body or captured
+ closure namespace between a frame's `Locals` and `Global`, each carrying only its own
+ directly-declared variables, instead of `Locals` flattening the whole parent chain into
+ one group. See [`docs/PROTOCOL.md`](../docs/PROTOCOL.md#0x11-scopes--answers-0x04).
diff --git a/agent/README.md b/agent/README.md
index 8ea225d..874de37 100644
--- a/agent/README.md
+++ b/agent/README.md
@@ -24,7 +24,7 @@ nested jar.
The goal is debugging BeanShell inside **third-party libraries that already bundle
bsh** — Maven plugins such as maven-enforcer being the motivating case. The code is
-fixed; only runtime behaviour can be changed. That rules out patching BeanShell and
+fixed; only runtime behavior can be changed. That rules out patching BeanShell and
makes an agent the only vehicle.
**JDWP is not usable, and the reason is a language property rather than an
@@ -113,14 +113,14 @@ the system loader cannot define a second copy.
are the init list, the condition, the update list *and* the body, all reporting
the `for` line. The child layouts were read off real parse trees:
- | node | children | statement position |
- |---|---|---|
- | `BSHIfStatement` | `[cond, then, else?]` | index ≥ 1 |
- | `BSHWhileStatement` (`while`) | `[cond, body]` | last |
- | `BSHWhileStatement` (`do`) | `[body, cond]` | **first** |
- | `BSHForStatement` | `[init?, cond?, update?, body]` | last |
- | `BSHEnhancedForStatement` | `[type?, iterable, body]` | last |
- | `BSHSwitchStatement` | `[expr, label, stmt, …]` | index ≥ 1 |
+ | node | children | statement position |
+ |-------------------------------|---------------------------------|--------------------|
+ | `BSHIfStatement` | `[cond, then, else?]` | index ≥ 1 |
+ | `BSHWhileStatement` (`while`) | `[cond, body]` | last |
+ | `BSHWhileStatement` (`do`) | `[body, cond]` | **first** |
+ | `BSHForStatement` | `[init?, cond?, update?, body]` | last |
+ | `BSHEnhancedForStatement` | `[type?, iterable, body]` | last |
+ | `BSHSwitchStatement` | `[expr, label, stmt, …]` | index ≥ 1 |
`do` and `while` are the *same node type* (`DoStatement() #WhileStatement`) with
opposite child order, separated by the package-private `isDoStatement` field.
@@ -138,9 +138,9 @@ the system loader cannot define a second copy.
### Not bit-transparent
-Behaviour is unchanged — every fixture produces identical output with and without
+Behavior is unchanged — every fixture produces identical output with and without
the agent — but **identity hash codes shift** deterministically (`Point@279f2327`
-becomes `Point@30f39991` and stays there), because initialising the hook on the
+becomes `Point@30f39991` and stays there), because initializing the hook on the
interpreter thread advances that thread's identity-hash sequence. Nothing correct
depends on those values, but a script printing a default `toString()` shows
different digits.
@@ -190,7 +190,7 @@ valid until the next resume and the table is dropped there, so the IDE can never
hold a reference into a script that has moved on — no stale-object problem to
solve, no cleanup protocol to get wrong. That is
[DAP's `variablesReference`](https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable)
-in a smaller encoding: adopting DAP later changes the serialisation, not the design.
+in a smaller encoding: adopting DAP later changes the serialization, not the design.
Requests are served **on the thread they concern**, from inside the same loop where it
waits for `RESUME`. Not a shortcut: that thread is parked there anyway, it owns the
@@ -218,9 +218,9 @@ does, so a filter is not optional: BeanShell's own commands (`print`, `pwd`, …
`.bsh` files on the classpath, and without a filter the session stops inside
`print.bsh` on every `print()` call. Two properties, ORed:
-| property | match | for |
-|---|---|---|
-| `bsh.debug.sources` | comma-separated, `endsWith` | a script that has a file name |
+| property | match | for |
+|--------------------------|------------------------------------------------|--------------------------------------|
+| `bsh.debug.sources` | comma-separated, `endsWith` | a script that has a file name |
| `bsh.debug.sources.file` | a file of prefixes, one per line, `startsWith` | a script handed over as a **string** |
The second exists because a string has no file name. BeanShell invents one:
@@ -300,7 +300,7 @@ correct in the CLI and does nothing in a library.
./gradlew :agent:samples:runHostWithAgent # the same, under the agent
```
-The two must agree, which is what pins down "behaviour unchanged". The README
+The two must agree, which is what pins down "behavior unchanged". The README
there lists the three differences that are legitimate.
The transport itself can be exercised without the IDE, and the instrumentation
diff --git a/agent/checks/02-maven-plugin-realm.sh b/agent/checks/02-maven-plugin-realm.sh
index 887984f..fd43bce 100755
--- a/agent/checks/02-maven-plugin-realm.sh
+++ b/agent/checks/02-maven-plugin-realm.sh
@@ -34,7 +34,7 @@ MAVEN_OPTS="-javaagent:$AGENT_JAR -Dbsh.debug.trace=1" \
mvn -o -q -f "$POM" validate > "$CHECK_TMP/bh.txt" 2>&1
grep 'bsh-agent' "$CHECK_TMP/bh.txt" > "$CHECK_TMP/bh-agent.txt" || true
-assert_contains "$CHECK_TMP/bh-agent.txt" 'src=inline evaluation of: ``prefix = project.getArtifactId();' \
+assert_contains "$CHECK_TMP/bh-agent.txt" 'src=inline evaluation of: ``name = project.artifactId + ":" + project.version;' \
"build-helper: the inline is instrumented inside the plugin realm" "$CHECK_TMP/bh.txt"
assert_contains "$CHECK_TMP/bh-agent.txt" 'line=1 src=inline evaluation of' \
"build-helper: lines are snippet-relative (first statement is line 1)" "$CHECK_TMP/bh.txt"
diff --git a/agent/checks/03-scopes-and-introspection.sh b/agent/checks/03-scopes-and-introspection.sh
index 7a7b1fa..2adef6f 100755
--- a/agent/checks/03-scopes-and-introspection.sh
+++ b/agent/checks/03-scopes-and-introspection.sh
@@ -5,11 +5,12 @@
# Drives the real transport -- mock-ide.py is the IDE end -- so this covers the socket conversation
# as well as the values, which no unit test on either side does alone.
#
-# The two things asserted here are easy to regress invisibly. A bsh.This must expand to the
+# The three things asserted here are easy to regress invisibly. A bsh.This must expand to the
# *namespace* it stands for rather than to its Java fields (that is what makes a closure's captured
-# scope, a scripted instance's _bshThis... field, and a This handed back to Java all readable), and
-# Global must appear when stopped inside a method, since that is where a script's top-level state
-# would otherwise become invisible.
+# scope, a scripted instance's _bshThis... field, and a This handed back to Java all readable), Global
+# must appear when stopped inside a method, since that is where a script's top-level state would
+# otherwise become invisible, and a `for` loop's own namespace must appear as its own level rather
+# than being lost inside Locals' or absorbed into Global.
source "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
@@ -79,4 +80,46 @@ assert_contains "$CHECK_TMP/ide.txt" 'count = 0 (int)' \
assert_not_contains "$CHECK_TMP/ide.txt" 'declaringInterpreter' \
"expanding a This does not leak bsh.XThis's own Java fields"
+# --- a `for` loop's own namespace is its own scope level ------------------------------------------
+#
+# BSHForStatement wraps the loop in a BlockNameSpace of its own (holding the init variable) and the
+# body runs in a second, subordinate BlockNameSpace -- two levels below the script's own Locals used
+# to be flattened into one, hiding a typed loop variable declared in the `for`'s own init behind
+# whichever scope's ancestor-walk happened to reach it first.
+
+cat > "$CHECK_TMP/forloop.bsh" <<'EOF'
+total = 0;
+for (int i = 1; i <= 3; i++) {
+ total += i;
+ print("step " + i);
+}
+EOF
+
+PORT2=$((20000 + RANDOM % 20000))
+python3 "$REPO_ROOT/plugin/tools/mock-ide.py" "$PORT2" \
+ --breakpoints forloop.bsh:3 --expand > "$CHECK_TMP/for-ide.txt" 2>&1 &
+FOR_IDE_PID=$!
+
+for _ in $(seq 50); do
+ grep -q 'listening on' "$CHECK_TMP/for-ide.txt" 2>/dev/null && break
+ sleep 0.1
+done
+
+"$JAVA" -javaagent:"$AGENT_JAR" -Dbsh.debug.port="$PORT2" -Dbsh.debug.sources=forloop.bsh \
+ -cp "$BSH_CLASSPATH" bsh.Interpreter "$CHECK_TMP/forloop.bsh" \
+ > "$CHECK_TMP/for-script.txt" 2>&1
+wait "$FOR_IDE_PID" 2>/dev/null || true
+
+assert_contains "$CHECK_TMP/for-ide.txt" 'Block:' \
+ "the for-loop's own namespace is offered as a level of its own"
+assert_contains "$CHECK_TMP/for-ide.txt" 'i = 1 (int)' \
+ "the loop variable is visible, in the Block level it was actually declared in" "$CHECK_TMP/for-ide.txt"
+
+# Global's own slice of the report, isolated so the assertion below cannot pass just because "i"
+# legitimately appears a few lines up, under Block.
+sed -n '/ Global:/,/^\[mock-ide\] STOPPED\|^\[mock-ide\] agent disconnected/p' "$CHECK_TMP/for-ide.txt" \
+ > "$CHECK_TMP/for-global-only.txt"
+assert_not_contains "$CHECK_TMP/for-global-only.txt" 'i = 1 (int)' \
+ "the loop variable is not repeated in Global -- each level reports only its own" "$CHECK_TMP/for-ide.txt"
+
finish
diff --git a/agent/checks/04-behaviour-unchanged.sh b/agent/checks/04-behavior-unchanged.sh
similarity index 81%
rename from agent/checks/04-behaviour-unchanged.sh
rename to agent/checks/04-behavior-unchanged.sh
index eec5cb9..a65cd32 100755
--- a/agent/checks/04-behaviour-unchanged.sh
+++ b/agent/checks/04-behavior-unchanged.sh
@@ -6,17 +6,17 @@
# comparison has to allow exactly three differences, all documented in agent/samples/README.md, and
# allowing them by pattern rather than by eye is the point of automating it:
#
-# * identity hash codes shift (Point@279f2327 -> Point@30f39991), because initialising the hook on
+# * identity hash codes shift (Point@279f2327 -> Point@30f39991), because initializing the hook on
# the interpreter thread advances that thread's identity-hash sequence;
# * the interleaving of the two threads in scenario 5 is not deterministic in either run;
# * bsh.NameSpace@... addresses, for the same reason as the first.
#
-# Anything else differing means the agent changed behaviour, which is a bug however useful the
+# Anything else differing means the agent changed behavior, which is a bug however useful the
# debugger is.
source "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
-banner "04 — behaviour is unchanged with the agent attached"
+banner "04 — behavior is unchanged with the agent attached"
if ! "$GRADLEW" -q -p "$REPO_ROOT" :agent:samples:runHost > "$CHECK_TMP/plain.txt" 2>"$CHECK_TMP/plain.err"; then
fail "the uninstrumented fixtures run" "$(tail -3 "$CHECK_TMP/plain.err")"
@@ -31,28 +31,28 @@ fi
pass "both runs completed"
-# Normalise the three legitimate differences away, then require equality. Sorting the thread-5 lines
+# Normalize the three legitimate differences away, then require equality. Sorting the thread-5 lines
# is what makes the interleaving irrelevant without hiding a missing line: a dropped or extra line
# still changes the sorted text.
-normalise() {
+normalize() {
sed -E \
-e 's/@[0-9a-f]{6,}/@HASH/g' \
-e 's/bsh\.NameSpace: [^ ]+ \(bsh\.NameSpace@HASH\)/bsh.NameSpace@HASH/g' \
"$1" | LC_ALL=C sort
}
-normalise "$CHECK_TMP/plain.txt" > "$CHECK_TMP/plain.norm"
-normalise "$CHECK_TMP/agent.txt" > "$CHECK_TMP/agent.norm"
+normalize "$CHECK_TMP/plain.txt" > "$CHECK_TMP/plain.norm"
+normalize "$CHECK_TMP/agent.txt" > "$CHECK_TMP/agent.norm"
if diff -q "$CHECK_TMP/plain.norm" "$CHECK_TMP/agent.norm" >/dev/null; then
- pass "output is identical once identity hashes and thread interleaving are normalised"
+ pass "output is identical once identity hashes and thread interleaving are normalized"
else
fail "output differs beyond the three documented differences" \
"$(diff "$CHECK_TMP/plain.norm" "$CHECK_TMP/agent.norm" | head -20)"
fi
# A weaker but independent assertion: the same number of lines, which catches a fixture that silently
-# stopped early under the agent even if normalisation were too generous.
+# stopped early under the agent even if normalization were too generous.
plain_lines=$(wc -l < "$CHECK_TMP/plain.txt")
agent_lines=$(wc -l < "$CHECK_TMP/agent.txt")
assert_equals "$plain_lines" "$agent_lines" "both runs produced the same number of output lines"
diff --git a/agent/checks/07-dap-transport.sh b/agent/checks/07-dap-transport.sh
index 2338de5..00ad08a 100755
--- a/agent/checks/07-dap-transport.sh
+++ b/agent/checks/07-dap-transport.sh
@@ -7,7 +7,7 @@
# keeps the native protocol, which check 03 and 05 cover.
#
# What this asserts is that a real DAP conversation works end to end -- the handshake in the right
-# order, breakpoints honoured, a stack with more than one frame, both scopes, and an expression
+# order, breakpoints honored, a stack with more than one frame, both scopes, and an expression
# evaluated in the stopped frame. Between them those cover every translation the DAP channel makes,
# and each one has a specific way of going wrong:
#
diff --git a/agent/checks/README.md b/agent/checks/README.md
index 67f4877..92c2c6e 100644
--- a/agent/checks/README.md
+++ b/agent/checks/README.md
@@ -8,22 +8,22 @@ End-to-end checks for the debug agent, as standalone bash scripts.
```
Each script builds what it needs, prints one `PASS`/`FAIL` line per assertion, and exits
-non-zero if any failed. `JAVA_HOME` is honoured; the agent targets Java 8, so anything
+non-zero if any failed. `JAVA_HOME` is honored; the agent targets Java 8, so anything
8+ works for the debugged JVM.
## Why these are not Gradle tests
Each one needs something a JVM test cannot arrange from inside itself:
-| check | needs |
-|---|---|
-| `01-inline-eval-source-name.sh` | a JVM launched with `-javaagent`, so the interpreter is instrumented before it loads |
-| `02-maven-plugin-realm.sh` | a **real `mvn` process**, because the thing under test is a Maven plugin's own classloader |
-| `03-scopes-and-introspection.sh` | two processes and a socket between them — the actual wire protocol |
-| `04-behaviour-unchanged.sh` | the same fixtures run twice, in separate JVMs, one with the agent |
-| `05-two-script-threads.sh` | two real threads, suspended at the same time over one socket |
-| `06-suspend-all.sh` | a thread stopping at a line that has no breakpoint on it |
-| `07-dap-transport.sh` | a real DAP conversation, handshake included, over a socket |
+| check | needs |
+|----------------------------------|--------------------------------------------------------------------------------------------|
+| `01-inline-eval-source-name.sh` | a JVM launched with `-javaagent`, so the interpreter is instrumented before it loads |
+| `02-maven-plugin-realm.sh` | a **real `mvn` process**, because the thing under test is a Maven plugin's own classloader |
+| `03-scopes-and-introspection.sh` | two processes and a socket between them — the actual wire protocol |
+| `04-behavior-unchanged.sh` | the same fixtures run twice, in separate JVMs, one with the agent |
+| `05-two-script-threads.sh` | two real threads, suspended at the same time over one socket |
+| `06-suspend-all.sh` | a thread stopping at a line that has no breakpoint on it |
+| `07-dap-transport.sh` | a real DAP conversation, handshake included, over a socket |
They are also the checks you want *while* changing the agent, one at a time, with the
output in front of you — which is the other reason they are scripts.
@@ -64,14 +64,14 @@ only observation that distinguishes the round-up from ordinary per-thread suspen
thread its own code so the breakpoint can belong to one of them alone.
**`07` — the DAP transport.** That the same debugger works over DAP: the handshake in the right
-order (a client that never sees `initialized` configures nothing), breakpoints honoured, a stack
+order (a client that never sees `initialized` configures nothing), breakpoints honored, a stack
with depth, both scopes, and an expression evaluated in the stopped frame. Between them those cover
every translation `DapChannel` performs. `dap-client.py` beside it is a standalone DAP client for
driving a session by hand, the way `mock-ide.py` is for the native protocol.
-**`04` — behaviour is unchanged.** The agent must not change what a script does. Allows
+**`04` — behavior is unchanged.** The agent must not change what a script does. Allows
exactly the three differences documented in [`../samples/README.md`](../samples/README.md)
-(identity hashes, `NameSpace` addresses, thread interleaving) by normalising them, and
+(identity hashes, `NameSpace` addresses, thread interleaving) by normalizing them, and
requires equality otherwise.
## Adding one
diff --git a/agent/checks/dap-client.py b/agent/checks/dap-client.py
index d847479..aa92e3e 100755
--- a/agent/checks/dap-client.py
+++ b/agent/checks/dap-client.py
@@ -23,7 +23,7 @@
[--evaluate 'expr'] [--set name=value] [--json]
Prints one line per protocol step, which is what the checks assert on. `--json` additionally
-dumps every message, for when the disagreement is about the wire rather than the behaviour.
+dumps every message, for when the disagreement is about the wire rather than the behavior.
"""
import argparse
import json
@@ -171,7 +171,7 @@ def main():
conn.request("configurationDone")
print("[dap] configurationDone", flush=True)
- for index in range(args.stops):
+ for _ in range(args.stops):
stopped = conn.wait_event("stopped", timeout=30)
if stopped is None:
print("[dap] no further stops", flush=True)
diff --git a/agent/checks/run-all.sh b/agent/checks/run-all.sh
index 807a126..c515006 100755
--- a/agent/checks/run-all.sh
+++ b/agent/checks/run-all.sh
@@ -7,10 +7,11 @@
# diagnosis.
set -uo pipefail
-cd "$(dirname "${BASH_SOURCE[0]}")"
+cd "$(dirname "${BASH_SOURCE[0]}")" || exit 1
+files=( [0-9][0-9]-*.sh )
failed=()
-for check in [0-9][0-9]-*.sh; do
+for check in "${files[@]}"; do
if ! bash "$check"; then
failed+=("$check")
fi
diff --git a/agent/hook/build.gradle.kts b/agent/hook/build.gradle.kts
index 3f5682f..08a7ac3 100644
--- a/agent/hook/build.gradle.kts
+++ b/agent/hook/build.gradle.kts
@@ -14,8 +14,19 @@ plugins {
group = "cz.loplex.bsh"
description = "Bootstrap-loaded hook invoked by the instrumented BeanShell interpreter"
+// See agent/instrument/build.gradle.kts for why the toolchain forks an actual JDK 8 javac
+// (no `--release` flag: that's a JDK 9+ flag javac 8 doesn't understand -- the toolchain's
+// own source/target default already is 8), and why sourceCompatibility is declared here too:
+// IntelliJ's Gradle sync doesn't evaluate `configureEach {}` blocks.
+java {
+ toolchain {
+ languageVersion.set(JavaLanguageVersion.of(8))
+ }
+ sourceCompatibility = JavaVersion.VERSION_1_8
+ targetCompatibility = JavaVersion.VERSION_1_8
+}
+
tasks.withType().configureEach {
- options.release.set(8)
options.encoding = "UTF-8"
}
diff --git a/agent/hook/src/main/java/cz/loplex/bsh/hook/BshHook.java b/agent/hook/src/main/java/cz/loplex/bsh/hook/BshHook.java
index 89e1ab7..dfe9987 100644
--- a/agent/hook/src/main/java/cz/loplex/bsh/hook/BshHook.java
+++ b/agent/hook/src/main/java/cz/loplex/bsh/hook/BshHook.java
@@ -1,16 +1,15 @@
package cz.loplex.bsh.hook;
-import java.io.BufferedOutputStream;
import java.io.BufferedReader;
-import java.io.DataInputStream;
import java.io.DataOutputStream;
-import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
-import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -33,9 +32,10 @@
* reflection throughout. Configuration arrives through system properties for the same reason.
*
*
The wire format is protocol 3, specified in {@code docs/PROTOCOL.md}. Per reported statement
- * the hook writes {@link #EVT_STOPPED} with the call stack and then blocks that thread,
- * answering whatever the IDE asks about its suspended frames — scopes, variables, an expression to
- * evaluate, a value to change — until it is resumed. Other script threads keep running and may
+ * the hook writes via {@link DebugChannel#sendStopped} with the call stack and then blocks
+ * that thread, answering whatever the IDE asks about its suspended frames — scopes,
+ * variables, an expression to evaluate, a value to change — until it is resumed. Other script
+ * threads keep running and may
* report alongside; see {@link ThreadState} and {@link #readerLoop} for how that works, and
* {@link #report} for why only the reporting thread suspends. Failure handling is described on
* {@link #onEval}.
@@ -116,7 +116,7 @@ public final class BshHook {
/**
* Upper bound on the children of one value. Lazy expansion removes the cost of unopened
* objects, not the cost of an opened one, and a million-element list would still stall the
- * interpreter thread while it serialised.
+ * interpreter thread while it serialized.
*/
private static final int MAX_CHILDREN = 1000;
@@ -134,7 +134,7 @@ public final class BshHook {
* own right: {@code BSHBlock} would add a stop on the {@code {} of every braced body, and
* {@code BSHSwitchLabel} is a {@code case} label, not a statement.
*/
- private static final Set NEVER_REPORTED = new HashSet(Arrays.asList(
+ private static final Set NEVER_REPORTED = new HashSet<>(Arrays.asList(
"BSHBlock",
"BSHSwitchLabel"));
@@ -159,7 +159,7 @@ public final class BshHook {
* indices.
*
* This is per-node-type knowledge, which the transformer deliberately avoids — but here it
- * degrades gracefully. An unrecognised container simply reports none of its direct children,
+ * degrades gracefully. An unrecognized container simply reports none of its direct children,
* exactly as before this rule existed, so a future BeanShell that renames or reshapes a node
* loses brace-less-body coverage instead of misbehaving.
*/
@@ -170,54 +170,6 @@ public final class BshHook {
private static final String SWITCH_STATEMENT = "BSHSwitchStatement";
private static final String BLOCK = "BSHBlock";
- /*
- * Commands the IDE may send on the return channel. RESUME releases a reported statement; any
- * number of the others may precede it.
- *
- * Until the IDE sends SET_BREAKPOINTS at least once every statement is reported, because an IDE
- * that configures nothing must not go blind. Once it does, the agent falls silent while running
- * and speaks up only at a breakpoint, which removes the round-trip per statement that made a
- * plain loop crawl.
- */
- private static final int CMD_RESUME = 0x01;
- private static final int CMD_SET_BREAKPOINTS = 0x02;
- private static final int CMD_SET_RUN_MODE = 0x03;
-
- /**
- * Turns "report everything, on every thread" on and off, so the IDE can round up the other
- * threads when a breakpoint says Suspend: All. Global rather than per thread — that is what it
- * means. See {@link #catchAll}.
- */
- private static final int CMD_SET_CATCH_ALL = 0x08;
-
- /**
- * Requests the IDE may issue while a statement is suspended, each answered with the matching
- * {@code EVT_*} reply before the loop goes back to waiting.
- *
- *
They are served on the interpreter thread, from inside the same command loop that waits
- * for {@link #CMD_RESUME}. That is not a shortcut: the thread is parked there anyway, it is
- * the thread that owns the BeanShell state being inspected, and answering anywhere else would
- * need a lock BeanShell does not offer.
- */
- private static final int CMD_SCOPES = 0x04;
- private static final int CMD_VARIABLES = 0x05;
- private static final int CMD_EVALUATE = 0x06;
- private static final int CMD_SET_VARIABLE = 0x07;
-
- /*
- * The agent-to-IDE direction is opcode-tagged as of protocol 2. It used to be a bare stream of
- * statement reports, which left no room for a reply to travel back the other way.
- *
- * There is no negotiation and no fallback to the old shape, because there is nothing to
- * negotiate with: the agent jar ships inside the plugin, so both ends are always the same
- * build. The tools in plugin/tools speak this format too.
- */
- private static final int EVT_STOPPED = 0x10;
- private static final int EVT_SCOPES = 0x11;
- private static final int EVT_VARIABLES = 0x12;
- private static final int EVT_EVALUATED = 0x13;
- private static final int EVT_VARIABLE_SET = 0x14;
-
/**
* Handle 0 is never issued, so the IDE can use it to mean "this value has no children" without
* a separate flag on every variable.
@@ -254,10 +206,10 @@ private static final class ThreadState {
/** Commands the reader thread has handed to this thread, in arrival order. */
final BlockingQueue mailbox =
- new LinkedBlockingQueue();
+ new LinkedBlockingQueue<>();
/** Objects the IDE may expand, valid only for this thread's current stop. */
- final Map handles = new HashMap();
+ final Map handles = new HashMap<>();
/** The frames of this thread's current stop, innermost first. Empty while running. */
Object[] frames = new Object[0];
@@ -285,7 +237,7 @@ private static final class ThreadState {
/** Live thread states by protocol id, for the reader thread to dispatch into. */
private static final Map threadsById =
- new ConcurrentHashMap();
+ new ConcurrentHashMap<>();
/**
* This thread's state, created on first use.
@@ -293,7 +245,7 @@ private static final class ThreadState {
*
A {@code ThreadLocal} rather than a lookup by {@code Thread.currentThread()} because
* {@link #onEval} consults it on every instrumented node — the hottest path in the agent.
*/
- private static final ThreadLocal STATE = new ThreadLocal();
+ private static final ThreadLocal STATE = new ThreadLocal<>();
private static final AtomicInteger nextThreadId = new AtomicInteger(1);
@@ -313,24 +265,12 @@ private static final class ThreadState {
* overflowed. It stays set for the whole of {@link #report}, so everything served while
* suspended is covered, including an expression that calls a script method.
*/
- private static final ThreadLocal REPORTING = new ThreadLocal();
+ private static final ThreadLocal REPORTING = new ThreadLocal<>();
- /**
- * Serialises writes to the socket, and nothing else.
- *
- *
This is the whole of what used to be {@code LOCK}. Before threads, one lock covered
- * connecting, writing, and being suspended — which is precisely why two threads could not be
- * suspended at once: the first held it for the duration of its stop. Now a stop holds no lock at
- * all; it parks on its own mailbox, and this guards only the moments when bytes are being put on
- * the wire, so a second thread can report while the first is still suspended.
- *
- *
A message must be written under a single acquisition, or two threads' fields would
- * interleave into an unparseable stream.
- */
/**
* Whether every thread should report its next statement, whatever the breakpoints say.
*
- *
How Suspend: All is honoured without pretending to be JDWP. A thread cannot be frozen from
+ *
How Suspend: All is honored without pretending to be JDWP. A thread cannot be frozen from
* outside — it only ever stops where it calls the hook — so "suspend all" is implemented as
* "everyone reports at the next statement, and the IDE decides who stays stopped". The IDE sets
* this when a Suspend: All breakpoint is hit and clears it on resume.
@@ -350,9 +290,7 @@ private static final class ThreadState {
*/
private static final long CONFIGURATION_TIMEOUT_MS = 30_000L;
- private static final Object WRITE_LOCK = new Object();
-
- /** Guards {@link #connect} and the reader-thread start, which must happen exactly once. */
+ /** Guards {@link DebugChannel#connect} and the reader-thread start, which must happen exactly once. */
private static final Object CONNECT_LOCK = new Object();
private static final int port;
@@ -367,7 +305,7 @@ private static final class ThreadState {
* channel decides how to encode it. That is what makes a second protocol a matter of one more
* implementation rather than a second debugger.
*/
- private static DebugChannel channel;
+ private static final DebugChannel channel;
// Reflection handles, resolved once. Every BSH* node inherits these from the
// package-private bsh.SimpleNode, so a single Method works for all of them.
@@ -378,7 +316,6 @@ private static final class ThreadState {
private static Method nodeGetChild;
private static Field whileIsDoStatement;
private static Method callStackDepth;
- private static Method callStackTop;
private static Method nameSpaceGetVariableNames;
private static Method nameSpaceGetVariable;
private static Method nameSpaceGetParent;
@@ -393,14 +330,6 @@ private static final class ThreadState {
private static Method primitiveGetType;
private static boolean reflectionFailed;
- /**
- * Objects the IDE may ask to expand, valid only for the current stop.
- *
- *
Discarded on every resume, which is the whole reason handles are safe: the IDE can never
- * hold a reference into a script that has moved on, so there is no stale-object problem to
- * solve and no cleanup protocol to get wrong. This mirrors DAP, where a
- * {@code variablesReference} is explicitly invalid once execution continues.
- */
static {
int parsed = -1;
String portProperty = System.getProperty(PORT_PROPERTY);
@@ -447,6 +376,7 @@ private BshHook() {
}
/** A port property, or [fallback] when unset or unparseable. */
+ @SuppressWarnings("SameParameterValue")
private static int parsedPort(String property, int fallback) {
String value = System.getProperty(property);
if (value == null || value.trim().isEmpty()) {
@@ -512,7 +442,7 @@ public static void onEval(Object node, Object callstack, Object interpreter) {
}
report(state, line, sourceFile, callstack, interpreter);
} catch (Throwable t) {
- // Never let a debugging problem change the behaviour of the debugged script.
+ // Never let a debugging problem change the behavior of the debugged script.
disabled = true;
System.err.println("[bsh-agent] disabling instrumentation after an internal error: " + t);
close();
@@ -545,22 +475,24 @@ private static boolean isStatement(Object node) throws Exception {
if (index < 0) {
return false;
}
- if (IF_STATEMENT.equals(parentName) || SWITCH_STATEMENT.equals(parentName)) {
- return index >= 1;
- }
- if (FOR_STATEMENT.equals(parentName) || ENHANCED_FOR_STATEMENT.equals(parentName)) {
- return index == count - 1;
- }
- if (WHILE_STATEMENT.equals(parentName)) {
- return isDoStatement(parent) ? index == 0 : index == count - 1;
+ switch (parentName) {
+ case IF_STATEMENT:
+ case SWITCH_STATEMENT:
+ return index >= 1;
+ case FOR_STATEMENT:
+ case ENHANCED_FOR_STATEMENT:
+ return index == count - 1;
+ case WHILE_STATEMENT:
+ return isDoStatement(parent) ? index == 0 : index == count - 1;
+ default:
+ return false;
}
- return false;
}
/** Identity search: nodes have no usable equals(), and the same subtree never repeats. */
private static int indexOfChild(Object parent, Object child, int count) throws Exception {
for (int i = 0; i < count; i++) {
- if (nodeGetChild.invoke(parent, Integer.valueOf(i)) == child) {
+ if (nodeGetChild.invoke(parent, i) == child) {
return i;
}
}
@@ -612,15 +544,15 @@ private static boolean shouldReport(ThreadState state, String sourceFile, int li
if (configured == null || state.runMode != MODE_RUN) {
return true;
}
- List files = configured.get(Integer.valueOf(line));
+ List files = configured.get(line);
if (files == null) {
return false;
}
if (sourceFile == null) {
return false;
}
- for (int i = 0; i < files.size(); i++) {
- if (pathsMatch(sourceFile, files.get(i))) {
+ for (String file : files) {
+ if (pathsMatch(sourceFile, file)) {
return true;
}
}
@@ -683,6 +615,7 @@ private static void drainMailbox(ThreadState state) {
* {@link #applyCommand}. That was never a shortcut: only that thread can safely touch its own
* BeanShell state, and answering from here would need a lock BeanShell does not offer.
*/
+ @SuppressWarnings("ResultOfMethodCallIgnored") // mailbox is an unbounded queue; offer() cannot fail
private static void readerLoop() {
try {
while (true) {
@@ -707,7 +640,7 @@ private static void readerLoop() {
default:
break;
}
- ThreadState target = threadsById.get(Integer.valueOf(command.threadId));
+ ThreadState target = threadsById.get(command.threadId);
if (target == null) {
// A command for a thread that has exited. Dropping it is right: there is nobody
// to answer for it, and the client will have been told the thread is gone.
@@ -735,7 +668,7 @@ private static void readerLoop() {
/**
* Applies one command on the thread that owns the state, and answers it if it expects an answer.
*
- *
Returns true when the thread should stop waiting. An unrecognised command counts as a
+ *
Returns true when the thread should stop waiting. An unrecognized command counts as a
* release: the worst case is a script that keeps running, whereas ignoring it could leave a thread
* parked for good.
*/
@@ -802,6 +735,7 @@ private static void awaitResume(ThreadState state) throws IOException {
}
/** One place for "the IDE went away", which must never abort the host program. */
+ @SuppressWarnings("ResultOfMethodCallIgnored") // mailbox is an unbounded queue; offer() cannot fail
private static void sessionLost(IOException ex) {
if (!disabled) {
System.err.println("[bsh-agent] debug session disconnected; continuing without debugging ("
@@ -810,7 +744,7 @@ private static void sessionLost(IOException ex) {
disabled = true;
close();
// Release everyone parked on a mailbox, or a suspended thread would wait for an IDE that is
- // gone. An empty message array is read as "unrecognised", which applyCommand treats as a
+ // gone. An empty message array is read as "unrecognized", which applyCommand treats as a
// release.
for (ThreadState waiting : threadsById.values()) {
waiting.mailbox.offer(DebugChannel.Command.simple(
@@ -820,14 +754,10 @@ private static void sessionLost(IOException ex) {
/** Replaces the breakpoint set with the one the client just sent. */
private static void applyBreakpoints(DebugChannel.Command command) {
- Map> parsed = new HashMap>();
+ Map> parsed = new HashMap<>();
for (int i = 0; i < command.breakpointLines.length; i++) {
- Integer key = Integer.valueOf(command.breakpointLines[i]);
- List files = parsed.get(key);
- if (files == null) {
- files = new ArrayList(2);
- parsed.put(key, files);
- }
+ Integer key = command.breakpointLines[i];
+ List files = parsed.computeIfAbsent(key, k -> new ArrayList<>(2));
files.add(command.breakpointFiles[i]);
}
// Published as a whole so a concurrent shouldReport() never sees a half-built map.
@@ -866,36 +796,27 @@ private static boolean isReportedSource(String sourceFile) {
* Reads the prefix list, or returns null when there is none to read.
*
*
An unreadable file returns null — "report everything" — rather than failing. A filter is an
- * optimisation over reporting every statement and letting the IDE decide; losing it costs speed,
+ * optimization over reporting every statement and letting the IDE decide; losing it costs speed,
* whereas throwing here would abort somebody's build over a missing temp file.
*/
private static String[] readSourcePrefixes(String path) {
if (path == null || path.trim().isEmpty()) {
return null;
}
- BufferedReader reader = null;
- try {
- reader = new BufferedReader(new InputStreamReader(new FileInputStream(path), "UTF-8"));
- List lines = new ArrayList();
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(Files.newInputStream(Paths.get(path)), StandardCharsets.UTF_8))) {
+ List lines = new ArrayList<>();
String line;
while ((line = reader.readLine()) != null) {
if (!line.isEmpty()) {
lines.add(line);
}
}
- return lines.isEmpty() ? null : lines.toArray(new String[lines.size()]);
+ return lines.isEmpty() ? null : lines.toArray(new String[0]);
} catch (IOException ex) {
System.err.println("[bsh-agent] cannot read " + SOURCE_PREFIXES_FILE_PROPERTY + "='" + path
+ "', reporting every source: " + ex);
return null;
- } finally {
- if (reader != null) {
- try {
- reader.close();
- } catch (IOException ignored) {
- // Nothing useful to do about a failed close on a file we only read.
- }
- }
}
}
@@ -931,7 +852,7 @@ private static void report(ThreadState state, int line, String sourceFile, Objec
state.frames = frames;
state.interpreter = interpreter;
try {
- List reported = new ArrayList(frames.length);
+ List reported = new ArrayList<>(frames.length);
for (int i = 0; i < frames.length; i++) {
// Frame 0 sits at the statement being reported; every outer frame sits at the call
// site recorded by the frame below it. Reading getInvocationLine() off the frame
@@ -979,11 +900,7 @@ private static boolean ensureConnected() {
+ port + " (" + ex + "); aborting");
System.exit(EXIT_DEBUG_UNAVAILABLE);
}
- Thread reader = new Thread(new Runnable() {
- public void run() {
- readerLoop();
- }
- }, "bsh-agent-reader");
+ Thread reader = new Thread(BshHook::readerLoop, "bsh-agent-reader");
reader.setDaemon(true);
reader.start();
}
@@ -1002,6 +919,7 @@ public void run() {
* program. Timing out means the script runs on unfiltered, which is the same outcome as an IDE that
* never sends a breakpoint set.
*/
+ @SuppressWarnings("BusyWait") // bounded 20ms poll against a 30s deadline, not an unbounded spin
private static boolean waitForConfiguration() {
if (!(channel instanceof DapChannel)) {
return true;
@@ -1038,7 +956,7 @@ private static ThreadState stateFor() {
Thread current = Thread.currentThread();
ThreadState created = new ThreadState(nextThreadId.getAndIncrement(), current.getName());
STATE.set(created);
- threadsById.put(Integer.valueOf(created.id), created);
+ threadsById.put(created.id, created);
return created;
}
@@ -1102,41 +1020,78 @@ private static String nullToEmpty(String value) {
}
/**
- * Answers {@link #CMD_SCOPES}: the scopes of one frame, each a handle the IDE can expand.
+ * Answers {@link DebugChannel.Command.Kind#SCOPES}: the scopes of one frame, each a handle the
+ * IDE can expand.
*
- *
Two scopes, and the second is the point of having the level at all: Global is the
- * interpreter's own namespace, which is where a script's top-level state lives once execution has
- * descended into a method. Without it, stopping inside a method shows the method's locals and
- * nothing else — the script's own globals become invisible exactly when they are most likely to
- * be what is wrong.
+ *
One scope per level of the namespace chain, innermost first, each reporting only the
+ * variables declared directly in it — not its own view of everything an enclosing scope also
+ * holds. A {@code for} loop's own namespace and the block inside it are two such levels, so a
+ * loop variable declared in the {@code for}'s init no longer gets lost inside a "Locals" group
+ * whose own reporting already walked past it while gathering something else.
*
- *
Global is omitted when it is the frame's namespace (a stop at top level, where the two
- * are the same object) and when there is no interpreter to ask — the rewriting path, which is
- * handed a namespace only.
+ *
Global — the interpreter's own namespace, where a script's top-level state lives once
+ * execution has descended into a method — is the last level reached, identified by object
+ * identity rather than by position: without it, stopping inside a method would show the method's
+ * locals and nothing else, exactly when the script's own globals are most likely to be what is
+ * wrong. It is naturally absent when the walk never reaches it — the rewriting path, which is
+ * handed a namespace only and has no interpreter to ask.
*/
private static List collectScopes(ThreadState state, int frameId) {
Object namespace = frame(state, frameId);
Object global = globalNameSpace(state);
- boolean hasGlobal = global != null && global != namespace;
- List scopes = new ArrayList(2);
- if (namespace != null) {
- scopes.add(new DebugChannel.Scope("Locals", handleFor(state, namespace)));
- }
- if (hasGlobal) {
- scopes.add(new DebugChannel.Scope("Global", handleFor(state, global)));
+ List scopes = new ArrayList<>();
+ Object level = namespace;
+ boolean innermost = true;
+ while (level != null) {
+ String name = innermost ? "Locals" : level == global ? "Global" : levelName(level);
+ scopes.add(new DebugChannel.Scope(name, handleFor(state, new ScopeLevel(level))));
+ if (level == global) {
+ break;
+ }
+ innermost = false;
+ level = parentOf(level);
}
return scopes;
}
+ /** A block ("for", "if", …) or a closure's own namespace, once its parent has already been walked. */
+ private static String levelName(Object namespace) {
+ return "BlockNameSpace".equals(simpleName(namespace)) ? "Block" : "Closure";
+ }
+
+ /** {@code NameSpace.getParent()}, or null once the chain ends or cannot be read further. */
+ private static Object parentOf(Object namespace) {
+ try {
+ return nameSpaceGetParent.invoke(namespace);
+ } catch (Throwable t) {
+ return null;
+ }
+ }
+
+ /**
+ * One level of {@link #collectScopes}'s namespace chain, so {@link #collectVariables} can tell it
+ * apart from a raw {@code NameSpace} — which still reports its whole parent chain, for expanding a
+ * closure's captured scope as a single value rather than as a list of levels.
+ */
+ private static final class ScopeLevel {
+ final Object namespace;
+
+ ScopeLevel(Object namespace) {
+ this.namespace = namespace;
+ }
+ }
+
/**
* The children of one handle, each with a handle of its own when it can be expanded further.
*/
private static List collectVariables(ThreadState state, int handle) {
- Object target = state.handles.get(Integer.valueOf(handle));
- List children = new ArrayList();
- List
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+