Skip to content

Fix fast-click auto-mode crash: overlapping runs + proof breaking proof listeners#3880

Merged
unp1 merged 2 commits into
mainfrom
bubel/automode-reentrancy-fix
Jul 2, 2026
Merged

Fix fast-click auto-mode crash: overlapping runs + proof breaking proof listeners#3880
unp1 merged 2 commits into
mainfrom
bubel/automode-reentrancy-fix

Conversation

@unp1

@unp1 unp1 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Intended Change

Rapidly clicking the auto-mode / apply-strategy toolbar button can crash the proof with a stray
exception on a SwingWorker thread. The crash turned out to be layered — two independent
causes, both fixed here.

Layer 1 — overlapping auto-mode/macro runs

Starting the automatic proof search (auto mode or a proof macro) while a previous run on the same
proof is still winding down runs two provers on one Proof concurrently and corrupts shared proof
state. The symptom is a stray exception on a SwingWorker thread — e.g. NullPointerException in
NamespaceSet.copy (from Goal.adaptNamespacesNewGoals), AssertionFailure: Expected an empty leaf node, or a null SemisequentTacletAppIndex.succIndex.

Root cause: MediatorProofControl guards re-entry only through the mediator's inAutoMode flag,
which ProofMacroWorker.done() clears. But SwingWorker.done() fires on cancellation, while the
cooperative doInBackground is still applying rules — so the flag drops while the run is still
winding down and the next click starts a second, overlapping run.

This PR adds an explicit re-entrancy guard (AtomicBoolean autoModeActive) in
MediatorProofControl: startAutoMode/runMacro claim it on entry and it is released only when
the worker's background task has truly finished
(doInBackground's finally, with a backstop for
the cancelled-before-start case). A new run is refused while one is active or winding down, so two
runs can never prove the same proof at once. It also hardens the analogous guard in
DefaultProofControl (volatile autoModeThread, synchronized runMacro); that control already
released at true completion and the CLI runs blocking, so this is defence-in-depth only.

Layer 2 — a listener must not be able to break the proof

Proofnotifies allRuleAppListeners and ProofTreeListeners with bare dispatch loops, so *any* observer (now or in the future) could break the proof. Rather than have each listener guard itself, this PR isolates them at the firing site: a new Proof.notifyListenershelper wraps every callback — a listener that throws is logged and **unregistered**, and the remaining listeners are still notified.fireRuleAppliedand thefireProof*` dispatches all route through it, so no listener can corrupt the proof.

The problem appeared in this case (but it is not specific to the DependencyTracker) with the slicing extension's always track setting on, DependencyTracker is registered as a RuleAppListener, so its ruleApplied(..) runs inside Goal.apply (via Proof.fireRuleApplied), after the proof tree and namespaces have already been updated for the step.
When tracking failed there — e.g. IllegalStateException: found formula that was not produced by any rule! on a loop-scope invariant while auto mode re-applied the loop-scope rule — the exception
propagated out of Goal.apply, aborting the run mid-application and leaving the proof inconsistent.
That broken state is what then produced the NullPointerException in Goal.adaptNamespacesNewGoals
on the next step.

DependencyTracker additionally records, on a tracking failure, that its dependency graph became
incomplete, so a later analyze() returns no slice rather than one built from a partial graph.

The deeper reason the tracker can miss a loop-scope formula on a re-run is a separate
slicing-correctness question (the naming part was addressed by the #3834 fix already on main); this
PR makes any listener fail safe rather than fatally.

Type of pull request

  • Bug fix (non-breaking change which fixes an issue)
  • There are changes to the (Java) code

Ensuring quality

  • I made sure that introduced/changed code is well documented (javadoc and inline comments).
  • I have tested the feature as follows:
    • Layer 1: reproduced the corruption headlessly (two concurrent auto-mode runs on one proof
      corrupt it reliably; interrupting a single run and waiting does not), confirmed that the GUI
      genuinely runs two auto-mode workers on one proof when fast-clicking, and verified that the guard
      prevents the overlap. Final check: fast-clicking the auto button in the GUI no longer crashes.
    • Layer 2: added ProofListenerIsolationTest — a RuleAppListener that always throws neither
      stops the proof from closing nor keeps firing (it is unregistered after its first failure).
      Verified it fails without the guard.
  • I have checked that runtime performance has not deteriorated: the re-entrancy guard is a single
    atomic CAS per run start; listener isolation only wraps the existing dispatch in a try/catch — no
    effect on the proof-search hot path.

The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.

Additional Information and contact(s)

Created with AI tooling support

The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.

Starting auto mode or a proof macro while a previous run on the same proof
is still winding down runs two provers on one Proof concurrently and
corrupts shared proof state (most easily triggered by rapidly clicking the
auto-mode toolbar button). The symptom is a stray exception on a
SwingWorker thread, e.g. a NullPointerException in NamespaceSet.copy from
Goal.adaptNamespacesNewGoals, an "Expected an empty leaf node" assertion,
or a null SemisequentTacletAppIndex.succIndex.

MediatorProofControl guarded re-entry only via the mediator's inAutoMode
flag, which ProofMacroWorker.done() clears -- but SwingWorker.done() fires
on cancellation while the cooperative doInBackground is still applying
rules, so the flag drops mid-winddown and the next click starts an
overlapping run.

Add an explicit AtomicBoolean guard in MediatorProofControl that
startAutoMode/runMacro claim on entry and that is released only when the
worker's background task has truly finished (doInBackground finally, with a
backstop for the cancelled-before-start case). Also harden the analogous
guard in DefaultProofControl (volatile autoModeThread, synchronized
runMacro); it already released at true completion and the CLI runs
blocking, so that is defence in depth.
@unp1 unp1 added this to the v3.0.0 milestone Jul 1, 2026
@unp1 unp1 added the 🐞 Bug label Jul 1, 2026
@unp1 unp1 self-assigned this Jul 1, 2026
@unp1
unp1 marked this pull request as ready for review July 1, 2026 15:36
@unp1
unp1 enabled auto-merge July 1, 2026 15:37
@unp1
unp1 disabled auto-merge July 1, 2026 16:31
@unp1
unp1 enabled auto-merge July 1, 2026 16:31
@unp1
unp1 marked this pull request as draft July 1, 2026 17:53
auto-merge was automatically disabled July 1, 2026 17:53

Pull request was converted to draft

@unp1 unp1 changed the title Prevent overlapping auto-mode/macro runs on the same proof (fast-click crash) Fix fast-click auto-mode crash: overlapping runs + dependency tracker Jul 1, 2026
@unp1
unp1 force-pushed the bubel/automode-reentrancy-fix branch from 4a5a0ff to 4647278 Compare July 1, 2026 19:10
@unp1 unp1 changed the title Fix fast-click auto-mode crash: overlapping runs + dependency tracker Fix fast-click auto-mode crash: overlapping runs + proof breaking proof listeners Jul 1, 2026
@unp1
unp1 marked this pull request as ready for review July 1, 2026 19:16
DependencyTracker (and every other RuleAppListener / ProofTreeListener) is
notified from within a running rule application -- e.g. Proof.fireRuleApplied is
called from Goal.apply, after the proof tree and namespaces have been updated
for the step. A listener that threw there aborted the proof search
mid-application and left the proof inconsistent (surfacing downstream as a
NullPointerException in Goal.adaptNamespacesNewGoals). Guarding each listener
individually does not scale to current and future listeners.

Instead isolate listeners at the firing site: Proof.notifyListeners wraps every
callback, logs a listener that throws and unregisters it, then keeps notifying
the rest. fireRuleApplied and the fireProof* dispatches all go through it, so no
observer can corrupt the proof.

DependencyTracker keeps a little self-hygiene: on a tracking failure it records
that its graph became incomplete (analyze() then returns no results) and
rethrows, letting Proof log and drop it. Add ProofListenerIsolationTest (a
throwing RuleAppListener neither stops the proof from closing nor keeps firing);
verified it fails without the guard.

This is the second, independent layer of the fast-click crash; layer one is the
re-entrancy guard in this PR.

Created with AI tooling support
@unp1
unp1 force-pushed the bubel/automode-reentrancy-fix branch from 4647278 to 41b732f Compare July 1, 2026 19:28
@unp1
unp1 enabled auto-merge July 1, 2026 19:35
@unp1 unp1 added the P:HIGH label Jul 2, 2026
@unp1
unp1 added this pull request to the merge queue Jul 2, 2026
Merged via the queue into main with commit b6ebc90 Jul 2, 2026
36 checks passed
@unp1
unp1 deleted the bubel/automode-reentrancy-fix branch July 2, 2026 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants