From d8c32bd068bbec0ebe9c33cb4ba0c567154cb393 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 19 Jun 2026 22:28:53 +0530 Subject: [PATCH 1/3] fix: add global analysis lock for Kotlin analysis Signed-off-by: Akash Yadav --- .../lsp/kotlin/compiler/modules/KtFileExts.kt | 38 ++++++++-- .../kotlin/completion/KotlinCompletions.kt | 69 ++++++++++--------- 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt index bdcc53abf5..5e530badcb 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt @@ -11,14 +11,40 @@ import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.UserDataProperty import java.nio.file.Path +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock private val KT_LSP_COMPLETION_BACKING_FILE = Key("KT_LSP_COMPLETION_BACKING_FILE") var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) -internal inline fun analyzeMaybeDangling(useSiteElement: KtElement, crossinline action: KaSession.() -> R): R { - if (useSiteElement is KtFile && useSiteElement.isDangling && useSiteElement.copyOrigin != null) { - return analyzeCopy(useSiteElement, KaDanglingFileResolutionMode.PREFER_SELF, action) - } +/** + * Serializes all Kotlin Analysis API access (`analyze` / `analyzeCopy`). + * + * The Analysis API tracks its `analyze` lifetime context in a per-thread stack and is not safe to + * drive concurrently from multiple background threads without the platform read-action coordination + * that this LSP replaces with a custom [com.itsaky.androidide.lsp.kotlin.compiler.read] lock. + * Indexing, diagnostics and completion all run analysis on `Dispatchers.Default` and frequently + * target the same edited file, so overlapping `analyze` calls corrupted the lifetime/session + * lifecycle and surfaced as + * `KaInaccessibleLifetimeOwnerAccessException: ... Called outside an \`analyze\` context.` + * + * Holding this lock around every analysis entry point makes analyses mutually exclusive. It is a + * [ReentrantLock] so an (indirect) nested analysis on the same thread cannot deadlock. + */ +private val analysisLock = ReentrantLock() + +/** + * Runs [action] while holding the shared [analysisLock]. **All** Analysis API access must go through + * this helper (or [analyzeMaybeDangling], which already does); never call `analyze` / `analyzeCopy` + * directly, or the serialization guarantee is lost. + */ +internal inline fun withAnalysisLock(action: () -> R): R = analysisLock.withLock(action) - return analyze(useSiteElement, action) -} +internal inline fun analyzeMaybeDangling(useSiteElement: KtElement, crossinline action: KaSession.() -> R): R = + withAnalysisLock { + if (useSiteElement is KtFile && useSiteElement.isDangling && useSiteElement.copyOrigin != null) { + analyzeCopy(useSiteElement, KaDanglingFileResolutionMode.PREFER_SELF, action) + } else { + analyze(useSiteElement, action) + } + } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt index a558208e94..1058ecc330 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt @@ -3,6 +3,7 @@ package com.itsaky.androidide.lsp.kotlin.completion import com.itsaky.androidide.lookup.Lookup import com.itsaky.androidide.lsp.api.describeSnippet import com.itsaky.androidide.lsp.kotlin.compiler.CompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.withAnalysisLock import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.AnalysisContext import com.itsaky.androidide.lsp.kotlin.utils.ContextKeywords @@ -149,41 +150,43 @@ internal fun doComplete(params: CompletionParams): CompletionResult { env.project.read { abortIfCancelled() - analyzeCopy( - useSiteElement = completionKtFile, - resolutionMode = KaDanglingFileResolutionMode.PREFER_SELF, - ) { - val ctx = - resolveAnalysisContext( - env = env, - file = params.file, - ktFile = completionKtFile, - offset = completionOffset, - partial = partial - ) - - if (ctx == null) { - logger.error( - "Unable to determine context at offset {} in file {}", - completionOffset, - params.file - ) - return@analyzeCopy CompletionResult.EMPTY - } - - abortIfCancelled() - context(ctx) { - val items = mutableListOf() - val completionContext = determineCompletionContext(ctx.psiElement) - when (completionContext) { - CompletionContext.Scope -> - collectScopeCompletions(to = items) - - CompletionContext.Member -> - collectMemberCompletions(to = items) + withAnalysisLock { + analyzeCopy( + useSiteElement = completionKtFile, + resolutionMode = KaDanglingFileResolutionMode.PREFER_SELF, + ) { + val ctx = + resolveAnalysisContext( + env = env, + file = params.file, + ktFile = completionKtFile, + offset = completionOffset, + partial = partial + ) + + if (ctx == null) { + logger.error( + "Unable to determine context at offset {} in file {}", + completionOffset, + params.file + ) + return@analyzeCopy CompletionResult.EMPTY } - CompletionResult(items) + abortIfCancelled() + context(ctx) { + val items = mutableListOf() + val completionContext = determineCompletionContext(ctx.psiElement) + when (completionContext) { + CompletionContext.Scope -> + collectScopeCompletions(to = items) + + CompletionContext.Member -> + collectMemberCompletions(to = items) + } + + CompletionResult(items) + } } } } From dc471d32a2557b68ca5626ff132f8d3bf513416b Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 19 Jun 2026 23:59:02 +0530 Subject: [PATCH 2/3] tests: add test case Signed-off-by: Akash Yadav --- .../modules/AnalysisSerializationTest.kt | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt new file mode 100644 index 0000000000..e2ffcd966c --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt @@ -0,0 +1,115 @@ +package com.itsaky.androidide.lsp.kotlin.compiler.modules + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.lsp.kotlin.compiler.read +import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import org.junit.Test +import java.util.Collections +import java.util.concurrent.atomic.AtomicInteger +import kotlin.math.max + +/** + * Regression tests for the `KaInaccessibleLifetimeOwnerAccessException: ... Called outside an + * `analyze` context.` reported in Sentry (APPDEVFORALL-VR / 7454434587). + * + * Root cause: indexing, diagnostics and completion all drove the stock Kotlin Analysis API + * concurrently from `Dispatchers.Default` threads (the modified-file indexer is debounced into + * independent coroutines), frequently against the same file. The Analysis API tracks its `analyze` + * lifetime context in a per-thread stack and is not safe to run concurrently without platform + * read-action coordination (which this LSP replaces with a shared read lock that does not serialize + * analysis). Overlapping `analyze` calls corrupted the lifetime/session lifecycle. + * + * Fix: [analyzeMaybeDangling] / [withAnalysisLock] hold a process-wide reentrant lock so analyses + * are mutually exclusive. + * + * Both tests fail before the fix (either by throwing the exception or by observing overlapping + * analyses) and pass after it. + */ +class AnalysisSerializationTest : KtLspTest() { + + @Test + fun `concurrent analyzeMaybeDangling never throws lifetime exception`(): Unit = runBlocking { + val files = (0 until 8).map { i -> + createSourceFile( + "Concurrent$i.kt", + """ + class Klass$i { + fun member$i(p: Int): Int = p + $i + val prop$i: String = "v$i" + } + + fun topLevel$i() = $i + """.trimIndent() + ) + } + + val errors = Collections.synchronizedList(mutableListOf()) + + // Many short, overlapping analyses on a high-parallelism dispatcher to reproduce the race. + coroutineScope { + repeat(240) { iter -> + launch(Dispatchers.IO) { + val file = files[iter % files.size] + try { + env.project.read { + analyzeMaybeDangling(file) { + // Touching declaration symbols is what triggered the lifetime check. + file.declarations.forEach { dcl -> + dcl.symbol + } + } + } + } catch (t: Throwable) { + errors.add(t) + } + } + } + } + + assertThat(errors).isEmpty() + } + + @Test + fun `analyzeMaybeDangling serializes overlapping analyses`(): Unit = runBlocking { + val files = (0 until 8).map { i -> + createSourceFile("Serialized$i.kt", "class S$i { fun f$i() = $i }") + } + + val inFlight = AtomicInteger(0) + val maxObserved = AtomicInteger(0) + val errors = Collections.synchronizedList(mutableListOf()) + + coroutineScope { + repeat(64) { iter -> + launch(Dispatchers.IO) { + val file = files[iter % files.size] + try { + env.project.read { + analyzeMaybeDangling(file) { + val concurrent = inFlight.incrementAndGet() + maxObserved.updateAndGet { max(it, concurrent) } + try { + file.declarations.forEach { it.symbol } + // Widen the window so any real overlap is observed. + Thread.sleep(2) + } finally { + inFlight.decrementAndGet() + } + } + } + } catch (t: Throwable) { + errors.add(t) + } + } + } + } + + assertThat(errors).isEmpty() + // The shared analysis lock must prevent two analyses from running at once. + assertThat(maxObserved.get()).isEqualTo(1) + } +} From 90e54711986bdb1c76c737aa6c3477516885db6b Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Tue, 23 Jun 2026 22:20:07 +0000 Subject: [PATCH 3/3] fix: address review findings (lifetime owner escape, write-lock race, completion cleanup) Apply the actionable items from Hal's review on PR #1428: - Diagnostics: extract the unresolved-reference name inside the analyze block instead of storing the live KaDiagnosticWithPsi (a KaLifetimeOwner) in DiagnosticItem.extra. AddImportAction now reads the pre-extracted string, preventing KaInaccessibleLifetimeOwnerAccessException from the quick-fix path. - CompilationEnvironment.notifyElementModifiedForPath: run handleElementModification inside project.write so the session mutation can't race a concurrent analyze (mirrors onFileContentChanged). - KotlinCompletions: collapse manual withAnalysisLock + analyzeCopy into analyzeMaybeDangling, removing the only in-prod direct analyzeCopy call. - KtFileExts: document that code under withAnalysisLock must not call project.write (non-upgradeable RW lock footgun). --- .../lsp/kotlin/actions/AddImportAction.kt | 12 ++-- .../kotlin/compiler/CompilationEnvironment.kt | 14 ++-- .../lsp/kotlin/compiler/modules/KtFileExts.kt | 6 ++ .../kotlin/completion/KotlinCompletions.kt | 69 +++++++++---------- .../diagnostic/KotlinDiagnosticProvider.kt | 16 ++++- 5 files changed, 64 insertions(+), 53 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt index 7d948477f7..3ca3e33683 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt @@ -18,7 +18,6 @@ import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.resources.R import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol -import org.jetbrains.kotlin.analysis.api.fir.diagnostics.KaFirDiagnostic import org.slf4j.LoggerFactory class AddImportAction : BaseKotlinCodeAction() { @@ -46,14 +45,13 @@ class AddImportAction : BaseKotlinCodeAction() { return } - val diagnostic = extra.diagnostic as? KaFirDiagnostic.UnresolvedReference? - if (diagnostic == null) { + val reference = extra.unresolvedReference + if (reference == null) { markInvisible() return } val env = extra.compilationEnv - val reference = diagnostic.reference val hasImportableSymbols = env.ktSymbolIndex .findSymbolBySimpleName(reference, limit = 0) .any { it.kind.isClassifier } @@ -65,10 +63,10 @@ class AddImportAction : BaseKotlinCodeAction() { } override suspend fun execAction(data: ActionData): Map> { - val (diagnostic, env) = data.require().extra as? KotlinDiagnosticExtra + val (reference, env) = data.require().extra as? KotlinDiagnosticExtra ?: return emptyMap() - diagnostic as KaFirDiagnostic.UnresolvedReference + if (reference == null) return emptyMap() val file = data.requireFile() val nioPath = file.toPath() @@ -77,7 +75,7 @@ class AddImportAction : BaseKotlinCodeAction() { ?: return emptyMap() return env.ktSymbolIndex - .findSymbolBySimpleName(diagnostic.reference, limit = 0) + .findSymbolBySimpleName(reference, limit = 0) .filter { it.kind.isClassifier } .associateWith { symbol -> insertImport(ktFile, symbol.fqName) } } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt index 92c01e5d73..fb3f2bd225 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt @@ -208,22 +208,24 @@ internal class CompilationEnvironment( @OptIn(KaImplementationDetail::class) private inline fun notifyElementModifiedForPath( path: Path, - typeProvider: (KtFile) -> KaElementModificationType, + crossinline typeProvider: (KtFile) -> KaElementModificationType, ) { val structureProvider = ProjectStructureProvider.getInstance(project) val ktFile = path.toVirtualFileOrNull()?.let { psiManager.findFile(it) as? KtFile } - if (ktFile != null) { - KaSourceModificationService.getInstance(project) - .handleElementModification(ktFile, typeProvider(ktFile)) - } - val module = (ktFile?.let { structureProvider.getModule(it, null) } ?: structureProvider.findModuleForSourceId(path.pathString)) as? AbstractKtModule project.write { + // Must run under the write lock so the session mutation can't race a concurrent + // `analyze` (which only holds the read lock); see onFileContentChanged. + if (ktFile != null) { + KaSourceModificationService.getInstance(project) + .handleElementModification(ktFile, typeProvider(ktFile)) + } + if (module != null) { module.invalidateSearchScope() project.publishModificationEvent( diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt index 5e530badcb..db20d93e91 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtFileExts.kt @@ -30,6 +30,12 @@ var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) * * Holding this lock around every analysis entry point makes analyses mutually exclusive. It is a * [ReentrantLock] so an (indirect) nested analysis on the same thread cannot deadlock. + * + * **Footgun:** analysis runs under the *read* (shared) side of the global + * [com.itsaky.androidide.lsp.kotlin.compiler.read] lock, and that `ReentrantReadWriteLock` is + * non-upgradeable. Code running inside [withAnalysisLock] / an `analyze` block must therefore never + * call [com.itsaky.androidide.lsp.kotlin.compiler.write] — upgrading read → write on the same thread + * deadlocks. */ private val analysisLock = ReentrantLock() diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt index 1058ecc330..d7e012f2ff 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt @@ -3,7 +3,7 @@ package com.itsaky.androidide.lsp.kotlin.completion import com.itsaky.androidide.lookup.Lookup import com.itsaky.androidide.lsp.api.describeSnippet import com.itsaky.androidide.lsp.kotlin.compiler.CompilationEnvironment -import com.itsaky.androidide.lsp.kotlin.compiler.modules.withAnalysisLock +import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.AnalysisContext import com.itsaky.androidide.lsp.kotlin.utils.ContextKeywords @@ -31,8 +31,6 @@ import org.jetbrains.kotlin.analysis.api.KaContextParameterApi import org.jetbrains.kotlin.analysis.api.KaExperimentalApi import org.jetbrains.kotlin.analysis.api.KaIdeApi import org.jetbrains.kotlin.analysis.api.KaSession -import org.jetbrains.kotlin.analysis.api.analyzeCopy -import org.jetbrains.kotlin.analysis.api.projectStructure.KaDanglingFileResolutionMode import org.jetbrains.kotlin.analysis.api.renderer.types.KaTypeRenderer import org.jetbrains.kotlin.analysis.api.renderer.types.impl.KaTypeRendererForSource import org.jetbrains.kotlin.analysis.api.symbols.KaCallableSymbol @@ -150,43 +148,38 @@ internal fun doComplete(params: CompletionParams): CompletionResult { env.project.read { abortIfCancelled() - withAnalysisLock { - analyzeCopy( - useSiteElement = completionKtFile, - resolutionMode = KaDanglingFileResolutionMode.PREFER_SELF, - ) { - val ctx = - resolveAnalysisContext( - env = env, - file = params.file, - ktFile = completionKtFile, - offset = completionOffset, - partial = partial - ) - - if (ctx == null) { - logger.error( - "Unable to determine context at offset {} in file {}", - completionOffset, - params.file - ) - return@analyzeCopy CompletionResult.EMPTY - } - - abortIfCancelled() - context(ctx) { - val items = mutableListOf() - val completionContext = determineCompletionContext(ctx.psiElement) - when (completionContext) { - CompletionContext.Scope -> - collectScopeCompletions(to = items) - - CompletionContext.Member -> - collectMemberCompletions(to = items) - } + analyzeMaybeDangling(completionKtFile) { + val ctx = + resolveAnalysisContext( + env = env, + file = params.file, + ktFile = completionKtFile, + offset = completionOffset, + partial = partial + ) + + if (ctx == null) { + logger.error( + "Unable to determine context at offset {} in file {}", + completionOffset, + params.file + ) + return@analyzeMaybeDangling CompletionResult.EMPTY + } - CompletionResult(items) + abortIfCancelled() + context(ctx) { + val items = mutableListOf() + val completionContext = determineCompletionContext(ctx.psiElement) + when (completionContext) { + CompletionContext.Scope -> + collectScopeCompletions(to = items) + + CompletionContext.Member -> + collectMemberCompletions(to = items) } + + CompletionResult(items) } } } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt index e529d029df..f9304a9230 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlin.analysis.api.KaExperimentalApi import org.jetbrains.kotlin.analysis.api.components.KaDiagnosticCheckerFilter import org.jetbrains.kotlin.analysis.api.diagnostics.KaDiagnosticWithPsi import org.jetbrains.kotlin.analysis.api.diagnostics.KaSeverity +import org.jetbrains.kotlin.analysis.api.fir.diagnostics.KaFirDiagnostic import org.jetbrains.kotlin.com.intellij.openapi.util.TextRange import org.jetbrains.kotlin.com.intellij.psi.PsiErrorElement import org.jetbrains.kotlin.com.intellij.psi.PsiFile @@ -23,7 +24,14 @@ import java.nio.file.Path private val logger = LoggerFactory.getLogger("KotlinDiagnosticProvider") internal data class KotlinDiagnosticExtra( - val diagnostic: KaDiagnosticWithPsi<*>, + /** + * The unresolved-reference name extracted from an [KaFirDiagnostic.UnresolvedReference] + * diagnostic, or `null` for any other diagnostic. This is plain data extracted *inside* the + * `analyze` block on purpose: storing the [KaDiagnosticWithPsi] (a `KaLifetimeOwner`) here and + * reading its members later from a code action would access it outside an `analyze` context and + * crash with `KaInaccessibleLifetimeOwnerAccessException`. + */ + val unresolvedReference: String?, val compilationEnv: CompilationEnvironment, ) @@ -79,8 +87,12 @@ private fun doAnalyze(file: Path, cancelChecker: ICancelChecker): DiagnosticResu ktFile.collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) .forEach { diagnostic -> cancelChecker.abortIfCancelled() + // Extract plain data while still inside the analyze context; never let + // the KaLifetimeOwner diagnostic escape (see KotlinDiagnosticExtra). + val unresolvedReference = + (diagnostic as? KaFirDiagnostic.UnresolvedReference)?.reference add(diagnostic.toDiagnosticItem().apply { - extra = KotlinDiagnosticExtra(diagnostic, env) + extra = KotlinDiagnosticExtra(unresolvedReference, env) }) } }