fix(ADFA-4384): stop Kotlin LSP index workers before disposing project#1424
Merged
Merged
Conversation
The Kotlin LSP background IndexWorker runs an unbounded coroutine loop that calls PsiManager.findFile(project) under a read lock. On LSP shutdown, AbstractCompilationEnvironment.close() disposed the IntelliJ Project immediately via Disposer.dispose(...), while the worker coroutine was still live. The next findFile() then threw "AssertionError: Project is already disposed", crashing the app. KtSymbolIndex.close() already implements the orderly shutdown (submit Stop, join the worker jobs) but nothing called it. Fix: - AbstractCompilationEnvironment.close(): stop & join the index workers via ktSymbolIndex.close() before Disposer.dispose(...). (In the base class so the lateinit isInitialized check is legal.) - CompilationEnvironment.close(): cancel coroutineScope (fileAnalyzer also reads the project) before super.close(). - KtSymbolIndex.close(): cancel its own scope after joining so the debounced modifiedFileIndexer can't fire post-dispose. - IndexWorker: defensive project.isDisposed guards for any disposal path that doesn't drain the worker first. Adds IndexWorkerDisposalTest (reproduces the crash without the guards) and a docs/process/learnings.md note on disposing the LSP test env inside a write action. Fixes APPDEVFORALL-17R
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughRelease Notes
Risks & Best Practices Concerns
WalkthroughFixes a crash where background ChangesIndexWorker Disposal Race Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt (1)
52-59:⚠️ Potential issue | 🟠 MajorAdd the same disposal guard to the debounced modified-file callback.
The
modifiedFileIndexercallback (line 55-59) callsindexSourceFile(project, ...)without aproject.isDisposedcheck. AlthoughKtSymbolIndex.close()cancels the scope before project disposal, a race condition remains: the debounced action can execute between the start of scope cancellation and its completion, causing the callback to access a disposed project. Add the guard to match the pattern already used in the main loop (lines 65, 80, 121).✅ Suggested patch
val modifiedFileIndexer = KeyedDebouncingAction<ModFileIndexKey>( scope = scope, debounceDuration = CompilationEnvironment.DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION ) { (path, ktFile), cancelChecker -> + if (project.isDisposed) return@KeyedDebouncingAction logger.debug("Indexing modified file: {}", path) indexSourceFile(project, ktFile, fileIndex, sourceIndex, cancelChecker) sourceIndexCount++ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt` around lines 52 - 59, The modifiedFileIndexer callback does not check if the project is disposed before calling indexSourceFile, creating a race condition where the debounced action may execute after scope cancellation begins but before it completes, causing access to a disposed project. Add a project.isDisposed check at the beginning of the callback lambda in modifiedFileIndexer (the one with parameters (path, ktFile), cancelChecker) before the indexSourceFile call, following the same pattern used elsewhere in the code at lines 65, 80, and 121. If the project is disposed, simply return early from the callback without executing the indexSourceFile call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt`:
- Around line 52-59: The modifiedFileIndexer callback does not check if the
project is disposed before calling indexSourceFile, creating a race condition
where the debounced action may execute after scope cancellation begins but
before it completes, causing access to a disposed project. Add a
project.isDisposed check at the beginning of the callback lambda in
modifiedFileIndexer (the one with parameters (path, ktFile), cancelChecker)
before the indexSourceFile call, following the same pattern used elsewhere in
the code at lines 65, 80, and 121. If the project is disposed, simply return
early from the callback without executing the indexSourceFile call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c70b10fa-8f96-49dc-bdc0-f8f0fb0d2342
📒 Files selected for processing (6)
docs/process/learnings.mdlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/AbstractCompilationEnvironment.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorkerDisposalTest.kt
Signed-off-by: Akash Yadav <itsaky01@gmail.com>
itsaky-adfa
approved these changes
Jun 19, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt`:
- Around line 77-82: The disposal check for project.isDisposed is non-atomic
with the project.read operation that happens inside the toMetadata() method
call, allowing disposal to occur between the check and the actual invocation.
Wrap the ktFile.toMetadata(project, isIndexed = true) call in an atomic read
operation using project.read or a similar mechanism that ensures the disposal
check and the metadata retrieval happen atomically, preventing any race
condition where the project could be disposed between the defensive check and
the method execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87e8ecee-22e2-4d3f-b91a-f40c4077f5ed
📒 Files selected for processing (2)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt
…er, atomic disposal check
Follow-up to code review on the disposal fix:
- AbstractCompilationEnvironment.close(): runBlocking { ktSymbolIndex.close() }
runs on the main thread during editor teardown; bound it with
withTimeoutOrNull(CLOSE_DRAIN_TIMEOUT_MS) so a slow PSI read can't ANR. The
project.isDisposed guards cover the rare timeout-before-drain case.
- CompilationEnvironment.close(): the fileAnalyzer scope was only cancel()'d
(fire-and-forget) before disposal, so an in-flight collectDiagnosticsFor
project.read could still touch a disposed project. Cancel AND join it
(bounded) before super.close().
- SourceFileIndexer.indexSourceFile(): the project.isDisposed fast-path was
non-atomic with toMetadata()'s internal project.read. Re-check disposal
inside the read lock so the check and the PSI access are atomic.
All :lsp:kotlin unit tests pass (41, 0 failures).
jatezzz
pushed a commit
that referenced
this pull request
Jun 22, 2026
#1424) * fix(ADFA-4384): stop Kotlin LSP index workers before disposing project The Kotlin LSP background IndexWorker runs an unbounded coroutine loop that calls PsiManager.findFile(project) under a read lock. On LSP shutdown, AbstractCompilationEnvironment.close() disposed the IntelliJ Project immediately via Disposer.dispose(...), while the worker coroutine was still live. The next findFile() then threw "AssertionError: Project is already disposed", crashing the app. KtSymbolIndex.close() already implements the orderly shutdown (submit Stop, join the worker jobs) but nothing called it. Fix: - AbstractCompilationEnvironment.close(): stop & join the index workers via ktSymbolIndex.close() before Disposer.dispose(...). (In the base class so the lateinit isInitialized check is legal.) - CompilationEnvironment.close(): cancel coroutineScope (fileAnalyzer also reads the project) before super.close(). - KtSymbolIndex.close(): cancel its own scope after joining so the debounced modifiedFileIndexer can't fire post-dispose. - IndexWorker: defensive project.isDisposed guards for any disposal path that doesn't drain the worker first. Adds IndexWorkerDisposalTest (reproduces the crash without the guards) and a docs/process/learnings.md note on disposing the LSP test env inside a write action. Fixes APPDEVFORALL-17R * fix: add defensive backstop in source file indexer Signed-off-by: Akash Yadav <itsaky01@gmail.com> * fix(ADFA-4384): address review — bound shutdown join, join fileAnalyzer, atomic disposal check Follow-up to code review on the disposal fix: - AbstractCompilationEnvironment.close(): runBlocking { ktSymbolIndex.close() } runs on the main thread during editor teardown; bound it with withTimeoutOrNull(CLOSE_DRAIN_TIMEOUT_MS) so a slow PSI read can't ANR. The project.isDisposed guards cover the rare timeout-before-drain case. - CompilationEnvironment.close(): the fileAnalyzer scope was only cancel()'d (fire-and-forget) before disposal, so an in-flight collectDiagnosticsFor project.read could still touch a disposed project. Cancel AND join it (bounded) before super.close(). - SourceFileIndexer.indexSourceFile(): the project.isDisposed fast-path was non-atomic with toMetadata()'s internal project.read. Re-check disposal inside the read lock so the check and the PSI access are atomic. All :lsp:kotlin unit tests pass (41, 0 failures). --------- Signed-off-by: Akash Yadav <itsaky01@gmail.com> Co-authored-by: Akash Yadav <itsaky01@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the top unassigned Sentry crash APPDEVFORALL-17R / ADFA-4384 —
AssertionError: Project is already disposed.Root cause
The Kotlin LSP background
IndexWorkerruns an unbounded coroutine loop that callsPsiManager.findFile(project)under a read lock. On LSP shutdown,AbstractCompilationEnvironment.close()disposed the IntelliJProjectimmediately (Disposer.dispose(...)) while the worker coroutine was still live. The nextfindFile()then threwAssertionError: Project is already disposedon a coroutine worker thread → fatal crash.KtSymbolIndex.close()already implements the orderly shutdown (submitStop, join the worker jobs) — but nothing ever called it.Fix
AbstractCompilationEnvironment.close()— stop & join the index workers viaktSymbolIndex.close()beforeDisposer.dispose(...). (Placed in the base class so the::ktSymbolIndex.isInitializedcheck can legally reach thelateinitbacking field.)CompilationEnvironment.close()— cancelcoroutineScope(thefileAnalyzeralso reads the project) beforesuper.close().KtSymbolIndex.close()— cancel its ownscopeafter joining, so the debouncedmodifiedFileIndexercan't fire post-dispose.IndexWorker— defensiveproject.isDisposedguards (loop top + before eachfindFile) for any disposal path that doesn't drain the worker first.Testing
IndexWorkerDisposalTest(JVM/Robolectric, no emulator) disposes the project via the real production path and asserts the worker exits cleanly.AssertionError— confirming it is a real regression test.:lsp:kotlinunit suite green: 41 tests, 0 failures.runWriteAction { env.close() }) indocs/process/learnings.md.Fixes APPDEVFORALL-17R