From 35bd5b58239f7e2f2d5555f19c79852898df0c06 Mon Sep 17 00:00:00 2001 From: Maksim Medvedev Date: Tue, 18 Aug 2026 20:34:21 +0000 Subject: [PATCH] [platform] IJPL-253271 fire write-action boundary listeners for outermost WA relative to the suspending-WA base During a suspending write action the outer write action is downgraded to a write-intent lock and `myWriteStackBase` is advanced past it, but the outer WA is not popped from `myWriteActionsStack`. A write action that becomes pending inside that window is the outermost write action relative to the base, yet the four before-start/after-finished boundary guards tested `myWriteActionsStack.isEmpty()` -- which is false because the downgraded outer WA is still on the stack. `beforeWriteActionStart` was therefore skipped, so `ProgressIndicatorUtilService` never cancelled reads running with write-action priority. The pending write upgrade then waited forever on the never-draining read, observed in the wild as an IDE freeze where a "Scanning" thread kept calling ProgressManager.checkCanceled() under runInReadActionWithWriteActionPriority while the EDT was stuck starting a write action. Fix: gate the boundary listeners on `myWriteActionsStack.size == myWriteStackBase` (new `isOutermostWriteAction()` helper) instead of an empty stack. At top level the base is 0, so behavior is unchanged; only the suspending-WA window is fixed. Adds a regression test in BackgroundWriteActionTest that fails before the fix (the write-priority read is never cancelled) and passes after. M-Session-Id: M-c931d72f-abc1-4c65-95ec-5611e404d28c (cherry picked from commit 8e43e5644e5bb85b20c355cda943dd57127c5e9e) GitOrigin-RevId: a7ad4e1a06fb198af5f91e45177d33a3e937f075 --- .../src/NestedLocksThreadingSupport.kt | 20 ++++++-- .../impl/BackgroundWriteActionTest.kt | 50 +++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/platform/locking-impl/src/NestedLocksThreadingSupport.kt b/platform/locking-impl/src/NestedLocksThreadingSupport.kt index 3d2ed8907e32..991bba1772f8 100644 --- a/platform/locking-impl/src/NestedLocksThreadingSupport.kt +++ b/platform/locking-impl/src/NestedLocksThreadingSupport.kt @@ -1019,7 +1019,7 @@ class NestedLocksThreadingSupport : ThreadingSupport { finally { drainWriteActionFollowups() writeIntentInitResult.release() - if (myWriteActionsStack.isEmpty()) { + if (isOutermostWriteAction()) { fireAfterWriteActionFinished(writeIntentInitResult.listeners, clazz) } } @@ -1033,7 +1033,7 @@ class NestedLocksThreadingSupport : ThreadingSupport { return proceedWithSuspendWriteLockAcquisitionFromWriteIntent(computationState, writeIntentInitResult, clazz, computation) } finally { - if (myWriteActionsStack.isEmpty()) { + if (isOutermostWriteAction()) { fireAfterWriteActionFinished(writeIntentInitResult.listeners, clazz) } } @@ -1105,7 +1105,7 @@ class NestedLocksThreadingSupport : ThreadingSupport { } finally { cleanup() - if (myWriteActionsStack.isEmpty()) { + if (isOutermostWriteAction()) { fireAfterWriteActionFinished(frozenListeners, clazz) } } @@ -1199,7 +1199,7 @@ class NestedLocksThreadingSupport : ThreadingSupport { startPendingWriteAction(state) - if (myWriteActionsStack.isEmpty()) { + if (isOutermostWriteAction()) { fireBeforeWriteActionStart(frozenListeners, clazz) } return frozenListeners @@ -1311,6 +1311,18 @@ class NestedLocksThreadingSupport : ThreadingSupport { } } + /** + * A write action is the outermost one when the write-action stack holds no entries above [myWriteStackBase]. + * + * In the common case [myWriteStackBase] is `0`, so this is equivalent to [myWriteActionsStack] being empty. + * During a suspending write action the outer write action is downgraded to a write-intent lock and the base is + * advanced past it (see [downgradeWriteLockToWriteIntent]) without popping it from the stack; a write action that + * starts inside that window is therefore outermost relative to the base even though the stack is not literally + * empty. The outermost-boundary listeners ([fireBeforeWriteActionStart]/[fireAfterWriteActionFinished]) must fire + * for it so that write-action-priority reads get cancelled. + */ + private fun isOutermostWriteAction(): Boolean = myWriteActionsStack.size == myWriteStackBase + fun downgradeWriteLockToWriteIntent(): AccessToken { val state = getComputationState() val permit = state.getThisThreadPermit() diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/BackgroundWriteActionTest.kt b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/BackgroundWriteActionTest.kt index 2dbc26ef7044..43696ef182b8 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/BackgroundWriteActionTest.kt +++ b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/BackgroundWriteActionTest.kt @@ -20,6 +20,7 @@ import com.intellij.openapi.application.useBackgroundWriteAction import com.intellij.openapi.application.useTrueSuspensionForWriteAction import com.intellij.openapi.components.service import com.intellij.openapi.progress.Cancellation +import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.progress.runBlockingCancellable import com.intellij.openapi.progress.util.ProgressIndicatorUtils @@ -558,6 +559,55 @@ class BackgroundWriteActionTest { } } + /** + * Regression test for a missed write-action-priority cancellation during a suspending write action. + * + * A suspending write action ([com.intellij.openapi.application.ThreadingSupport.executeSuspendingWriteAction]) + * temporarily downgrades the outer write action to a write-intent lock and advances the write-stack base past it. A + * write action that becomes pending inside that window is the outermost write action relative to the base, so it must + * fire `beforeWriteActionStart` -- the event [com.intellij.openapi.progress.util.ProgressIndicatorUtilService] uses to + * cancel read actions running with write-action priority. + * + * The firing used to be gated on a literally-empty write-action stack, which is never empty during the window (the + * downgraded outer write action is still on the stack). As a result the write-priority read was never canceled and the + * pending write upgrade waited forever on the never-draining read -- observed in the wild as an IDE freeze where a + * "Scanning" thread kept calling [ProgressManager.checkCanceled] under `runInReadActionWithWriteActionPriority` while + * the EDT was stuck trying to start a write action. + */ + @Suppress("DEPRECATION") + @Test + fun `write action pending inside a suspending write action cancels a write-priority read`(): Unit = + timeoutRunBlocking(context = Dispatchers.Default, timeout = 30.seconds) { + val readStarted = Job() + val readCanceled = AtomicBoolean(false) + edtWriteAction { + // downgrades the outer (EDT) write action to a write-intent lock and advances the write-stack base past it + getGlobalThreadingSupport().executeSuspendingWriteAction { + launch(Dispatchers.Default) { + ProgressIndicatorUtils.runInReadActionWithWriteActionPriority({ + readStarted.complete() + // spin with write-action priority; a pending write action must cancel this read. + // bounded so that on the buggy code path the test fails with a clear assertion instead of hanging. + val deadlineNs = System.nanoTime() + 15.seconds.inWholeNanoseconds + try { + while (System.nanoTime() < deadlineNs) { + ProgressManager.checkCanceled() + } + } + catch (_: ProcessCanceledException) { + readCanceled.set(true) + } + }, null) + } + // make sure the read holds a read permit and registered its write-action-priority cancellation + readStarted.asCompletableFuture().join() + // this write action becomes pending during the downgrade window; it must cancel the read above + application.runWriteAction { } + } + } + assertTrue(readCanceled.get(), "the write-priority read must be canceled by the write action pending inside the suspending write action") + } + /** * This test is not set in stone; if you feel that the platform is ready to block same-level read actions, the feel free to adjust the test. */