Repository navigation
Follow-ups from the PR #46 code review: per-stop reasons, per-invocation watchdog timing, late ring writes, race tests #47
Description
Follow-up to PR #46 (#42 fast breakpoints, #45 suspension ownership). The Codex code review before merge found these as MINOR, or as test gaps; they were deferred so the PR could close the MAJOR findings first.
1. A stop's reason belongs to the entry, not the stop
FastLogpointService keeps lastStopReason per fast breakpoint, and the stop report reads it when it renders. With stopOnce=false and suspendPolicy=thread, two threads can be held at stops of the same ID at once:
- T1 stops with "condition true".
- T2 stops with "condition failed: …".
- Rendering T1's stop now shows T2's failure.
Fix: carry the reason, the error text and the generation in each stop's snapshot or context, not in the entry. Test: two stops of one ID held at once, with different reasons.
2. The watchdog times the lease, not the invocation
SuspensionWatchdog keys its age and MONITOR samples by the dispatcher lease. A lease can span host-side work (classpath discovery during configureCompilerClasspath) and several native invocations (defineClass, initialisation, the call). Two consequences:
- A native invocation that is less than 3 s old can qualify, once the host work before it aged the lease.
- Samples can straddle two invocations.
The effect is that a queued set is cancelled somewhat early. Nothing is released unless every other check in unblock holds.
Fix: record entry, exit and age for each actual invokeMethod, and reset the monitor history at those boundaries.
Related: SuspensionSession.heldThreads now treats a leased thread whose stop was released mid-evaluation as UNKNOWN_HOLD until the lease ends. This is a conservative stand-in for the same missing information: what the native invocation phase is. With per-invocation tracking, that rule could judge the thread's count exactly, and stop deferring redefinitions for the whole evaluation.
3. Late writes into a fast logpoint's ring
Open since the § 19 review of #45 (finding 9). A probe thread that passed the kill switch can finish its write after the final drain, the class's restore and the ring cleanup. The record then goes into the probe's static ring after the debugger has dropped its handle. It is lost, and its string stays reachable through the probe class. A deferred removal (REMOVAL_PENDING) now also drops the handle before the redefinition.
Fix: keep the retirement state until in-flight writers are done, then drain and clean up. Alternatively, make target-side ring detachment safe for late writers.
4. Test gaps
The fake suspend model and the mocked frame tools keep their snapshots stable at the moments these races need. Add:
-
Deterministic barriers around:
- reservation, then the evaluation check, then lease entry;
- the hold check, then a frame tool's retry;
- the owner query, then
unblock; - compilation, then a replaced stop.
-
Live cases:
- a stop-all anchor transferred to a running evaluation;
jdwp_resetduring an invocation;- an invocation that never returns, followed by disconnect;
- a cancelled thread-policy set followed by a new stop of the same thread.
Each should assert that every count ends at 0 and the application keeps running after every owner is released.
-
A live case for co-delivered arming hits: a fast logpoint and a fast breakpoint on one line, set before either fires. This needs a condition true on exactly that pass. Today a listener unit test pins the order.
Activity
chore: release 2.11.0 — fast logpoints, fast breakpoints and suspensi…