From 75927f07c1443818aaaa3ade34d1415c0e498dac Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Sat, 25 Oct 2025 13:17:49 +0200 Subject: [PATCH] Fix race condition in AbstractReconciler.reset() causing test timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2708 The test FastAbstractReconcilerTest#testReplacingDocumentWhenClean() was intermittently failing with a 5-second timeout. The test would wait at a barrier expecting the reconciler to process, but the reconciler thread never executed the process() method. Root Cause: ----------- The race condition occurred in the BackgroundWorker.reset() method due to improper ordering of operations with two separate locks (the BackgroundWorker instance lock and the fDirtyRegionQueue lock): Old sequence: 1. Set fIsDirty=true and fReset=true (under BackgroundWorker lock) 2. Notify fDirtyRegionQueue 3. Call informNotFinished() which calls aboutToWork() 4. aboutToWork() may call signalWaitForFinish() which sets waitFinish=true The problem: The reconciler thread could wake up from the notification in step 2 BEFORE waitFinish was set to true in step 4, causing it to delay again for the full fDelay period instead of processing immediately. Additionally, due to the two separate locks, the reconciler thread could check isDirty() and see a stale value before fIsDirty was set. The Fix: -------- Move the informNotFinished() call to AFTER setting fIsDirty/fReset but BEFORE notifying the queue: New sequence: 1. Set fIsDirty=true and fReset=true (under BackgroundWorker lock) 2. Call informNotFinished() → aboutToWork() → signalWaitForFinish() 3. signalWaitForFinish() sets waitFinish=true and notifies queue 4. Final notifyAll() on queue (redundant but harmless) This ensures that when the reconciler thread wakes up, both fIsDirty and waitFinish are already set to their correct values, eliminating the race. Testing: -------- - Compiled successfully with mvn clean compile -Pbuild-individual-bundles - The fix preserves the existing behavior while ensuring proper synchronization - FastAbstractReconcilerTest.testReplacingDocumentWhenClean should no longer timeout 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../org/eclipse/jface/text/reconciler/AbstractReconciler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java index 2978a8b9ec9..359cbe08e50 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java @@ -142,6 +142,7 @@ public void reset() { fIsDirty= true; fReset= true; } + informNotFinished(); synchronized (fDirtyRegionQueue) { fDirtyRegionQueue.notifyAll(); // wake up wait(fDelay); } @@ -151,13 +152,12 @@ public void reset() { synchronized (this) { fIsDirty= true; } - + informNotFinished(); synchronized (fDirtyRegionQueue) { fDirtyRegionQueue.notifyAll(); } } - informNotFinished(); reconcilerReset(); }