From d8c32bd068bbec0ebe9c33cb4ba0c567154cb393 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 19 Jun 2026 22:28:53 +0530 Subject: [PATCH 01/18] 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 02/18] 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 206c0c146dca4ad2bfd38310b6f639054fedf843 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Tue, 23 Jun 2026 22:20:07 +0000 Subject: [PATCH 03/18] 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) }) } } From f281d0e80644d42d4f2e6496babd2b10e1764491 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Tue, 23 Jun 2026 22:45:44 +0000 Subject: [PATCH 04/18] feat(ADFA-4174): priority-aware preemptive analysis scheduler Replace the FIFO analysis lock with AnalysisScheduler, a process-global, priority-aware, preemptive, reentrant lock that serializes all Kotlin Analysis API access while letting interactive work win. Priority order: Completion > Diagnostics > Indexing. - A higher-priority request preempts a strictly lower-priority in-progress analysis. Preemption is cooperative (the Analysis API can't be interrupted mid-analyze): the holder's ScheduledCancelChecker is flagged and the running analysis bails at its next abortIfCancelled() checkpoint with AnalysisPreemptedException. - A lower-priority request waits while a higher-priority one holds. - Preempted diagnostics/indexing work is auto-rescheduled so it still completes: diagnostics re-schedules via the fileAnalyzer; indexing re-queues the command. - Indexing is now actively preemptible (was ICancelChecker.NOOP). Wiring: - KotlinCompletions: COMPLETION priority, request-scoped checker from Lookup. - KotlinDiagnosticProvider: DIAGNOSTICS priority; CompilationEnvironment's fileAnalyzer catches AnalysisPreemptedException to re-schedule. - IndexWorker/SourceFileIndexer: INDEXING priority; re-queue on preemption. Tests: extend AnalysisSerializationTest with reentrancy, higher-preempts-lower, and lower-waits-for-higher cases (all 5 tests pass). --- .../kotlin/compiler/CompilationEnvironment.kt | 14 +- .../lsp/kotlin/compiler/index/IndexWorker.kt | 36 ++-- .../compiler/index/SourceFileIndexer.kt | 18 +- .../compiler/modules/AnalysisScheduler.kt | 159 ++++++++++++++++++ .../lsp/kotlin/compiler/modules/KtFileExts.kt | 41 +++-- .../kotlin/completion/KotlinCompletions.kt | 10 +- .../diagnostic/KotlinDiagnosticProvider.kt | 14 +- .../modules/AnalysisSerializationTest.kt | 110 +++++++++++- 8 files changed, 359 insertions(+), 43 deletions(-) create mode 100644 lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt 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 fb3f2bd225..a9a0eb89d0 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 @@ -3,6 +3,7 @@ package com.itsaky.androidide.lsp.kotlin.compiler import com.itsaky.androidide.lsp.api.ILanguageClient import com.itsaky.androidide.lsp.kotlin.compiler.index.KtSymbolIndex import com.itsaky.androidide.lsp.kotlin.compiler.modules.AbstractKtModule +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPreemptedException import com.itsaky.androidide.lsp.kotlin.compiler.modules.KtModule import com.itsaky.androidide.lsp.kotlin.compiler.modules.asFlatSequence import com.itsaky.androidide.lsp.kotlin.compiler.modules.backingFilePath @@ -166,9 +167,16 @@ internal class CompilationEnvironment( scope = coroutineScope, debounceDuration = DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION, ) { path, cancelChecker -> - val result = collectDiagnosticsFor(path, cancelChecker) - withContext(Dispatchers.Main.immediate) { - languageClient?.publishDiagnostics(result) + try { + val result = collectDiagnosticsFor(path, cancelChecker) + withContext(Dispatchers.Main.immediate) { + languageClient?.publishDiagnostics(result) + } + } catch (e: AnalysisPreemptedException) { + // A higher-priority analysis (completion) preempted this diagnostics run. + // Re-schedule so diagnostics still run once the higher-priority work finishes. + logger.debug("diagnostics for {} preempted; rescheduling", path) + fileAnalyzer.schedule(path) } } } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt index e27bd84f94..556912f4c9 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt @@ -1,6 +1,7 @@ package com.itsaky.androidide.lsp.kotlin.compiler.index import com.itsaky.androidide.lsp.kotlin.compiler.CompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPreemptedException import com.itsaky.androidide.lsp.kotlin.compiler.modules.backingFilePath import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.progress.ICancelChecker @@ -8,6 +9,7 @@ import com.itsaky.androidide.utils.KeyedDebouncingAction import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch import org.appdevforall.codeonthego.indexing.jvm.JvmSymbolIndex import org.appdevforall.codeonthego.indexing.jvm.KtFileMetadata import org.appdevforall.codeonthego.indexing.jvm.KtFileMetadataIndex @@ -54,8 +56,14 @@ internal class IndexWorker( debounceDuration = CompilationEnvironment.DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION ) { (path, ktFile), cancelChecker -> logger.debug("Indexing modified file: {}", path) - indexSourceFile(project, ktFile, fileIndex, sourceIndex, cancelChecker) - sourceIndexCount++ + try { + indexSourceFile(project, ktFile, fileIndex, sourceIndex, cancelChecker) + sourceIndexCount++ + } catch (e: AnalysisPreemptedException) { + // Preempted by higher-priority analysis; re-queue so the edit still gets indexed. + logger.debug("Indexing of modified file {} preempted; re-queueing", path) + scope.launch { submitCommand(IndexCommand.IndexModifiedFile(ktFile)) } + } } while (isActive) { @@ -82,15 +90,23 @@ internal class IndexWorker( continue } - indexSourceFile( - project = project, - ktFile = ktFile, - fileIndex = fileIndex, - symbolsIndex = sourceIndex, - cancelChecker = ICancelChecker.NOOP - ) + try { + indexSourceFile( + project = project, + ktFile = ktFile, + fileIndex = fileIndex, + symbolsIndex = sourceIndex, + // A real (cancellable) checker so the scheduler can preempt this pass + // in favour of completion/diagnostics. + cancelChecker = ICancelChecker.Default() + ) - sourceIndexCount++ + sourceIndexCount++ + } catch (e: AnalysisPreemptedException) { + // Preempted by higher-priority analysis; re-queue so the file still gets indexed. + logger.debug("Indexing of {} preempted; re-queueing", cmd.vf.path) + scope.launch { submitCommand(cmd) } + } } is IndexCommand.IndexModifiedFile -> { diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt index 6d38dee820..11b73381e1 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt @@ -1,5 +1,7 @@ package com.itsaky.androidide.lsp.kotlin.compiler.index +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.modules.backingFilePath import com.itsaky.androidide.lsp.kotlin.compiler.read @@ -74,9 +76,15 @@ internal suspend fun indexSourceFile( symbolsIndex: JvmSymbolIndex, cancelChecker: ICancelChecker, ) { + // Indexing runs at the lowest (INDEXING) priority: it yields to both completion and diagnostics. + // Wrapping the checker lets the scheduler preempt an in-progress index pass; the preemption + // surfaces as AnalysisPreemptedException at the abortIfCancelled() checkpoints below, which the + // IndexWorker catches to re-queue the file. + val checker = cancelChecker as? ScheduledCancelChecker ?: ScheduledCancelChecker(cancelChecker) + val newFile = ktFile.toMetadata(project, isIndexed = true) val existingFile = fileIndex.get(newFile.filePath) - cancelChecker.abortIfCancelled() + checker.abortIfCancelled() if (KtFileMetadata.shouldBeSkipped(existingFile, newFile) && existingFile?.isIndexed == true) { return @@ -85,18 +93,18 @@ internal suspend fun indexSourceFile( // Remove stale symbols written during the previous indexing pass. if (existingFile?.isIndexed == true) { symbolsIndex.removeBySource(newFile.filePath) - cancelChecker.abortIfCancelled() + checker.abortIfCancelled() } val symbols = project.read { val list = mutableListOf() - analyzeMaybeDangling(ktFile) { + analyzeMaybeDangling(ktFile, AnalysisPriority.INDEXING, checker) { val session = this ktFile.accept(object : KtTreeVisitorVoid() { override fun visitDeclaration(dcl: KtDeclaration) { - cancelChecker.abortIfCancelled() + checker.abortIfCancelled() val symbol = with(session) { analyzeDeclaration(newFile.filePath, dcl) } - cancelChecker.abortIfCancelled() + checker.abortIfCancelled() symbol?.let { list.add(it) } super.visitDeclaration(dcl) } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt new file mode 100644 index 0000000000..42b7ba9d96 --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -0,0 +1,159 @@ +package com.itsaky.androidide.lsp.kotlin.compiler.modules + +import com.itsaky.androidide.progress.ICancelChecker +import java.util.concurrent.CancellationException +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock + +/** + * Priority of an Analysis API request. Higher [ordinal] wins: a request can preempt any strictly + * lower-priority analysis that is currently running, and is served before any lower-priority request + * that is merely waiting. + * + * Order: [INDEXING] < [DIAGNOSTICS] < [COMPLETION] — interactive completion beats background + * diagnostics, which beats bulk indexing. + */ +internal enum class AnalysisPriority { + INDEXING, + DIAGNOSTICS, + COMPLETION, +} + +/** + * Thrown at an `abortIfCancelled()` checkpoint when the running analysis has been preempted by a + * higher-priority request (see [AnalysisScheduler]). It is a [CancellationException] so it unwinds + * cleanly through the existing cancellation-aware `catch` blocks; callers that want the preempted work + * to run later catch this specific type and re-schedule it. + */ +internal class AnalysisPreemptedException : + CancellationException("analysis preempted by a higher-priority request") + +/** + * An [ICancelChecker] that adds a cooperative *preemption* signal on top of an existing [delegate] + * checker. The Analysis API cannot be interrupted mid-`analyze` (it runs with a no-op + * `ProgressManager`), so [AnalysisScheduler] flags preemption here and the running analysis notices it + * at its next [abortIfCancelled] checkpoint. + * + * Preemption is distinct from ordinary cancellation: [abortIfCancelled] throws + * [AnalysisPreemptedException] (so the source can re-schedule the work) while still honouring the + * delegate's own cancellation (e.g. a superseding edit or a closed file). + */ +internal class ScheduledCancelChecker( + private val delegate: ICancelChecker, +) : ICancelChecker { + + @Volatile + private var preempted = false + + /** Marks this analysis as preempted; the next [abortIfCancelled] will throw. */ + fun preempt() { + preempted = true + } + + override fun cancel() { + delegate.cancel() + } + + override fun isCancelled(): Boolean = preempted || delegate.isCancelled() + + override fun abortIfCancelled() { + if (preempted) { + throw AnalysisPreemptedException() + } + delegate.abortIfCancelled() + } +} + +/** + * A process-global, priority-aware, preemptive lock that serializes all Kotlin Analysis API access. + * + * It replaces the plain FIFO lock that previously guarded `analyze` / `analyzeCopy`. Semantics: + * - only one analysis runs at a time (the Analysis API is not safe to drive concurrently); + * - a higher-priority requester **preempts** a strictly lower-priority holder by invoking its + * `onPreempt` callback once (cooperative — the holder bails at its next `abortIfCancelled()`); + * - when the lock frees, the highest-priority waiter acquires it next; + * - it is **reentrant**: a nested analysis on the same thread re-enters without deadlocking. + * + * Access it through [withAnalysisLock] / [analyzeMaybeDangling] rather than directly. + */ +internal object AnalysisScheduler { + + private val mutex = ReentrantLock() + private val available = mutex.newCondition() + + private var holderThread: Thread? = null + private var holderPriority: AnalysisPriority? = null + private var holderReentry = 0 + private var holderPreempted = false + private var holderPreempt: (() -> Unit)? = null + + /** Number of threads currently waiting to acquire, per priority. */ + private val waiting = IntArray(AnalysisPriority.entries.size) + + /** + * Acquire the analysis lock at the given [priority]. Blocks until the current thread may run. If a + * strictly lower-priority analysis is in progress, [onPreempt] of *that* holder is invoked so it + * yields; [onPreempt] passed here is stored and used if this acquisition is later preempted. + */ + fun acquire(priority: AnalysisPriority, onPreempt: () -> Unit) { + mutex.withLock { + val me = Thread.currentThread() + if (holderThread === me) { + // Reentrant: nested analysis on the same thread shares the outer hold. + holderReentry++ + return + } + + waiting[priority.ordinal]++ + try { + while (true) { + val hp = holderPriority + if (holderThread != null && hp != null && + hp.ordinal < priority.ordinal && !holderPreempted + ) { + // Signal the lower-priority holder to bail (once). + holderPreempted = true + holderPreempt?.invoke() + } + + if (holderThread == null && !higherPriorityWaiting(priority)) { + break + } + available.await() + } + } finally { + waiting[priority.ordinal]-- + } + + holderThread = me + holderPriority = priority + holderPreempt = onPreempt + holderPreempted = false + holderReentry = 1 + } + } + + /** Release a hold acquired via [acquire]. Wakes waiters when the outermost hold is released. */ + fun release() { + mutex.withLock { + if (holderThread !== Thread.currentThread()) { + return + } + if (--holderReentry > 0) { + return + } + holderThread = null + holderPriority = null + holderPreempt = null + holderPreempted = false + available.signalAll() + } + } + + private fun higherPriorityWaiting(priority: AnalysisPriority): Boolean { + for (i in priority.ordinal + 1 until waiting.size) { + if (waiting[i] > 0) return true + } + return false + } +} 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 db20d93e91..260e99eb0e 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,12 @@ 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) /** - * Serializes all Kotlin Analysis API access (`analyze` / `analyzeCopy`). + * Runs [action] while holding the global analysis lock at the given [priority]. * * 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 @@ -28,8 +26,12 @@ var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) * 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. + * Serialization is handled by [AnalysisScheduler], which is priority-aware and preemptive: a + * higher-priority request preempts a strictly lower-priority one (cooperatively, via [cancelChecker]). + * The scheduler is reentrant, so an (indirect) nested analysis on the same thread cannot deadlock. + * + * **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. * * **Footgun:** analysis runs under the *read* (shared) side of the global * [com.itsaky.androidide.lsp.kotlin.compiler.read] lock, and that `ReentrantReadWriteLock` is @@ -37,17 +39,26 @@ var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) * call [com.itsaky.androidide.lsp.kotlin.compiler.write] — upgrading read → write on the same thread * deadlocks. */ -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) +internal inline fun withAnalysisLock( + priority: AnalysisPriority, + cancelChecker: ScheduledCancelChecker, + action: () -> R, +): R { + AnalysisScheduler.acquire(priority, cancelChecker::preempt) + try { + return action() + } finally { + AnalysisScheduler.release() + } +} -internal inline fun analyzeMaybeDangling(useSiteElement: KtElement, crossinline action: KaSession.() -> R): R = - withAnalysisLock { +internal inline fun analyzeMaybeDangling( + useSiteElement: KtElement, + priority: AnalysisPriority, + cancelChecker: ScheduledCancelChecker, + crossinline action: KaSession.() -> R, +): R = + withAnalysisLock(priority, cancelChecker) { if (useSiteElement is KtFile && useSiteElement.isDangling && useSiteElement.copyOrigin != null) { analyzeCopy(useSiteElement, KaDanglingFileResolutionMode.PREFER_SELF, action) } else { 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 d7e012f2ff..c8617c4732 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,8 @@ 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.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker 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 @@ -144,11 +146,17 @@ internal fun doComplete(params: CompletionParams): CompletionResult { abortIfCancelled() + // Completion is the highest-priority analysis: it preempts in-progress diagnostics/indexing and + // is never itself preempted. The cancel checker is the editor's request-scoped one (from Lookup). + val cancelChecker = ScheduledCancelChecker( + Lookup.getDefault().lookup(ICancelChecker::class.java) ?: ICancelChecker.NOOP + ) + return try { env.project.read { abortIfCancelled() - analyzeMaybeDangling(completionKtFile) { + analyzeMaybeDangling(completionKtFile, AnalysisPriority.COMPLETION, cancelChecker) { val ctx = resolveAnalysisContext( env = env, 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 f9304a9230..2a14070ed7 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 @@ -1,6 +1,8 @@ package com.itsaky.androidide.lsp.kotlin.diagnostic import com.itsaky.androidide.lsp.kotlin.compiler.CompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.toRange @@ -64,11 +66,17 @@ private fun doAnalyze(file: Path, cancelChecker: ICancelChecker): DiagnosticResu return DiagnosticResult.NO_UPDATE } + // Diagnostics run at DIAGNOSTICS priority: they yield to completion but preempt indexing. The + // wrapped checker turns a scheduler preemption into an AnalysisPreemptedException, which + // CompilationEnvironment's fileAnalyzer catches to re-schedule this run after the higher-priority + // work finishes. + val checker = ScheduledCancelChecker(cancelChecker) + val diagnostics = env.project.read { buildList { PsiTreeUtil.collectElementsOfType(ktFile, PsiErrorElement::class.java) .forEach { errorElement -> - cancelChecker.abortIfCancelled() + checker.abortIfCancelled() add( diagnosticItem( file = ktFile, @@ -83,10 +91,10 @@ private fun doAnalyze(file: Path, cancelChecker: ICancelChecker): DiagnosticResu // The analysis API uses a no-op implementation of // Intellij's ProgressManager for cancellations, so the following // isn't really cancellable at the moment - analyzeMaybeDangling(ktFile) { + analyzeMaybeDangling(ktFile, AnalysisPriority.DIAGNOSTICS, checker) { ktFile.collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) .forEach { diagnostic -> - cancelChecker.abortIfCancelled() + checker.abortIfCancelled() // Extract plain data while still inside the analyze context; never let // the KaLifetimeOwner diagnostic escape (see KotlinDiagnosticExtra). val unresolvedReference = 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 index e2ffcd966c..0cea853185 100644 --- 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 @@ -3,12 +3,16 @@ 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 com.itsaky.androidide.progress.ICancelChecker 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.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger import kotlin.math.max @@ -23,11 +27,12 @@ import kotlin.math.max * 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. + * Fix: [analyzeMaybeDangling] / [withAnalysisLock] route through [AnalysisScheduler], a process-wide + * priority-aware, preemptive, 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. + * The first two tests cover serialization (they fail before the fix, either by throwing the exception + * or by observing overlapping analyses). The remaining tests cover the scheduler's priority, + * preemption, and reentrancy behaviour. */ class AnalysisSerializationTest : KtLspTest() { @@ -56,7 +61,11 @@ class AnalysisSerializationTest : KtLspTest() { val file = files[iter % files.size] try { env.project.read { - analyzeMaybeDangling(file) { + analyzeMaybeDangling( + file, + AnalysisPriority.DIAGNOSTICS, + ScheduledCancelChecker(ICancelChecker.NOOP), + ) { // Touching declaration symbols is what triggered the lifetime check. file.declarations.forEach { dcl -> dcl.symbol @@ -89,7 +98,11 @@ class AnalysisSerializationTest : KtLspTest() { val file = files[iter % files.size] try { env.project.read { - analyzeMaybeDangling(file) { + analyzeMaybeDangling( + file, + AnalysisPriority.DIAGNOSTICS, + ScheduledCancelChecker(ICancelChecker.NOOP), + ) { val concurrent = inFlight.incrementAndGet() maxObserved.updateAndGet { max(it, concurrent) } try { @@ -112,4 +125,89 @@ class AnalysisSerializationTest : KtLspTest() { // The shared analysis lock must prevent two analyses from running at once. assertThat(maxObserved.get()).isEqualTo(1) } + + @Test(timeout = 10_000) + fun `reentrant withAnalysisLock on the same thread does not deadlock`() { + var innerRan = false + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + innerRan = true + } + } + assertThat(innerRan).isTrue() + } + + @Test(timeout = 10_000) + fun `higher priority request preempts a lower priority holder`() { + val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + val holding = CountDownLatch(1) + val preempted = AtomicBoolean(false) + val higherRan = AtomicBoolean(false) + + // Low-priority (indexing) holder runs a long, cooperatively-cancellable analysis. + val lower = Thread { + try { + withAnalysisLock(AnalysisPriority.INDEXING, holderChecker) { + holding.countDown() + repeat(2_000) { + holderChecker.abortIfCancelled() + Thread.sleep(5) + } + } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) + } + } + lower.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + // A completion request must preempt the in-progress indexing. + val higher = Thread { + withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + higherRan.set(true) + } + } + higher.start() + higher.join(5_000) + lower.join(5_000) + + assertThat(preempted.get()).isTrue() + assertThat(higherRan.get()).isTrue() + } + + @Test(timeout = 10_000) + fun `lower priority request waits while a higher priority holder runs`() { + val holding = CountDownLatch(1) + val release = CountDownLatch(1) + val lowerEntered = AtomicBoolean(false) + + // High-priority (completion) holder holds the lock until released. + val higher = Thread { + withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + holding.countDown() + release.await() + } + } + higher.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + // A diagnostics request is strictly lower priority: it must not preempt completion. + val lower = Thread { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + lowerEntered.set(true) + } + } + lower.start() + + // Give the lower-priority request time to (incorrectly) barge in. + Thread.sleep(300) + val enteredWhileHeld = lowerEntered.get() + + release.countDown() + higher.join(5_000) + lower.join(5_000) + + assertThat(enteredWhileHeld).isFalse() + assertThat(lowerEntered.get()).isTrue() + } } From c9349b0ce850942369971f87a79f5c8e2d8a9cd8 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Thu, 2 Jul 2026 11:31:07 +0000 Subject: [PATCH 05/18] fix: integrate cancel checker with ProgressManager --- .../compiler/modules/AnalysisScheduler.kt | 7 +-- .../modules/CancelCheckerProgressIndicator.kt | 37 ++++++++++++++++ .../lsp/kotlin/compiler/modules/KtFileExts.kt | 38 ++++++++++++++-- .../diagnostic/KotlinDiagnosticProvider.kt | 7 ++- .../modules/AnalysisSerializationTest.kt | 44 +++++++++++++++++++ 5 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index 42b7ba9d96..219a1ebc9b 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -30,9 +30,10 @@ internal class AnalysisPreemptedException : /** * An [ICancelChecker] that adds a cooperative *preemption* signal on top of an existing [delegate] - * checker. The Analysis API cannot be interrupted mid-`analyze` (it runs with a no-op - * `ProgressManager`), so [AnalysisScheduler] flags preemption here and the running analysis notices it - * at its next [abortIfCancelled] checkpoint. + * checker. [AnalysisScheduler] flags preemption here; the running analysis observes it both at its + * LSP-level [abortIfCancelled] checkpoints and, because [withAnalysisLock] installs a + * [CancelCheckerProgressIndicator] bridging [isCancelled] to the compiler's `ProgressManager`, + * mid-`analyze` at the compiler's own internal cancellation checks. * * Preemption is distinct from ordinary cancellation: [abortIfCancelled] throws * [AnalysisPreemptedException] (so the source can re-schedule the work) while still honouring the diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt new file mode 100644 index 0000000000..7c47d33034 --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt @@ -0,0 +1,37 @@ +package com.itsaky.androidide.lsp.kotlin.compiler.modules + +import com.itsaky.androidide.progress.ICancelChecker +import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException +import org.jetbrains.kotlin.com.intellij.openapi.progress.util.AbstractProgressIndicatorBase + +/** + * A [com.intellij.openapi.progress.ProgressIndicator] whose cancellation state is driven by an + * [ICancelChecker]. Installing it as the analysis thread's indicator (via + * [withAnalysisLock]) is what makes the Kotlin Analysis API actually interruptible *mid*-`analyze`. + * + * The embeddable analysis API ships the full IntelliJ `CoreProgressManager`, whose + * `ProgressManager.checkCanceled()` (called densely throughout FIR resolution) re-fetches the + * current thread's indicator and rethrows its [checkCanceled]. By bridging that to [checker], a + * preemption or ordinary cancellation aborts the running analysis at the compiler's next internal + * checkpoint instead of only at the coarse LSP-level [ICancelChecker.abortIfCancelled] checks. + * + * This extends [AbstractProgressIndicatorBase] rather than `EmptyProgressIndicator` on purpose: + * the base is a *non-standard* indicator, so `CoreProgressManager` runs a background task that + * polls [checkCanceled] every ~10ms. That poll flips the manager's internal "should check + * cancelled" flag to active once [checker] reports cancellation, which is what arms the in-`analyze` + * checks. [cancel] (invoked synchronously when this analysis is preempted) flips the same flag + * immediately, so preemption does not have to wait for the poll. + */ +internal class CancelCheckerProgressIndicator( + private val checker: ICancelChecker, +) : AbstractProgressIndicatorBase() { + + override fun isCanceled(): Boolean = super.isCanceled() || checker.isCancelled() + + override fun checkCanceled() { + if (checker.isCancelled()) { + throw ProcessCanceledException() + } + super.checkCanceled() + } +} 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 260e99eb0e..f9ca1d8720 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 @@ -6,15 +6,20 @@ import org.jetbrains.kotlin.analysis.api.analyzeCopy import org.jetbrains.kotlin.analysis.api.projectStructure.KaDanglingFileResolutionMode import org.jetbrains.kotlin.analysis.api.projectStructure.copyOrigin import org.jetbrains.kotlin.analysis.api.projectStructure.isDangling +import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException +import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager import org.jetbrains.kotlin.com.intellij.openapi.util.Key import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.UserDataProperty +import org.slf4j.LoggerFactory import java.nio.file.Path private val KT_LSP_COMPLETION_BACKING_FILE = Key("KT_LSP_COMPLETION_BACKING_FILE") var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) +private val logger = LoggerFactory.getLogger("KtFileExts") + /** * Runs [action] while holding the global analysis lock at the given [priority]. * @@ -33,6 +38,15 @@ var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) * **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. * + * **Cancellation.** The [action] runs under a [CancelCheckerProgressIndicator] installed as the + * thread's IntelliJ progress indicator, so the compiler's own `ProgressManager.checkCanceled()` + * calls (dense throughout FIR resolution) abort the analysis *mid*-`analyze` once [cancelChecker] + * reports preemption or cancellation — not just at the coarse LSP [ScheduledCancelChecker.abortIfCancelled] + * checkpoints between work units. The compiler signals this by throwing [ProcessCanceledException]; + * we translate it back into the typed exception the callers already handle + * ([AnalysisPreemptedException] when preempted, else the delegate's `CancellationException`) so + * preempted work is rescheduled rather than silently dropped. + * * **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 @@ -42,11 +56,29 @@ var KtFile.backingFilePath by UserDataProperty(KT_LSP_COMPLETION_BACKING_FILE) internal inline fun withAnalysisLock( priority: AnalysisPriority, cancelChecker: ScheduledCancelChecker, - action: () -> R, + crossinline action: () -> R, ): R { - AnalysisScheduler.acquire(priority, cancelChecker::preempt) + val indicator = CancelCheckerProgressIndicator(cancelChecker) + // When this analysis is preempted, also cancel the indicator so the compiler's in-`analyze` + // cancellation checks fire immediately instead of waiting for the manager's background poll. + AnalysisScheduler.acquire(priority) { + cancelChecker.preempt() + indicator.cancel() + } try { - return action() + val holder = arrayOfNulls(1) + try { + ProgressManager.getInstance() + .executeProcessUnderProgress({ holder[0] = action() }, indicator) + } catch (e: ProcessCanceledException) { + logger.debug("process cancelled: prio={}", priority) + // Re-derive the semantically-correct exception callers expect (re-throws + // AnalysisPreemptedException when preempted, or the delegate's CancellationException). + cancelChecker.abortIfCancelled() + throw e + } + @Suppress("UNCHECKED_CAST") + return holder[0] as R } finally { AnalysisScheduler.release() } 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 2a14070ed7..c60bece3fe 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 @@ -87,10 +87,9 @@ private fun doAnalyze(file: Path, cancelChecker: ICancelChecker): DiagnosticResu ) } - // This should be canceled as well - // The analysis API uses a no-op implementation of - // Intellij's ProgressManager for cancellations, so the following - // isn't really cancellable at the moment + // analyzeMaybeDangling installs a CancelCheckerProgressIndicator, so this analysis is + // cancellable mid-`analyze`: it aborts at the compiler's internal checkCanceled() once + // `checker` reports preemption/cancellation (in addition to the abortIfCancelled() below). analyzeMaybeDangling(ktFile, AnalysisPriority.DIAGNOSTICS, checker) { ktFile.collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) .forEach { diagnostic -> 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 index 0cea853185..7b5ef23b05 100644 --- 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 @@ -4,6 +4,7 @@ import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import com.itsaky.androidide.progress.ICancelChecker +import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch @@ -175,6 +176,49 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(higherRan.get()).isTrue() } + @Test(timeout = 10_000) + fun `analysis is interrupted mid-analyze at the compiler's ProgressManager checkpoint`() { + // The Kotlin Analysis API calls ProgressManager.checkCanceled() densely during FIR + // resolution, but never the LSP-level ICancelChecker.abortIfCancelled(). This body mimics + // that: it only polls ProgressManager.checkCanceled(). Before withAnalysisLock installed a + // CancelCheckerProgressIndicator, that call was inert (no indicator => the manager's + // check-cancelled behaviour stayed disabled) and the work ran to completion regardless of + // preemption. It must now be interruptible. + val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + val holding = CountDownLatch(1) + val preempted = AtomicBoolean(false) + val ranToCompletion = AtomicBoolean(false) + + val lower = Thread { + try { + withAnalysisLock(AnalysisPriority.INDEXING, holderChecker) { + holding.countDown() + repeat(2_000) { + // Compiler-level checkpoint only — no abortIfCancelled() here. + ProgressManager.checkCanceled() + Thread.sleep(5) + } + ranToCompletion.set(true) + } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) + } + } + lower.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + // A completion request preempts the in-progress (indexing) analysis. + val higher = Thread { + withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) {} + } + higher.start() + higher.join(5_000) + lower.join(5_000) + + assertThat(preempted.get()).isTrue() + assertThat(ranToCompletion.get()).isFalse() + } + @Test(timeout = 10_000) fun `lower priority request waits while a higher priority holder runs`() { val holding = CountDownLatch(1) From 959e0f0b7ea9fde00abe1b778431312c52f492e7 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Thu, 2 Jul 2026 18:41:43 +0530 Subject: [PATCH 06/18] fix: allow same-priority pre-emption for code completion requests Signed-off-by: Akash Yadav --- .claude/.gitignore | 12 +++ build.gradle.kts | 6 ++ .../compiler/modules/AnalysisScheduler.kt | 33 +++++--- .../kotlin/completion/KotlinCompletions.kt | 6 +- .../modules/AnalysisSerializationTest.kt | 75 +++++++++++++++++++ 5 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 .claude/.gitignore diff --git a/.claude/.gitignore b/.claude/.gitignore new file mode 100644 index 0000000000..6d88c79ccb --- /dev/null +++ b/.claude/.gitignore @@ -0,0 +1,12 @@ +# Local, machine-specific settings (not shared) +settings.local.json + +# Claude Code worktrees +worktrees/ + +# Session/runtime artifacts +projects/ +todos/ +shell-snapshots/ +statsig/ +.credentials.json diff --git a/build.gradle.kts b/build.gradle.kts index 142e2c4ca4..8bb596384a 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -97,11 +97,17 @@ subprojects { // setAccessible on java.lang.Class fields. // - java.base/java.io, java.util: needed by Robolectric/Gradle worker // reflection in the same test JVM. + // - java.base/java.util.concurrent: the embedded IntelliJ scheduler + // (BoundedTaskExecutor.info -> AppDelayQueue "Periodic tasks thread") + // reflectively reads FutureTask.callable. Without this the periodic + // thread dies, disabling CoreProgressManager's cancellation poll that + // makes the Kotlin Analysis API interruptible mid-`analyze`. jvmArgs( "--add-opens=java.base/java.lang=ALL-UNNAMED", "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", "--add-opens=java.base/java.io=ALL-UNNAMED", "--add-opens=java.base/java.util=ALL-UNNAMED", + "--add-opens=java.base/java.util.concurrent=ALL-UNNAMED", "--add-opens=jdk.unsupported/sun.misc=ALL-UNNAMED", ) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index 219a1ebc9b..aaaad78169 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -12,11 +12,20 @@ import kotlin.concurrent.withLock * * Order: [INDEXING] < [DIAGNOSTICS] < [COMPLETION] — interactive completion beats background * diagnostics, which beats bulk indexing. + * + * [supersedesSamePriority] additionally lets a *newer* request preempt an in-flight request of the + * **same** priority. This is enabled only for [COMPLETION]: when the user types fast, several + * completion requests fire in a row and the in-flight one is computing results for a now-stale cursor + * position, so the newer request cancels it and the fresh position is analysed immediately. A + * superseded completion is simply *discarded* — nothing reschedules it (see + * `KotlinCompletions.codeComplete`). It is intentionally off for [DIAGNOSTICS] and [INDEXING], whose + * preempted work is re-queued rather than dropped; same-priority preemption there would livelock, as + * two contenders would endlessly re-queue and re-preempt each other. */ -internal enum class AnalysisPriority { - INDEXING, - DIAGNOSTICS, - COMPLETION, +internal enum class AnalysisPriority(val supersedesSamePriority: Boolean) { + INDEXING(supersedesSamePriority = false), + DIAGNOSTICS(supersedesSamePriority = false), + COMPLETION(supersedesSamePriority = true), } /** @@ -72,6 +81,9 @@ internal class ScheduledCancelChecker( * - only one analysis runs at a time (the Analysis API is not safe to drive concurrently); * - a higher-priority requester **preempts** a strictly lower-priority holder by invoking its * `onPreempt` callback once (cooperative — the holder bails at its next `abortIfCancelled()`); + * - a newer requester of the **same** priority likewise preempts the holder when that priority is + * [AnalysisPriority.supersedesSamePriority] (completion only — its superseded work is discarded, not + * rescheduled); * - when the lock frees, the highest-priority waiter acquires it next; * - it is **reentrant**: a nested analysis on the same thread re-enters without deadlocking. * @@ -93,8 +105,9 @@ internal object AnalysisScheduler { /** * Acquire the analysis lock at the given [priority]. Blocks until the current thread may run. If a - * strictly lower-priority analysis is in progress, [onPreempt] of *that* holder is invoked so it - * yields; [onPreempt] passed here is stored and used if this acquisition is later preempted. + * preemptable analysis is in progress — strictly lower priority, or the same priority when that + * priority is [AnalysisPriority.supersedesSamePriority] — [onPreempt] of *that* holder is invoked so + * it yields; [onPreempt] passed here is stored and used if this acquisition is later preempted. */ fun acquire(priority: AnalysisPriority, onPreempt: () -> Unit) { mutex.withLock { @@ -109,10 +122,12 @@ internal object AnalysisScheduler { try { while (true) { val hp = holderPriority - if (holderThread != null && hp != null && - hp.ordinal < priority.ordinal && !holderPreempted + if (holderThread != null && hp != null && !holderPreempted && + (hp.ordinal < priority.ordinal || + (hp == priority && priority.supersedesSamePriority)) ) { - // Signal the lower-priority holder to bail (once). + // Signal the holder to bail (once): either it is strictly lower priority, or a + // newer same-priority request supersedes it (completion only). holderPreempted = true holderPreempt?.invoke() } 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 c8617c4732..0370632f71 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 @@ -146,8 +146,10 @@ internal fun doComplete(params: CompletionParams): CompletionResult { abortIfCancelled() - // Completion is the highest-priority analysis: it preempts in-progress diagnostics/indexing and - // is never itself preempted. The cancel checker is the editor's request-scoped one (from Lookup). + // Completion is the highest-priority analysis: it preempts in-progress diagnostics/indexing and is + // never preempted by lower-priority analysis. It can, however, be superseded by a *newer* completion + // request (the user typing on) — that in-flight completion is then cancelled and simply discarded. + // The cancel checker is the editor's request-scoped one (from Lookup). val cancelChecker = ScheduledCancelChecker( Lookup.getDefault().lookup(ICancelChecker::class.java) ?: ICancelChecker.NOOP ) 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 index 7b5ef23b05..fe80666dda 100644 --- 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 @@ -254,4 +254,79 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(enteredWhileHeld).isFalse() assertThat(lowerEntered.get()).isTrue() } + + @Test(timeout = 10_000) + fun `same priority completion supersedes an in-flight completion`() { + val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + val holding = CountDownLatch(1) + val preempted = AtomicBoolean(false) + val newerRan = AtomicBoolean(false) + + // An in-flight completion runs a long, cooperatively-cancellable analysis. + val older = Thread { + try { + withAnalysisLock(AnalysisPriority.COMPLETION, holderChecker) { + holding.countDown() + repeat(2_000) { + holderChecker.abortIfCancelled() + Thread.sleep(5) + } + } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) + } + } + older.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + // A newer completion request (user typed on) must supersede the in-flight one. + val newer = Thread { + withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + newerRan.set(true) + } + } + newer.start() + newer.join(5_000) + older.join(5_000) + + assertThat(preempted.get()).isTrue() + assertThat(newerRan.get()).isTrue() + } + + @Test(timeout = 10_000) + fun `same priority diagnostics does not preempt an in-flight diagnostics`() { + val holding = CountDownLatch(1) + val release = CountDownLatch(1) + val secondEntered = AtomicBoolean(false) + + // A diagnostics holder holds the lock until released. + val first = Thread { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + holding.countDown() + release.await() + } + } + first.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + // A second diagnostics request is the same priority but must NOT supersede the holder + // (only completion supersedes same-priority work); it waits until the holder releases. + val second = Thread { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + secondEntered.set(true) + } + } + second.start() + + // Give the second request time to (incorrectly) barge in. + Thread.sleep(300) + val enteredWhileHeld = secondEntered.get() + + release.countDown() + first.join(5_000) + second.join(5_000) + + assertThat(enteredWhileHeld).isFalse() + assertThat(secondEntered.get()).isTrue() + } } From b17aabdbf0b51c15af89384a84645f03bc43390a Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 3 Jul 2026 15:03:56 +0000 Subject: [PATCH 07/18] feat: add push cancellation and thread registration to shared cancel checker ICancelChecker.invokeOnCancel(listener) pushes on cancel (fire-once, immediate if already cancelled, no-op on NOOP). ProgressManager gains register/unregister so cancel(thread) flips the request's own checker. Adds ICancelCheckerTest and ProgressManagerTest. --- .../androidide/progress/ICancelChecker.kt | 40 +++++++- .../androidide/progress/ProgressManager.kt | 35 +++++-- .../androidide/progress/ICancelCheckerTest.kt | 98 +++++++++++++++++++ .../progress/ProgressManagerTest.kt | 58 +++++++++++ 4 files changed, 222 insertions(+), 9 deletions(-) create mode 100644 shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt create mode 100644 shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt index 059824966f..ce0bcb1974 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt @@ -18,6 +18,7 @@ package com.itsaky.androidide.progress import java.util.concurrent.CancellationException +import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.atomic.AtomicBoolean /** @@ -45,12 +46,31 @@ interface ICancelChecker { @Throws(CancellationException::class) fun abortIfCancelled() + /** + * Register [listener] to run when this process is cancelled — a *push* notification, so a consumer + * can react to cancellation immediately instead of polling [isCancelled]. If already cancelled, + * [listener] runs synchronously now. [listener] runs at most once. + * + * The default implementation only fires when already cancelled; an implementation that can transition + * to cancelled after registration (e.g. [Default]) overrides this to fire on the transition. + */ + fun invokeOnCancel(listener: () -> Unit) { + if (isCancelled()) { + listener() + } + } + open class Default(cancelled: Boolean = false) : ICancelChecker { private val cancelled = AtomicBoolean(cancelled) + private val onCancelListeners = CopyOnWriteArrayList<() -> Unit>() override fun cancel() { - cancelled.set(true) + // Fire listeners once, on the false -> true transition only. + if (cancelled.compareAndSet(false, true)) { + onCancelListeners.forEach { it() } + onCancelListeners.clear() + } } override fun isCancelled(): Boolean { @@ -62,6 +82,19 @@ interface ICancelChecker { throw CancellationException() } } + + override fun invokeOnCancel(listener: () -> Unit) { + if (isCancelled()) { + listener() + return + } + onCancelListeners.add(listener) + // Guard the race where cancel() ran between the check above and the add: if we now observe + // cancellation, run the listener ourselves (removing it so cancel() can't also run it). + if (isCancelled() && onCancelListeners.remove(listener)) { + listener() + } + } } companion object { @@ -70,7 +103,10 @@ interface ICancelChecker { * A no-op cancel checker. The task is never cancelled. */ @JvmField - val NOOP = Default(false) + val NOOP = object : Default(false) { + // Never transitions to cancelled, so retaining listeners would only leak them. + override fun invokeOnCancel(listener: () -> Unit) = Unit + } /** * An already cancelled cancel checker. diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt index b977b9ac33..def9416f1e 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt @@ -40,21 +40,42 @@ class ProgressManager private constructor() { } } + /** + * Associate an existing [checker] with [thread]. A subsequent [cancel] of [thread] then flips + * *this* checker (rather than a throwaway [Default]), so a caller that also polls [checker] + * observes the cancellation. Used by the editor to make a completion's cancel checker cancellable + * via the thread it runs on. Pair with [unregister]. + */ + fun register(thread: Thread, checker: ICancelChecker) { + synchronized(threads) { + threads[thread] = checker + } + } + + /** Remove any checker previously associated with [thread] via [register]. */ + fun unregister(thread: Thread) { + synchronized(threads) { + threads.remove(thread) + } + } + fun cancel(thread: Thread) { - var checker = threads[thread] - if (checker == null) { - checker = Default() + synchronized(threads) { + var checker = threads[thread] + if (checker == null) { + checker = Default() + threads[thread] = checker + } + checker.cancel() } - checker.cancel() - threads[thread] = checker } @JvmName("internalAbortIfCancelled") private fun abortIfCancelled() { val thisThread = Thread.currentThread() - val checker = threads[thisThread] + val checker = synchronized(threads) { threads[thisThread] } if (checker != null && checker.isCancelled()) { - threads.remove(thisThread) + synchronized(threads) { threads.remove(thisThread) } throw CancellationException() } } diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt new file mode 100644 index 0000000000..571fec354b --- /dev/null +++ b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt @@ -0,0 +1,98 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.progress + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import java.util.concurrent.atomic.AtomicInteger + +/** + * Tests for the push-based [ICancelChecker.invokeOnCancel] added for ADFA-4174, which lets the Kotlin + * LSP abort an in-flight `analyze` the moment cancellation happens instead of polling. + */ +class ICancelCheckerTest { + + @Test + fun `invokeOnCancel fires when cancel is called`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + + checker.invokeOnCancel { fired.incrementAndGet() } + assertThat(fired.get()).isEqualTo(0) + + checker.cancel() + assertThat(fired.get()).isEqualTo(1) + } + + @Test + fun `invokeOnCancel fires immediately when already cancelled`() { + val checker = ICancelChecker.Default(cancelled = true) + val fired = AtomicInteger(0) + + checker.invokeOnCancel { fired.incrementAndGet() } + + assertThat(fired.get()).isEqualTo(1) + } + + @Test + fun `invokeOnCancel fires at most once across repeated cancel calls`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + + checker.invokeOnCancel { fired.incrementAndGet() } + checker.cancel() + checker.cancel() + checker.cancel() + + assertThat(fired.get()).isEqualTo(1) + } + + @Test + fun `multiple listeners all fire on cancel`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + + checker.invokeOnCancel { fired.incrementAndGet() } + checker.invokeOnCancel { fired.incrementAndGet() } + + checker.cancel() + + assertThat(fired.get()).isEqualTo(2) + } + + @Test + fun `NOOP invokeOnCancel is a no-op`() { + val fired = AtomicInteger(0) + + // NOOP is a shared singleton that is never cancelled; registering must be a no-op so listeners + // (which may capture large objects) do not accumulate on it forever. We deliberately do NOT call + // NOOP.cancel() — flipping the shared singleton would corrupt every other user of it. + ICancelChecker.NOOP.invokeOnCancel { fired.incrementAndGet() } + + assertThat(fired.get()).isEqualTo(0) + } + + @Test + fun `CANCELLED fires immediately`() { + val fired = AtomicInteger(0) + + ICancelChecker.CANCELLED.invokeOnCancel { fired.incrementAndGet() } + + assertThat(fired.get()).isEqualTo(1) + } +} diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt new file mode 100644 index 0000000000..41edc171c7 --- /dev/null +++ b/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt @@ -0,0 +1,58 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.progress + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +class ProgressManagerTest { + + @Test + fun `cancel flips a registered checker`() { + // Regression for ADFA-4174: cancel(thread) must act on the *registered* checker so a caller that + // polls that same checker (the completion cancel checker driving mid-analyze abort) observes the + // cancellation — before, cancel() always stored a throwaway Default and the registered checker + // never became cancelled. + val checker = ICancelChecker.Default() + val thread = Thread.currentThread() + + ProgressManager.instance.register(thread, checker) + try { + assertThat(checker.isCancelled()).isFalse() + + ProgressManager.instance.cancel(thread) + + assertThat(checker.isCancelled()).isTrue() + } finally { + ProgressManager.instance.unregister(thread) + } + } + + @Test + fun `unregister detaches the checker so cancel no longer affects it`() { + val checker = ICancelChecker.Default() + val thread = Thread.currentThread() + + ProgressManager.instance.register(thread, checker) + ProgressManager.instance.unregister(thread) + + ProgressManager.instance.cancel(thread) + + assertThat(checker.isCancelled()).isFalse() + } +} From fdb4e5ee4446f34dd9a2416859017fc7f5f082af Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 3 Jul 2026 15:04:13 +0000 Subject: [PATCH 08/18] fix: abort in-flight Kotlin analysis promptly on completion cancellation withAnalysisLock installs a cancellable Job and registers an invokeOnCancel listener so both scheduler preemption and ordinary editor cancellation abort mid-analyze via a single push (no polling). AnalysisScheduler.acquire is cancellation-aware while waiting so superseded requests bail instead of parking. KotlinCompletions uses params.cancelChecker (race-free) and classifies all cancellation exception types uniformly. Adds AnalysisSerializationTest coverage. --- .../compiler/modules/AnalysisScheduler.kt | 37 +++++- .../modules/AnalysisThreadContext.java | 37 ++++++ .../lsp/kotlin/compiler/modules/KtFileExts.kt | 51 +++++--- .../kotlin/completion/KotlinCompletions.kt | 81 ++++++++++--- .../modules/AnalysisSerializationTest.kt | 109 ++++++++++++++++++ 5 files changed, 286 insertions(+), 29 deletions(-) create mode 100644 lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index aaaad78169..73c3c26a63 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -2,6 +2,8 @@ package com.itsaky.androidide.lsp.kotlin.compiler.modules import com.itsaky.androidide.progress.ICancelChecker import java.util.concurrent.CancellationException +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.TimeUnit import java.util.concurrent.locks.ReentrantLock import kotlin.concurrent.withLock @@ -55,9 +57,14 @@ internal class ScheduledCancelChecker( @Volatile private var preempted = false + private val onCancelListeners = CopyOnWriteArrayList<() -> Unit>() + /** Marks this analysis as preempted; the next [abortIfCancelled] will throw. */ fun preempt() { preempted = true + // Push: preemption is a cancellation too, so notify [invokeOnCancel] listeners immediately. + onCancelListeners.forEach { it() } + onCancelListeners.clear() } override fun cancel() { @@ -72,6 +79,18 @@ internal class ScheduledCancelChecker( } delegate.abortIfCancelled() } + + override fun invokeOnCancel(listener: () -> Unit) { + // Fire on either scheduler preemption (stored locally, run by preempt()) or the delegate's own + // cancellation (forwarded so the editor's CompletionCancelChecker.cancel() pushes it). The + // listener body is idempotent, so firing via both paths is harmless. + onCancelListeners.add(listener) + delegate.invokeOnCancel(listener) + // Guard the race where preempt() ran between add and now. + if (preempted && onCancelListeners.remove(listener)) { + listener() + } + } } /** @@ -91,6 +110,9 @@ internal class ScheduledCancelChecker( */ internal object AnalysisScheduler { + /** Upper bound on how long a queued requester waits before re-checking its cancellation. */ + private const val WAIT_POLL_MILLIS = 25L + private val mutex = ReentrantLock() private val available = mutex.newCondition() @@ -108,8 +130,14 @@ internal object AnalysisScheduler { * preemptable analysis is in progress — strictly lower priority, or the same priority when that * priority is [AnalysisPriority.supersedesSamePriority] — [onPreempt] of *that* holder is invoked so * it yields; [onPreempt] passed here is stored and used if this acquisition is later preempted. + * + * [cancelChecker] is *this* requester's checker: while waiting for the lock, the wait is re-checked on + * a short timer, and if the requester has been cancelled — e.g. the editor superseded + * this completion — [acquire] throws instead of parking until the lock frees. This is what stops + * superseded completions from piling up holding heavy state (KtFile copies, symbol lists) while they + * wait, which on-device saturated the heap and triggered multi-second GC stalls. */ - fun acquire(priority: AnalysisPriority, onPreempt: () -> Unit) { + fun acquire(priority: AnalysisPriority, cancelChecker: ICancelChecker, onPreempt: () -> Unit) { mutex.withLock { val me = Thread.currentThread() if (holderThread === me) { @@ -121,6 +149,9 @@ internal object AnalysisScheduler { waiting[priority.ordinal]++ try { while (true) { + // Bail out (rather than keep waiting) if this requester was cancelled while queued. + cancelChecker.abortIfCancelled() + val hp = holderPriority if (holderThread != null && hp != null && !holderPreempted && (hp.ordinal < priority.ordinal || @@ -135,7 +166,9 @@ internal object AnalysisScheduler { if (holderThread == null && !higherPriorityWaiting(priority)) { break } - available.await() + // Timed wait so a cancellation that arrives without a lock-state change (no signal) is + // still observed within WAIT_POLL_MILLIS, bounding how long a superseded waiter parks. + available.await(WAIT_POLL_MILLIS, TimeUnit.MILLISECONDS) } } finally { waiting[priority.ordinal]-- diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java new file mode 100644 index 0000000000..35aa3dde9c --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java @@ -0,0 +1,37 @@ +package com.itsaky.androidide.lsp.kotlin.compiler.modules; + +import kotlin.coroutines.CoroutineContext; +import kotlinx.coroutines.Job; +import org.jetbrains.kotlin.com.intellij.concurrency.ThreadContext; +import org.jetbrains.kotlin.com.intellij.openapi.application.AccessToken; + +/** + * Bridge to the embeddable IntelliJ {@link ThreadContext} coroutine-context API. + * + *

{@code currentThreadContext}/{@code installThreadContext} live in a Kotlin file facade whose + * metadata the Kotlin compiler cannot resolve against from this module (they surface as + * "unresolved reference" from Kotlin). At the bytecode level, though, they are plain + * {@code public static} methods, so Java can call them directly. + * + *

This is used by {@code withAnalysisLock} to install a cancellable {@link Job} into the analysis + * thread's context: the embeddable {@code CoreProgressManager.checkCanceled()} routes through + * {@code Cancellation.checkCancelled()}, which throws as soon as that Job is cancelled, aborting the + * running analysis mid-{@code analyze}. + */ +public final class AnalysisThreadContext { + + private AnalysisThreadContext() { + } + + /** + * Installs {@code job} into the current thread's IntelliJ coroutine context (preserving any + * context already present) and returns a token that restores the previous context when closed. + * + *

Public because it is referenced from the {@code internal inline} {@code withAnalysisLock}; + * an inline function may only reference declarations at least as accessible as itself. + */ + public static AccessToken installJob(Job job) { + CoroutineContext context = ThreadContext.currentThreadContext().plus(job); + return ThreadContext.installThreadContext(context, true); + } +} 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 f9ca1d8720..58813a34f0 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 @@ -6,6 +6,7 @@ import org.jetbrains.kotlin.analysis.api.analyzeCopy import org.jetbrains.kotlin.analysis.api.projectStructure.KaDanglingFileResolutionMode import org.jetbrains.kotlin.analysis.api.projectStructure.copyOrigin import org.jetbrains.kotlin.analysis.api.projectStructure.isDangling +import kotlinx.coroutines.Job import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager import org.jetbrains.kotlin.com.intellij.openapi.util.Key @@ -38,14 +39,16 @@ private val logger = LoggerFactory.getLogger("KtFileExts") * **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. * - * **Cancellation.** The [action] runs under a [CancelCheckerProgressIndicator] installed as the - * thread's IntelliJ progress indicator, so the compiler's own `ProgressManager.checkCanceled()` - * calls (dense throughout FIR resolution) abort the analysis *mid*-`analyze` once [cancelChecker] - * reports preemption or cancellation — not just at the coarse LSP [ScheduledCancelChecker.abortIfCancelled] - * checkpoints between work units. The compiler signals this by throwing [ProcessCanceledException]; - * we translate it back into the typed exception the callers already handle - * ([AnalysisPreemptedException] when preempted, else the delegate's `CancellationException`) so - * preempted work is rescheduled rather than silently dropped. + * **Cancellation.** The [action] runs with a [kotlinx.coroutines.Job] installed in the thread's + * IntelliJ coroutine context. The compiler's own `ProgressManager.checkCanceled()` calls (dense + * throughout FIR resolution) route through `Cancellation.checkCancelled()`, which throws as soon as + * that Job is cancelled — aborting the analysis *mid*-`analyze`, not just at the coarse LSP + * [ScheduledCancelChecker.abortIfCancelled] checkpoints between work units. (A + * [CancelCheckerProgressIndicator] is also installed as a fallback, but its poll-driven arming does + * not run in this embeddable environment; see the body.) The compiler signals cancellation by + * throwing [ProcessCanceledException]; we translate it back into the typed exception the callers + * already handle ([AnalysisPreemptedException] when preempted, else the delegate's + * `CancellationException`) so preempted work is rescheduled rather than silently dropped. * * **Footgun:** analysis runs under the *read* (shared) side of the global * [com.itsaky.androidide.lsp.kotlin.compiler.read] lock, and that `ReentrantReadWriteLock` is @@ -59,19 +62,39 @@ internal inline fun withAnalysisLock( crossinline action: () -> R, ): R { val indicator = CancelCheckerProgressIndicator(cancelChecker) - // When this analysis is preempted, also cancel the indicator so the compiler's in-`analyze` - // cancellation checks fire immediately instead of waiting for the manager's background poll. - AnalysisScheduler.acquire(priority) { + + // Cancelling [job] is what actually aborts the running analysis *mid*-`analyze`: the embeddable + // `CoreProgressManager.checkCanceled()` (called densely throughout FIR resolution) invokes + // `Cancellation.checkCancelled()` *unconditionally* — before any indicator/`CheckCanceledBehavior` + // gating — and that throws a `CeProcessCanceledException` (a `ProcessCanceledException`) the moment + // the [Job] installed in this thread's context is cancelled. + // + // The [indicator] is kept only as a fallback for environments whose `CoreProgressManager` runs its + // ~10ms non-standard-indicator poll; that poll does not run in this (Android, embeddable) one. + val job = Job() + + AnalysisScheduler.acquire(priority, cancelChecker) { + // Preemption is signalled by flipping the checker; the invokeOnCancel listener below turns that + // (and ordinary editor cancellation) into the actual mid-`analyze` abort. cancelChecker.preempt() + } + // Single push path for *both* preemption and ordinary editor cancellation: [ScheduledCancelChecker] + // fires this on preempt() and on its delegate's cancel(), so the running analyze aborts immediately + // — no polling, and unaffected by GC pauses that would stall a poll thread. + cancelChecker.invokeOnCancel { indicator.cancel() + job.cancel() } try { val holder = arrayOfNulls(1) try { - ProgressManager.getInstance() - .executeProcessUnderProgress({ holder[0] = action() }, indicator) + // Install the Job for the duration of the analysis and restore the previous context after. + AnalysisThreadContext.installJob(job).use { + ProgressManager.getInstance() + .executeProcessUnderProgress({ holder[0] = action() }, indicator) + } } catch (e: ProcessCanceledException) { - logger.debug("process cancelled: prio={}", priority) + logger.debug("process cancelled: prio={}", priority) // Re-derive the semantically-correct exception callers expect (re-throws // AnalysisPreemptedException when preempted, or the delegate's CancellationException). cancelChecker.abortIfCancelled() 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 0370632f71..7b760991ce 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.AnalysisPreemptedException import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling @@ -23,7 +24,9 @@ import com.itsaky.androidide.preferences.utils.indentationString import com.itsaky.androidide.progress.ICancelChecker import com.itsaky.androidide.progress.ProgressManager import com.itsaky.androidide.projects.FileManager +import io.github.rosemoe.sora.lang.completion.CompletionCancelledException import kotlinx.coroutines.CancellationException +import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException import org.appdevforall.codeonthego.indexing.jvm.JvmClassInfo import org.appdevforall.codeonthego.indexing.jvm.JvmFunctionInfo import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol @@ -77,13 +80,49 @@ private const val KT_COMPLETION_PLACEHOLDER = "KT_COMPLETION_PLACEHOLDER" private val logger = LoggerFactory.getLogger("KotlinCompletions") +/** Max unimported symbols pulled from each index for scope completion (see [collectUnimportedSymbols]). */ +private const val UNIMPORTED_SYMBOL_LIMIT = 100 + +/** + * The [ScheduledCancelChecker] for the completion request running on this thread, set for the + * duration of [doComplete]. The LSP-level [abortIfCancelled] checkpoints consult it so they observe + * scheduler *preemption* (a newer completion superseding this one, or this one yielding to a + * higher-priority request) — not just the editor's own request cancellation. Without this, those + * checkpoints only saw the raw delegate from [Lookup] and every preemption was misreported as an + * ordinary cancellation. + */ +private val currentCancelChecker = ThreadLocal() + private fun abortIfCancelled() { ProgressManager.abortIfCancelled() - Lookup.getDefault() - .lookup(ICancelChecker::class.java) - ?.abortIfCancelled() + val checker = currentCancelChecker.get() + if (checker != null) { + checker.abortIfCancelled() + } else { + Lookup.getDefault() + .lookup(ICancelChecker::class.java) + ?.abortIfCancelled() + } } +/** + * A cancelled completion surfaces as one of several exception types depending on *where* it was + * observed, and none of them is an error: + * - [CancellationException]/[AnalysisPreemptedException] — a coarse [abortIfCancelled] checkpoint; + * - [ProcessCanceledException] — the compiler's mid-`analyze` cancellation (our `job` was cancelled); + * - [CompletionCancelledException] — the editor's sora publisher was cancelled (surfaced via the + * [com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker] delegate); + * - [InterruptedException] — the sora `CompletionThread` was interrupted mid-resolution. + * + * All of them mean "this completion was superseded/cancelled"; treat them uniformly so mid-`analyze` + * cancellations are reported cleanly instead of logged as spurious errors with a stack trace. + */ +private fun Throwable.isCancellation(): Boolean = + this is CancellationException || + this is InterruptedException || + this is ProcessCanceledException || + this is CompletionCancelledException + /** * Provide code completion for the given completion parameters. * @@ -96,8 +135,9 @@ internal fun codeComplete(params: CompletionParams): CompletionResult { return try { doComplete(params) } catch (error: Throwable) { - if (error is CancellationException || error is InterruptedException) { - logger.info("completion cancelled") + if (error.isCancellation()) { + val isPreempted = error is AnalysisPreemptedException + logger.info("completion cancelled (preempted={})", isPreempted) if (error is InterruptedException) { Thread.interrupted() } @@ -149,10 +189,17 @@ internal fun doComplete(params: CompletionParams): CompletionResult { // Completion is the highest-priority analysis: it preempts in-progress diagnostics/indexing and is // never preempted by lower-priority analysis. It can, however, be superseded by a *newer* completion // request (the user typing on) — that in-flight completion is then cancelled and simply discarded. - // The cancel checker is the editor's request-scoped one (from Lookup). - val cancelChecker = ScheduledCancelChecker( - Lookup.getDefault().lookup(ICancelChecker::class.java) ?: ICancelChecker.NOOP - ) + // + // Use the request-scoped checker carried on [params] rather than the global Lookup: Lookup holds a + // single ICancelChecker updated per request, so with concurrent completion threads an older request + // could read a newer request's checker and never observe its own cancellation. [params.cancelChecker] + // is the editor's own CompletionCancelChecker for *this* request. Fall back to Lookup only if a + // caller supplied the NOOP checker (e.g. tests that don't set one). + val delegate = params.cancelChecker.takeUnless { it === ICancelChecker.NOOP } + ?: Lookup.getDefault().lookup(ICancelChecker::class.java) + ?: ICancelChecker.NOOP + val cancelChecker = ScheduledCancelChecker(delegate) + currentCancelChecker.set(cancelChecker) return try { env.project.read { @@ -194,12 +241,17 @@ internal fun doComplete(params: CompletionParams): CompletionResult { } } } catch (e: Throwable) { - if (e is CancellationException) { + // Let cancellation (incl. mid-`analyze` ProcessCanceledException / sora + // CompletionCancelledException / InterruptedException) propagate to codeComplete's uniform + // handler rather than logging it as an error. + if (e.isCancellation()) { throw e } logger.warn("An error occurred while computing completions for {}", params.file, e) return CompletionResult.EMPTY + } finally { + currentCancelChecker.remove() } } @@ -362,13 +414,16 @@ private fun KaSession.collectUnimportedSymbols( buildUnimportedSymbolItem(symbol)?.let { to += it } } - env.libraryIndex?.findByPrefix(ctx.partial, limit = 0) + // Bounded: limit = 0 means "unlimited", which pulled every matching symbol from all three + // indexes on every keystroke — a major allocation/GC source. A capped result set is more than + // enough for a completion popup and keeps the per-request footprint small. + env.libraryIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) - env.sourceIndex?.findByPrefix(ctx.partial, limit = 0) + env.sourceIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) - env.generatedIndex?.findByPrefix(ctx.partial, limit = 0) + env.generatedIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) } 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 index fe80666dda..29156f98d2 100644 --- 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 @@ -11,10 +11,12 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import org.junit.Test import java.util.Collections +import java.util.concurrent.CancellationException import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference import kotlin.math.max /** @@ -219,6 +221,113 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(ranToCompletion.get()).isFalse() } + @Test(timeout = 10_000) + fun `ordinary cancellation aborts an in-flight analysis mid-analyze`() { + // Regression for ADFA-4174. Unlike preemption (a competing higher/same-priority request), an + // *ordinary* cancellation — the editor cancelling because the user typed on / moved the cursor + // / dismissed the popup — flips the request's ICancelChecker. Via ScheduledCancelChecker's + // invokeOnCancel push, cancelling the delegate must abort the compiler's mid-`analyze` FIR + // resolution promptly, rather than letting it run to completion (observed on-device as ~900ms + // stalls and piled-up completion threads). + val delegate = ICancelChecker.Default() + val file = createSourceFile("OrdinaryCancel.kt", "class C { fun f(): Int = 1 }") + val holding = CountDownLatch(1) + val ranToCompletion = AtomicBoolean(false) + val caught = AtomicReference(null) + + // Run inside a real analyze under the read lock: this also guards against a read/write-lock + // upgrade deadlock regression (withAnalysisLock runs under the shared read lock). + val worker = Thread { + try { + env.project.read { + analyzeMaybeDangling( + file, + AnalysisPriority.COMPLETION, + ScheduledCancelChecker(delegate), + ) { + holding.countDown() + repeat(2_000) { + // Compiler-level checkpoint only — mirrors FIR resolution, which never calls + // the LSP-level abortIfCancelled(). + ProgressManager.checkCanceled() + Thread.sleep(5) + } + ranToCompletion.set(true) + } + } + } catch (t: Throwable) { + caught.set(t) + } + } + worker.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + val startNs = System.nanoTime() + // Simulates EditorCompletionWindow.cancelCompletion() -> ProgressManager.cancel(thread) -> + // CompletionCancelChecker.cancel(). + delegate.cancel() + worker.join(2_000) + val elapsedMs = (System.nanoTime() - startNs) / 1_000_000 + + assertThat(worker.isAlive).isFalse() + assertThat(ranToCompletion.get()).isFalse() + // Ordinary cancellation, not preemption: the delegate's CancellationException, not + // AnalysisPreemptedException. + assertThat(caught.get()).isInstanceOf(CancellationException::class.java) + assertThat(caught.get()).isNotInstanceOf(AnalysisPreemptedException::class.java) + assertThat(elapsedMs).isLessThan(500) + } + + @Test(timeout = 10_000) + fun `a waiting requester bails when cancelled instead of waiting for the lock`() { + // Regression for ADFA-4174: a superseded completion that is queued behind another analysis must + // abort as soon as it is cancelled, rather than parking (holding heavy state) until the lock + // frees. On-device, parked-until-release completions piled up and saturated the heap. + val holding = CountDownLatch(1) + val release = CountDownLatch(1) + val waiterDelegate = ICancelChecker.Default() + val waiterThrew = AtomicReference(null) + val waiterEntered = AtomicBoolean(false) + + // A completion holder keeps the lock until released. + val holder = Thread { + withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + holding.countDown() + release.await() + } + } + holder.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + // A lower-priority (diagnostics) requester must wait behind the completion holder (it does not + // preempt). It is cancelled while waiting and must bail from acquire() promptly. + val waiter = Thread { + try { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(waiterDelegate)) { + waiterEntered.set(true) + } + } catch (t: Throwable) { + waiterThrew.set(t) + } + } + waiter.start() + + // Let it enter the wait loop, then cancel it. + Thread.sleep(200) + waiterDelegate.cancel() + + // The waiter must finish (bail) while the holder still holds the lock. + waiter.join(2_000) + val bailedWhileHeld = !waiter.isAlive + + release.countDown() + holder.join(5_000) + + assertThat(bailedWhileHeld).isTrue() + assertThat(waiterEntered.get()).isFalse() + assertThat(waiterThrew.get()).isInstanceOf(CancellationException::class.java) + } + @Test(timeout = 10_000) fun `lower priority request waits while a higher priority holder runs`() { val holding = CountDownLatch(1) From 9b3de6afcd2a63887742b1168320bc785714921c Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 3 Jul 2026 15:04:25 +0000 Subject: [PATCH 09/18] fix: coalesce Kotlin completion requests to a single in-flight analysis EditorCompletionWindow debounces keystrokes so at most one completion analysis runs at a time for the latest cursor position, preventing the CompletionThread/allocation pile-up. IDELanguage registers the request's CompletionCancelChecker on its thread so the editor's cancel routes to it and pushes the mid-analyze abort. --- .../androidide/editor/language/IDELanguage.kt | 8 +++ .../editor/ui/EditorCompletionWindow.kt | 57 ++++++++++++++++--- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt b/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt index ddca102e71..f074ec747a 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt @@ -25,6 +25,7 @@ import com.itsaky.androidide.lsp.debug.model.BreakpointDefinition import com.itsaky.androidide.lsp.debug.model.BreakpointRequest import com.itsaky.androidide.preferences.internal.EditorPreferences import com.itsaky.androidide.progress.ICancelChecker +import com.itsaky.androidide.progress.ProgressManager import io.github.rosemoe.sora.lang.Language import io.github.rosemoe.sora.lang.completion.CompletionCancelledException import io.github.rosemoe.sora.lang.completion.CompletionPublisher @@ -67,11 +68,18 @@ abstract class IDELanguage : Language { publisher: CompletionPublisher, extraArguments: Bundle ) { + val completionThread = Thread.currentThread() try { val cancelChecker = CompletionCancelChecker(publisher) Lookup.getDefault().update(ICancelChecker::class.java, cancelChecker) + // Bind the checker to this completion thread. EditorCompletionWindow.cancelCompletion() + // cancels the (old) thread via ProgressManager.cancel(thread); routing that to *this* + // checker's cancel() lets the LSP's invokeOnCancel listener abort the running analysis + // mid-`analyze` immediately, instead of only at coarse checkpoints. + ProgressManager.instance.register(completionThread, cancelChecker) doComplete(content, position, publisher, cancelChecker, extraArguments) } finally { + ProgressManager.instance.unregister(completionThread) Lookup.getDefault().unregister( ICancelChecker::class.java ) diff --git a/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt b/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt index 8561b79de3..ad5e4f5709 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt @@ -41,9 +41,22 @@ class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(edito private var listView: ListView? = null private val items: MutableList = mutableListOf() + /** + * A scheduled-but-not-yet-started completion request, kept so a newer keystroke can cancel it. + * See [requireCompletion]. + */ + private var pendingCompletion: Runnable? = null + companion object { private val log = LoggerFactory.getLogger(EditorCompletionWindow::class.java) + + /** + * Quiet period used to coalesce a burst of keystrokes into a single completion request. Rapid + * typing (re)schedules the start; only after the user pauses this long does one analysis run, + * for the latest cursor position. Keeps at most one completion in flight. + */ + private const val COMPLETION_DEBOUNCE_MS = 80L } init { @@ -119,29 +132,57 @@ class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(edito } override fun cancelCompletion() { + // Drop any request that was scheduled but hasn't started yet. + pendingCompletion?.let { editor.handler.removeCallbacks(it) } + pendingCompletion = null if (completionThread != null) { ProgressManager.instance.cancel(completionThread) } super.cancelCompletion() } - override fun requireCompletion() { + /** + * Whether a completion may be shown for the current editor state. Hides the window (matching the + * prior inline behaviour) when the cursor is selected or completion is otherwise not applicable. + */ + private fun canStartCompletion(): Boolean { if (cancelShowUp || !isEnabled || !editor.isAttachedToWindow) { - return + return false } - - val text = editor.text - if (text.cursor.isSelected || checkNoCompletion()) { + if (editor.text.cursor.isSelected || checkNoCompletion()) { hide() - return + return false } + return true + } - if (System.nanoTime() - requestTime < editor.props.cancelCompletionNs) { - requestTime = System.nanoTime() + override fun requireCompletion() { + if (!canStartCompletion()) { return } + // Coalesce a burst of keystrokes into a single completion. Cancel the in-flight completion and + // any pending (not-yet-started) one, then (re)schedule one start after a short quiet period. + // This guarantees at most one completion analysis in flight and that only the latest cursor + // position is computed — preventing the CompletionThread/allocation pile-up that saturated the + // heap and froze the editor during fast typing. cancelCompletion() clears any pending request, + // so we always schedule exactly one. cancelCompletion() + + val request = Runnable { startCompletion() } + pendingCompletion = request + editor.handler.postDelayed(request, COMPLETION_DEBOUNCE_MS) + } + + /** Starts a single completion for the current cursor position. Runs on the UI thread. */ + private fun startCompletion() { + pendingCompletion = null + + // The editor state may have changed during the debounce delay; re-check the guards. + if (!canStartCompletion()) { + return + } + requestTime = System.nanoTime() currentSelection = -1 From 6c68d9395b894a190149853707103e2e839762cb Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Wed, 8 Jul 2026 19:17:11 +0530 Subject: [PATCH 10/18] fix: fix CodeRabbit comments Signed-off-by: Akash Yadav --- .../compiler/modules/AnalysisScheduler.kt | 20 +++++++++++ .../lsp/kotlin/compiler/modules/KtFileExts.kt | 33 ++++++++++--------- .../androidide/progress/ProgressManager.kt | 12 ++++--- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index 73c3c26a63..cd6e61fc00 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -182,6 +182,26 @@ internal object AnalysisScheduler { } } + /** + * Run [action] while holding the analysis lock at the given [priority], guaranteeing the hold taken + * by [acquire] is always released — even if [action] throws. Mirrors [kotlin.concurrent.withLock]: + * prefer this over calling [acquire]/[release] directly, so a lock can never leak and permanently + * deadlock all subsequent analysis. See [acquire] for the meaning of [cancelChecker] and [onPreempt]. + */ + inline fun withLock( + priority: AnalysisPriority, + cancelChecker: ICancelChecker, + noinline onPreempt: () -> Unit, + action: () -> R, + ): R { + acquire(priority, cancelChecker, onPreempt) + try { + return action() + } finally { + release() + } + } + /** Release a hold acquired via [acquire]. Wakes waiters when the outermost hold is released. */ fun release() { mutex.withLock { 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 58813a34f0..757498323b 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 @@ -73,19 +73,22 @@ internal inline fun withAnalysisLock( // ~10ms non-standard-indicator poll; that poll does not run in this (Android, embeddable) one. val job = Job() - AnalysisScheduler.acquire(priority, cancelChecker) { - // Preemption is signalled by flipping the checker; the invokeOnCancel listener below turns that - // (and ordinary editor cancellation) into the actual mid-`analyze` abort. - cancelChecker.preempt() - } - // Single push path for *both* preemption and ordinary editor cancellation: [ScheduledCancelChecker] - // fires this on preempt() and on its delegate's cancel(), so the running analyze aborts immediately - // — no polling, and unaffected by GC pauses that would stall a poll thread. - cancelChecker.invokeOnCancel { - indicator.cancel() - job.cancel() - } - try { + return AnalysisScheduler.withLock( + priority, + cancelChecker, + onPreempt = { + // Preemption is signalled by flipping the checker; the invokeOnCancel listener below turns + // that (and ordinary editor cancellation) into the actual mid-`analyze` abort. + cancelChecker.preempt() + }, + ) { + // Single push path for *both* preemption and ordinary editor cancellation: [ScheduledCancelChecker] + // fires this on preempt() and on its delegate's cancel(), so the running analyze aborts immediately + // — no polling, and unaffected by GC pauses that would stall a poll thread. + cancelChecker.invokeOnCancel { + indicator.cancel() + job.cancel() + } val holder = arrayOfNulls(1) try { // Install the Job for the duration of the analysis and restore the previous context after. @@ -101,9 +104,7 @@ internal inline fun withAnalysisLock( throw e } @Suppress("UNCHECKED_CAST") - return holder[0] as R - } finally { - AnalysisScheduler.release() + holder[0] as R } } diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt index def9416f1e..b50cd9b994 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt @@ -73,10 +73,14 @@ class ProgressManager private constructor() { @JvmName("internalAbortIfCancelled") private fun abortIfCancelled() { val thisThread = Thread.currentThread() - val checker = synchronized(threads) { threads[thisThread] } - if (checker != null && checker.isCancelled()) { - synchronized(threads) { threads.remove(thisThread) } - throw CancellationException() + // Check and remove atomically: a separate check-then-remove could race a concurrent register() + // reusing this thread and delete the new, unrelated registration instead of the stale one. + synchronized(threads) { + val checker = threads[thisThread] + if (checker != null && checker.isCancelled()) { + threads.remove(thisThread) + throw CancellationException() + } } } } \ No newline at end of file From 0054e2a8608bd450f5a8d7b343dc52d08569ae7e Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Wed, 8 Jul 2026 19:26:52 +0530 Subject: [PATCH 11/18] chore: simplify comments Signed-off-by: Akash Yadav --- build.gradle.kts | 7 +- .../androidide/editor/language/IDELanguage.kt | 7 +- .../editor/ui/EditorCompletionWindow.kt | 27 +++----- .../kotlin/compiler/CompilationEnvironment.kt | 3 +- .../compiler/index/SourceFileIndexer.kt | 7 +- .../compiler/modules/AnalysisScheduler.kt | 24 +++---- .../modules/AnalysisThreadContext.java | 22 +++---- .../modules/CancelCheckerProgressIndicator.kt | 23 +++---- .../lsp/kotlin/compiler/modules/KtFileExts.kt | 65 +++++++------------ .../kotlin/completion/KotlinCompletions.kt | 50 ++++++-------- .../diagnostic/KotlinDiagnosticProvider.kt | 13 ++-- .../modules/AnalysisSerializationTest.kt | 32 ++++----- .../androidide/progress/ICancelChecker.kt | 10 ++- .../androidide/progress/ProgressManager.kt | 7 +- .../androidide/progress/ICancelCheckerTest.kt | 10 +-- .../progress/ProgressManagerTest.kt | 7 +- 16 files changed, 126 insertions(+), 188 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 8bb596384a..9efe96f422 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -98,10 +98,9 @@ subprojects { // - java.base/java.io, java.util: needed by Robolectric/Gradle worker // reflection in the same test JVM. // - java.base/java.util.concurrent: the embedded IntelliJ scheduler - // (BoundedTaskExecutor.info -> AppDelayQueue "Periodic tasks thread") - // reflectively reads FutureTask.callable. Without this the periodic - // thread dies, disabling CoreProgressManager's cancellation poll that - // makes the Kotlin Analysis API interruptible mid-`analyze`. + // reflectively reads FutureTask.callable; without this its periodic + // thread dies, disabling the cancellation poll that makes the Kotlin + // Analysis API interruptible mid-`analyze`. jvmArgs( "--add-opens=java.base/java.lang=ALL-UNNAMED", "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", diff --git a/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt b/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt index f074ec747a..a47e08d86c 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt @@ -72,10 +72,9 @@ abstract class IDELanguage : Language { try { val cancelChecker = CompletionCancelChecker(publisher) Lookup.getDefault().update(ICancelChecker::class.java, cancelChecker) - // Bind the checker to this completion thread. EditorCompletionWindow.cancelCompletion() - // cancels the (old) thread via ProgressManager.cancel(thread); routing that to *this* - // checker's cancel() lets the LSP's invokeOnCancel listener abort the running analysis - // mid-`analyze` immediately, instead of only at coarse checkpoints. + // Bind the checker to this thread so cancelCompletion()'s ProgressManager.cancel(thread) + // routes here, letting the LSP's invokeOnCancel abort the running analysis mid-`analyze` + // immediately rather than only at coarse checkpoints. ProgressManager.instance.register(completionThread, cancelChecker) doComplete(content, position, publisher, cancelChecker, extraArguments) } finally { diff --git a/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt b/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt index ad5e4f5709..227915c782 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt @@ -51,11 +51,7 @@ class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(edito private val log = LoggerFactory.getLogger(EditorCompletionWindow::class.java) - /** - * Quiet period used to coalesce a burst of keystrokes into a single completion request. Rapid - * typing (re)schedules the start; only after the user pauses this long does one analysis run, - * for the latest cursor position. Keeps at most one completion in flight. - */ + /** Quiet period for coalescing a keystroke burst: analysis runs only after typing pauses this long. */ private const val COMPLETION_DEBOUNCE_MS = 80L } @@ -141,10 +137,7 @@ class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(edito super.cancelCompletion() } - /** - * Whether a completion may be shown for the current editor state. Hides the window (matching the - * prior inline behaviour) when the cursor is selected or completion is otherwise not applicable. - */ + /** Whether completion may run now; hides the window when the cursor is selected or otherwise not applicable. */ private fun canStartCompletion(): Boolean { if (cancelShowUp || !isEnabled || !editor.isAttachedToWindow) { return false @@ -156,17 +149,17 @@ class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(edito return true } + /** + * Coalesces a keystroke burst into one completion for the latest cursor position, keeping at most one + * analysis in flight. This prevents the CompletionThread/allocation pile-up that saturated the heap and + * froze the editor during fast typing. + */ override fun requireCompletion() { if (!canStartCompletion()) { return } - // Coalesce a burst of keystrokes into a single completion. Cancel the in-flight completion and - // any pending (not-yet-started) one, then (re)schedule one start after a short quiet period. - // This guarantees at most one completion analysis in flight and that only the latest cursor - // position is computed — preventing the CompletionThread/allocation pile-up that saturated the - // heap and froze the editor during fast typing. cancelCompletion() clears any pending request, - // so we always schedule exactly one. + // cancelCompletion() clears any in-flight and pending request, so we then schedule exactly one. cancelCompletion() val request = Runnable { startCompletion() } @@ -174,11 +167,11 @@ class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(edito editor.handler.postDelayed(request, COMPLETION_DEBOUNCE_MS) } - /** Starts a single completion for the current cursor position. Runs on the UI thread. */ + /** Runs on the UI thread. */ private fun startCompletion() { pendingCompletion = null - // The editor state may have changed during the debounce delay; re-check the guards. + // Editor state may have changed during the debounce delay; re-check the guards. if (!canStartCompletion()) { return } 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 20512c20b0..f9a194924e 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 @@ -186,8 +186,7 @@ internal class CompilationEnvironment( languageClient?.publishDiagnostics(result) } } catch (e: AnalysisPreemptedException) { - // A higher-priority analysis (completion) preempted this diagnostics run. - // Re-schedule so diagnostics still run once the higher-priority work finishes. + // Preempted by completion; re-schedule so diagnostics still run once it finishes. logger.debug("diagnostics for {} preempted; rescheduling", path) fileAnalyzer.schedule(path) } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt index 5765998a9c..39f6b655f6 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt @@ -76,10 +76,9 @@ internal suspend fun indexSourceFile( symbolsIndex: JvmSymbolIndex, cancelChecker: ICancelChecker, ) { - // Indexing runs at the lowest (INDEXING) priority: it yields to both completion and diagnostics. - // Wrapping the checker lets the scheduler preempt an in-progress index pass; the preemption - // surfaces as AnalysisPreemptedException at the abortIfCancelled() checkpoints below, which the - // IndexWorker catches to re-queue the file. + // Indexing runs at the lowest priority, yielding to completion and diagnostics. Wrapping the checker + // lets the scheduler preempt an in-progress pass; the preemption surfaces as AnalysisPreemptedException + // at the abortIfCancelled() checkpoints below, which IndexWorker catches to re-queue the file. val checker = cancelChecker as? ScheduledCancelChecker ?: ScheduledCancelChecker(cancelChecker) // Defensive backstop: this runs on the debounced/async index scope, so a disposal path that diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index cd6e61fc00..15a109eb69 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -15,14 +15,11 @@ import kotlin.concurrent.withLock * Order: [INDEXING] < [DIAGNOSTICS] < [COMPLETION] — interactive completion beats background * diagnostics, which beats bulk indexing. * - * [supersedesSamePriority] additionally lets a *newer* request preempt an in-flight request of the - * **same** priority. This is enabled only for [COMPLETION]: when the user types fast, several - * completion requests fire in a row and the in-flight one is computing results for a now-stale cursor - * position, so the newer request cancels it and the fresh position is analysed immediately. A - * superseded completion is simply *discarded* — nothing reschedules it (see - * `KotlinCompletions.codeComplete`). It is intentionally off for [DIAGNOSTICS] and [INDEXING], whose - * preempted work is re-queued rather than dropped; same-priority preemption there would livelock, as - * two contenders would endlessly re-queue and re-preempt each other. + * [supersedesSamePriority] additionally lets a *newer* request preempt an in-flight one of the + * **same** priority. On for [COMPLETION] only: rapid typing makes the in-flight completion stale, so + * the newer one cancels it and the superseded work is *discarded* (nothing reschedules it). Off for + * [DIAGNOSTICS]/[INDEXING], whose preempted work is re-queued — there same-priority preemption would + * livelock, two contenders endlessly re-queuing and re-preempting each other. */ internal enum class AnalysisPriority(val supersedesSamePriority: Boolean) { INDEXING(supersedesSamePriority = false), @@ -62,7 +59,7 @@ internal class ScheduledCancelChecker( /** Marks this analysis as preempted; the next [abortIfCancelled] will throw. */ fun preempt() { preempted = true - // Push: preemption is a cancellation too, so notify [invokeOnCancel] listeners immediately. + // Preemption is a cancellation too: fire invokeOnCancel listeners now, don't wait for a poll. onCancelListeners.forEach { it() } onCancelListeners.clear() } @@ -131,11 +128,10 @@ internal object AnalysisScheduler { * priority is [AnalysisPriority.supersedesSamePriority] — [onPreempt] of *that* holder is invoked so * it yields; [onPreempt] passed here is stored and used if this acquisition is later preempted. * - * [cancelChecker] is *this* requester's checker: while waiting for the lock, the wait is re-checked on - * a short timer, and if the requester has been cancelled — e.g. the editor superseded - * this completion — [acquire] throws instead of parking until the lock frees. This is what stops - * superseded completions from piling up holding heavy state (KtFile copies, symbol lists) while they - * wait, which on-device saturated the heap and triggered multi-second GC stalls. + * [cancelChecker] is *this* requester's checker: a queued requester re-checks it on a short timer and + * [acquire] throws (rather than park until the lock frees) once cancelled — e.g. the editor superseded + * this completion. This stops superseded completions from piling up holding heavy state (KtFile copies, + * symbol lists), which on-device saturated the heap and triggered multi-second GC stalls. */ fun acquire(priority: AnalysisPriority, cancelChecker: ICancelChecker, onPreempt: () -> Unit) { mutex.withLock { diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java index 35aa3dde9c..ddf18687e9 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java @@ -6,17 +6,11 @@ import org.jetbrains.kotlin.com.intellij.openapi.application.AccessToken; /** - * Bridge to the embeddable IntelliJ {@link ThreadContext} coroutine-context API. + * Java bridge to the embeddable IntelliJ {@link ThreadContext} coroutine-context API. * - *

{@code currentThreadContext}/{@code installThreadContext} live in a Kotlin file facade whose - * metadata the Kotlin compiler cannot resolve against from this module (they surface as - * "unresolved reference" from Kotlin). At the bytecode level, though, they are plain - * {@code public static} methods, so Java can call them directly. - * - *

This is used by {@code withAnalysisLock} to install a cancellable {@link Job} into the analysis - * thread's context: the embeddable {@code CoreProgressManager.checkCanceled()} routes through - * {@code Cancellation.checkCancelled()}, which throws as soon as that Job is cancelled, aborting the - * running analysis mid-{@code analyze}. + *

Exists in Java because {@code currentThreadContext}/{@code installThreadContext} live in a + * Kotlin file-facade whose metadata this module's Kotlin compiler cannot resolve ("unresolved + * reference"), yet at the bytecode level they are plain {@code public static} methods Java can call. */ public final class AnalysisThreadContext { @@ -25,10 +19,12 @@ private AnalysisThreadContext() { /** * Installs {@code job} into the current thread's IntelliJ coroutine context (preserving any - * context already present) and returns a token that restores the previous context when closed. + * existing context) and returns a token that restores the previous context when closed. Cancelling + * {@code job} then aborts the running analysis mid-{@code analyze}, since the embeddable + * {@code CoreProgressManager.checkCanceled()} throws once the installed Job is cancelled. * - *

Public because it is referenced from the {@code internal inline} {@code withAnalysisLock}; - * an inline function may only reference declarations at least as accessible as itself. + *

Public (not package-private) because the {@code internal inline} {@code withAnalysisLock} + * references it: an inline function may only reference declarations at least as accessible as itself. */ public static AccessToken installJob(Job job) { CoroutineContext context = ThreadContext.currentThreadContext().plus(job); diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt index 7c47d33034..8b12678094 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt @@ -5,22 +5,15 @@ import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledExcepti import org.jetbrains.kotlin.com.intellij.openapi.progress.util.AbstractProgressIndicatorBase /** - * A [com.intellij.openapi.progress.ProgressIndicator] whose cancellation state is driven by an - * [ICancelChecker]. Installing it as the analysis thread's indicator (via - * [withAnalysisLock]) is what makes the Kotlin Analysis API actually interruptible *mid*-`analyze`. + * A progress indicator whose cancellation is driven by an [ICancelChecker]. Installing it as the + * analysis thread's indicator (via [withAnalysisLock]) makes the Kotlin Analysis API interruptible + * *mid*-`analyze`: the embeddable `CoreProgressManager.checkCanceled()` (called densely throughout + * FIR resolution) rethrows this indicator's [checkCanceled], so a preemption or cancellation aborts + * at the compiler's next internal checkpoint, not only at the coarse [ICancelChecker.abortIfCancelled]. * - * The embeddable analysis API ships the full IntelliJ `CoreProgressManager`, whose - * `ProgressManager.checkCanceled()` (called densely throughout FIR resolution) re-fetches the - * current thread's indicator and rethrows its [checkCanceled]. By bridging that to [checker], a - * preemption or ordinary cancellation aborts the running analysis at the compiler's next internal - * checkpoint instead of only at the coarse LSP-level [ICancelChecker.abortIfCancelled] checks. - * - * This extends [AbstractProgressIndicatorBase] rather than `EmptyProgressIndicator` on purpose: - * the base is a *non-standard* indicator, so `CoreProgressManager` runs a background task that - * polls [checkCanceled] every ~10ms. That poll flips the manager's internal "should check - * cancelled" flag to active once [checker] reports cancellation, which is what arms the in-`analyze` - * checks. [cancel] (invoked synchronously when this analysis is preempted) flips the same flag - * immediately, so preemption does not have to wait for the poll. + * Extends [AbstractProgressIndicatorBase] (a *non-standard* indicator) on purpose: `CoreProgressManager` + * then polls [checkCanceled] every ~10ms, arming its internal "check cancelled" flag once [checker] + * reports cancellation. `cancel()` flips that flag immediately so preemption need not wait for the poll. */ internal class CancelCheckerProgressIndicator( private val checker: ICancelChecker, 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 757498323b..61cd6a1367 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 @@ -24,37 +24,24 @@ private val logger = LoggerFactory.getLogger("KtFileExts") /** * Runs [action] while holding the global analysis lock at the given [priority]. * - * 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.` + * The Analysis API keeps per-thread `analyze` lifetime state and is unsafe to drive concurrently; + * indexing, diagnostics and completion all analyze on `Dispatchers.Default` and often target the same + * file, so overlapping `analyze` calls corrupted the session lifecycle + * (`KaInaccessibleLifetimeOwnerAccessException: ... Called outside an \`analyze\` context.`). + * [AnalysisScheduler] serializes access; it is priority-aware, preemptive (via [cancelChecker]) and + * reentrant. **All** Analysis API access must go through this helper (or [analyzeMaybeDangling]); never + * call `analyze` / `analyzeCopy` directly, or the serialization guarantee is lost. * - * Serialization is handled by [AnalysisScheduler], which is priority-aware and preemptive: a - * higher-priority request preempts a strictly lower-priority one (cooperatively, via [cancelChecker]). - * The scheduler is reentrant, so an (indirect) nested analysis on the same thread cannot deadlock. + * **Cancellation.** [action] runs with a [kotlinx.coroutines.Job] installed in the thread's IntelliJ + * context; the compiler's dense `checkCanceled()` calls throw once that Job is cancelled, aborting + * *mid*-`analyze` rather than only at the coarse [ScheduledCancelChecker.abortIfCancelled] checkpoints + * (a [CancelCheckerProgressIndicator] is installed as a fallback). On [ProcessCanceledException] we + * re-derive the typed exception callers expect ([AnalysisPreemptedException] when preempted, else the + * delegate's `CancellationException`) so preempted work is rescheduled, not silently dropped. * - * **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. - * - * **Cancellation.** The [action] runs with a [kotlinx.coroutines.Job] installed in the thread's - * IntelliJ coroutine context. The compiler's own `ProgressManager.checkCanceled()` calls (dense - * throughout FIR resolution) route through `Cancellation.checkCancelled()`, which throws as soon as - * that Job is cancelled — aborting the analysis *mid*-`analyze`, not just at the coarse LSP - * [ScheduledCancelChecker.abortIfCancelled] checkpoints between work units. (A - * [CancelCheckerProgressIndicator] is also installed as a fallback, but its poll-driven arming does - * not run in this embeddable environment; see the body.) The compiler signals cancellation by - * throwing [ProcessCanceledException]; we translate it back into the typed exception the callers - * already handle ([AnalysisPreemptedException] when preempted, else the delegate's - * `CancellationException`) so preempted work is rescheduled rather than silently dropped. - * - * **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. + * **Footgun:** analysis holds the *read* side of the non-upgradeable + * [com.itsaky.androidide.lsp.kotlin.compiler.read] lock, so code inside an `analyze` block must never + * call [com.itsaky.androidide.lsp.kotlin.compiler.write] — read → write on the same thread deadlocks. */ internal inline fun withAnalysisLock( priority: AnalysisPriority, @@ -63,35 +50,27 @@ internal inline fun withAnalysisLock( ): R { val indicator = CancelCheckerProgressIndicator(cancelChecker) - // Cancelling [job] is what actually aborts the running analysis *mid*-`analyze`: the embeddable - // `CoreProgressManager.checkCanceled()` (called densely throughout FIR resolution) invokes - // `Cancellation.checkCancelled()` *unconditionally* — before any indicator/`CheckCanceledBehavior` - // gating — and that throws a `CeProcessCanceledException` (a `ProcessCanceledException`) the moment - // the [Job] installed in this thread's context is cancelled. - // - // The [indicator] is kept only as a fallback for environments whose `CoreProgressManager` runs its - // ~10ms non-standard-indicator poll; that poll does not run in this (Android, embeddable) one. + // Cancelling [job] is what aborts analysis *mid*-`analyze`: the embeddable `checkCanceled()` throws + // unconditionally the moment this thread's installed Job is cancelled. [indicator] is only a fallback + // for environments that run the ~10ms indicator poll — this (Android, embeddable) one does not. val job = Job() return AnalysisScheduler.withLock( priority, cancelChecker, onPreempt = { - // Preemption is signalled by flipping the checker; the invokeOnCancel listener below turns - // that (and ordinary editor cancellation) into the actual mid-`analyze` abort. + // Flipping the checker only signals preempt; the invokeOnCancel listener below does the abort. cancelChecker.preempt() }, ) { - // Single push path for *both* preemption and ordinary editor cancellation: [ScheduledCancelChecker] - // fires this on preempt() and on its delegate's cancel(), so the running analyze aborts immediately - // — no polling, and unaffected by GC pauses that would stall a poll thread. + // Single push path for both preemption and editor cancellation: fires immediately, no polling + // and so unaffected by GC pauses that would stall a poll thread. cancelChecker.invokeOnCancel { indicator.cancel() job.cancel() } val holder = arrayOfNulls(1) try { - // Install the Job for the duration of the analysis and restore the previous context after. AnalysisThreadContext.installJob(job).use { ProgressManager.getInstance() .executeProcessUnderProgress({ holder[0] = action() }, indicator) 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 7b760991ce..389bfc8908 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 @@ -84,12 +84,9 @@ private val logger = LoggerFactory.getLogger("KotlinCompletions") private const val UNIMPORTED_SYMBOL_LIMIT = 100 /** - * The [ScheduledCancelChecker] for the completion request running on this thread, set for the - * duration of [doComplete]. The LSP-level [abortIfCancelled] checkpoints consult it so they observe - * scheduler *preemption* (a newer completion superseding this one, or this one yielding to a - * higher-priority request) — not just the editor's own request cancellation. Without this, those - * checkpoints only saw the raw delegate from [Lookup] and every preemption was misreported as an - * ordinary cancellation. + * The [ScheduledCancelChecker] for the completion running on this thread, set for the duration of + * [doComplete]. The [abortIfCancelled] checkpoints consult it so they observe scheduler *preemption* + * (a newer completion superseding this one), not just the editor's own request cancellation. */ private val currentCancelChecker = ThreadLocal() @@ -106,16 +103,11 @@ private fun abortIfCancelled() { } /** - * A cancelled completion surfaces as one of several exception types depending on *where* it was - * observed, and none of them is an error: - * - [CancellationException]/[AnalysisPreemptedException] — a coarse [abortIfCancelled] checkpoint; - * - [ProcessCanceledException] — the compiler's mid-`analyze` cancellation (our `job` was cancelled); - * - [CompletionCancelledException] — the editor's sora publisher was cancelled (surfaced via the - * [com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker] delegate); - * - [InterruptedException] — the sora `CompletionThread` was interrupted mid-resolution. - * - * All of them mean "this completion was superseded/cancelled"; treat them uniformly so mid-`analyze` - * cancellations are reported cleanly instead of logged as spurious errors with a stack trace. + * A cancelled completion surfaces as different exception types depending on where it was observed + * ([CancellationException]/[AnalysisPreemptedException] at a checkpoint, [ProcessCanceledException] + * mid-`analyze`, [CompletionCancelledException] from the sora publisher, [InterruptedException] on + * the sora completion thread). All mean "superseded/cancelled"; treat them uniformly so none is + * logged as a spurious error. */ private fun Throwable.isCancellation(): Boolean = this is CancellationException || @@ -149,6 +141,11 @@ internal fun codeComplete(params: CompletionParams): CompletionResult { } } +/** + * Runs at the highest [AnalysisPriority.COMPLETION]: preempts in-progress diagnostics/indexing and + * is never preempted by lower-priority work, but is superseded (cancelled and discarded) by a newer + * completion request as the user keeps typing. + */ context(env: CompilationEnvironment) internal fun doComplete(params: CompletionParams): CompletionResult { val ktFile = env.ktSymbolIndex.getOpenedKtFile(params.file) @@ -186,15 +183,9 @@ internal fun doComplete(params: CompletionParams): CompletionResult { abortIfCancelled() - // Completion is the highest-priority analysis: it preempts in-progress diagnostics/indexing and is - // never preempted by lower-priority analysis. It can, however, be superseded by a *newer* completion - // request (the user typing on) — that in-flight completion is then cancelled and simply discarded. - // - // Use the request-scoped checker carried on [params] rather than the global Lookup: Lookup holds a - // single ICancelChecker updated per request, so with concurrent completion threads an older request - // could read a newer request's checker and never observe its own cancellation. [params.cancelChecker] - // is the editor's own CompletionCancelChecker for *this* request. Fall back to Lookup only if a - // caller supplied the NOOP checker (e.g. tests that don't set one). + // Use the request-scoped checker on params, not the global Lookup: Lookup holds one ICancelChecker + // updated per request, so with concurrent completions an older request could read a newer request's + // checker and never observe its own cancellation. Fall back to Lookup only for a NOOP checker (tests). val delegate = params.cancelChecker.takeUnless { it === ICancelChecker.NOOP } ?: Lookup.getDefault().lookup(ICancelChecker::class.java) ?: ICancelChecker.NOOP @@ -241,9 +232,7 @@ internal fun doComplete(params: CompletionParams): CompletionResult { } } } catch (e: Throwable) { - // Let cancellation (incl. mid-`analyze` ProcessCanceledException / sora - // CompletionCancelledException / InterruptedException) propagate to codeComplete's uniform - // handler rather than logging it as an error. + // Let cancellation propagate to codeComplete's uniform handler instead of logging it as an error. if (e.isCancellation()) { throw e } @@ -414,9 +403,8 @@ private fun KaSession.collectUnimportedSymbols( buildUnimportedSymbolItem(symbol)?.let { to += it } } - // Bounded: limit = 0 means "unlimited", which pulled every matching symbol from all three - // indexes on every keystroke — a major allocation/GC source. A capped result set is more than - // enough for a completion popup and keeps the per-request footprint small. + // Bounded: limit = 0 ("unlimited") pulled every matching symbol from all three indexes on every + // keystroke — a major allocation/GC source, and a capped set is plenty for a completion popup. env.libraryIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) 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 c60bece3fe..71578e4800 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 @@ -66,10 +66,9 @@ private fun doAnalyze(file: Path, cancelChecker: ICancelChecker): DiagnosticResu return DiagnosticResult.NO_UPDATE } - // Diagnostics run at DIAGNOSTICS priority: they yield to completion but preempt indexing. The - // wrapped checker turns a scheduler preemption into an AnalysisPreemptedException, which - // CompilationEnvironment's fileAnalyzer catches to re-schedule this run after the higher-priority - // work finishes. + // Diagnostics yield to completion but preempt indexing. The wrapped checker turns a scheduler + // preemption into an AnalysisPreemptedException, which CompilationEnvironment's fileAnalyzer catches + // to re-schedule this run once the higher-priority work finishes. val checker = ScheduledCancelChecker(cancelChecker) val diagnostics = env.project.read { @@ -87,9 +86,9 @@ private fun doAnalyze(file: Path, cancelChecker: ICancelChecker): DiagnosticResu ) } - // analyzeMaybeDangling installs a CancelCheckerProgressIndicator, so this analysis is - // cancellable mid-`analyze`: it aborts at the compiler's internal checkCanceled() once - // `checker` reports preemption/cancellation (in addition to the abortIfCancelled() below). + // analyzeMaybeDangling installs a CancelCheckerProgressIndicator, so this is cancellable + // mid-`analyze`: it aborts at the compiler's internal checkCanceled() once `checker` reports + // preemption/cancellation. (Previously this analysis was not cancellable at all.) analyzeMaybeDangling(ktFile, AnalysisPriority.DIAGNOSTICS, checker) { ktFile.collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) .forEach { diagnostic -> 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 index 29156f98d2..3f1903b76c 100644 --- 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 @@ -178,14 +178,14 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(higherRan.get()).isTrue() } + /** + * FIR resolution polls [ProgressManager.checkCanceled] densely but never the LSP-level + * [ICancelChecker.abortIfCancelled], so this body only polls the former. Preemption must still + * interrupt it: [withAnalysisLock] installs a `CancelCheckerProgressIndicator` so that + * otherwise-inert checkpoint aborts. + */ @Test(timeout = 10_000) fun `analysis is interrupted mid-analyze at the compiler's ProgressManager checkpoint`() { - // The Kotlin Analysis API calls ProgressManager.checkCanceled() densely during FIR - // resolution, but never the LSP-level ICancelChecker.abortIfCancelled(). This body mimics - // that: it only polls ProgressManager.checkCanceled(). Before withAnalysisLock installed a - // CancelCheckerProgressIndicator, that call was inert (no indicator => the manager's - // check-cancelled behaviour stayed disabled) and the work ran to completion regardless of - // preemption. It must now be interruptible. val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) val holding = CountDownLatch(1) val preempted = AtomicBoolean(false) @@ -221,14 +221,14 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(ranToCompletion.get()).isFalse() } + /** + * Regression for ADFA-4174: an *ordinary* cancellation (the editor cancelling on a keystroke, + * cursor move or popup dismissal), distinct from preemption, must abort the compiler's + * mid-`analyze` FIR resolution promptly via [ScheduledCancelChecker]'s `invokeOnCancel` push — + * on-device this showed up as ~900ms stalls and piled-up completion threads. + */ @Test(timeout = 10_000) fun `ordinary cancellation aborts an in-flight analysis mid-analyze`() { - // Regression for ADFA-4174. Unlike preemption (a competing higher/same-priority request), an - // *ordinary* cancellation — the editor cancelling because the user typed on / moved the cursor - // / dismissed the popup — flips the request's ICancelChecker. Via ScheduledCancelChecker's - // invokeOnCancel push, cancelling the delegate must abort the compiler's mid-`analyze` FIR - // resolution promptly, rather than letting it run to completion (observed on-device as ~900ms - // stalls and piled-up completion threads). val delegate = ICancelChecker.Default() val file = createSourceFile("OrdinaryCancel.kt", "class C { fun f(): Int = 1 }") val holding = CountDownLatch(1) @@ -278,11 +278,13 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(elapsedMs).isLessThan(500) } + /** + * Regression for ADFA-4174: a cancelled requester queued behind another analysis must bail + * immediately instead of parking (holding heavy state) until the lock frees — on-device these + * parked completions piled up and saturated the heap. + */ @Test(timeout = 10_000) fun `a waiting requester bails when cancelled instead of waiting for the lock`() { - // Regression for ADFA-4174: a superseded completion that is queued behind another analysis must - // abort as soon as it is cancelled, rather than parking (holding heavy state) until the lock - // frees. On-device, parked-until-release completions piled up and saturated the heap. val holding = CountDownLatch(1) val release = CountDownLatch(1) val waiterDelegate = ICancelChecker.Default() diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt index ce0bcb1974..99f2da692a 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt @@ -47,12 +47,11 @@ interface ICancelChecker { fun abortIfCancelled() /** - * Register [listener] to run when this process is cancelled — a *push* notification, so a consumer - * can react to cancellation immediately instead of polling [isCancelled]. If already cancelled, - * [listener] runs synchronously now. [listener] runs at most once. + * Register [listener] to fire when this process is cancelled, so a consumer can react immediately + * instead of polling [isCancelled]. Fires synchronously now if already cancelled, and at most once. * - * The default implementation only fires when already cancelled; an implementation that can transition - * to cancelled after registration (e.g. [Default]) overrides this to fire on the transition. + * This default only fires when already cancelled; an implementation that can transition to cancelled + * after registration (e.g. [Default]) must override to fire on the transition. */ fun invokeOnCancel(listener: () -> Unit) { if (isCancelled()) { @@ -66,7 +65,6 @@ interface ICancelChecker { private val onCancelListeners = CopyOnWriteArrayList<() -> Unit>() override fun cancel() { - // Fire listeners once, on the false -> true transition only. if (cancelled.compareAndSet(false, true)) { onCancelListeners.forEach { it() } onCancelListeners.clear() diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt index b50cd9b994..80d976bdf8 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt @@ -41,10 +41,9 @@ class ProgressManager private constructor() { } /** - * Associate an existing [checker] with [thread]. A subsequent [cancel] of [thread] then flips - * *this* checker (rather than a throwaway [Default]), so a caller that also polls [checker] - * observes the cancellation. Used by the editor to make a completion's cancel checker cancellable - * via the thread it runs on. Pair with [unregister]. + * Associate an existing [checker] with [thread] so a later [cancel] of [thread] flips *this* + * checker (not a throwaway [Default]), letting a caller that polls [checker] observe the + * cancellation. Pair with [unregister]. */ fun register(thread: Thread, checker: ICancelChecker) { synchronized(threads) { diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt index 571fec354b..00f8c75200 100644 --- a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt +++ b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt @@ -22,8 +22,8 @@ import org.junit.Test import java.util.concurrent.atomic.AtomicInteger /** - * Tests for the push-based [ICancelChecker.invokeOnCancel] added for ADFA-4174, which lets the Kotlin - * LSP abort an in-flight `analyze` the moment cancellation happens instead of polling. + * Tests for the push-based [ICancelChecker.invokeOnCancel] (ADFA-4174), which lets the Kotlin LSP + * abort an in-flight `analyze` the moment cancellation happens instead of polling. */ class ICancelCheckerTest { @@ -79,9 +79,9 @@ class ICancelCheckerTest { fun `NOOP invokeOnCancel is a no-op`() { val fired = AtomicInteger(0) - // NOOP is a shared singleton that is never cancelled; registering must be a no-op so listeners - // (which may capture large objects) do not accumulate on it forever. We deliberately do NOT call - // NOOP.cancel() — flipping the shared singleton would corrupt every other user of it. + // NOOP is a shared singleton that never cancels: registering must be a no-op so captured listeners + // don't accumulate forever. We deliberately don't call NOOP.cancel() — flipping the shared + // singleton would corrupt every other user of it. ICancelChecker.NOOP.invokeOnCancel { fired.incrementAndGet() } assertThat(fired.get()).isEqualTo(0) diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt index 41edc171c7..c782fc6133 100644 --- a/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt +++ b/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt @@ -24,10 +24,9 @@ class ProgressManagerTest { @Test fun `cancel flips a registered checker`() { - // Regression for ADFA-4174: cancel(thread) must act on the *registered* checker so a caller that - // polls that same checker (the completion cancel checker driving mid-analyze abort) observes the - // cancellation — before, cancel() always stored a throwaway Default and the registered checker - // never became cancelled. + // Regression for ADFA-4174: cancel(thread) must flip the *registered* checker so a caller polling + // it observes the cancellation. Previously cancel() stored a throwaway Default and the registered + // checker never became cancelled. val checker = ICancelChecker.Default() val thread = Thread.currentThread() From c035d6f8ad521a751564732f594d8c8608b988f9 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Mon, 13 Jul 2026 17:31:53 +0000 Subject: [PATCH 12/18] ADFA-4174: Rename AnalysisPriority.COMPLETION to INTERACTIVE --- .../kotlin/compiler/modules/AnalysisScheduler.kt | 8 ++++---- .../lsp/kotlin/completion/KotlinCompletions.kt | 4 ++-- .../modules/AnalysisSerializationTest.kt | 16 ++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index 15a109eb69..b35c948823 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -12,11 +12,11 @@ import kotlin.concurrent.withLock * lower-priority analysis that is currently running, and is served before any lower-priority request * that is merely waiting. * - * Order: [INDEXING] < [DIAGNOSTICS] < [COMPLETION] — interactive completion beats background - * diagnostics, which beats bulk indexing. + * Order: [INDEXING] < [DIAGNOSTICS] < [INTERACTIVE] — interactive requests (completion, signature + * help) beat background diagnostics, which beats bulk indexing. * * [supersedesSamePriority] additionally lets a *newer* request preempt an in-flight one of the - * **same** priority. On for [COMPLETION] only: rapid typing makes the in-flight completion stale, so + * **same** priority. On for [INTERACTIVE] only: rapid typing makes the in-flight request stale, so * the newer one cancels it and the superseded work is *discarded* (nothing reschedules it). Off for * [DIAGNOSTICS]/[INDEXING], whose preempted work is re-queued — there same-priority preemption would * livelock, two contenders endlessly re-queuing and re-preempting each other. @@ -24,7 +24,7 @@ import kotlin.concurrent.withLock internal enum class AnalysisPriority(val supersedesSamePriority: Boolean) { INDEXING(supersedesSamePriority = false), DIAGNOSTICS(supersedesSamePriority = false), - COMPLETION(supersedesSamePriority = true), + INTERACTIVE(supersedesSamePriority = true), } /** 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 ba6e188c7e..08b6952d7a 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 @@ -140,7 +140,7 @@ internal fun codeComplete(params: CompletionParams): CompletionResult { } /** - * Runs at the highest [AnalysisPriority.COMPLETION]: preempts in-progress diagnostics/indexing and + * Runs at the highest [AnalysisPriority.INTERACTIVE]: preempts in-progress diagnostics/indexing and * is never preempted by lower-priority work, but is superseded (cancelled and discarded) by a newer * completion request as the user keeps typing. */ @@ -194,7 +194,7 @@ internal fun doComplete(params: CompletionParams): CompletionResult { env.project.read { abortIfCancelled() - analyzeMaybeDangling(completionKtFile, AnalysisPriority.COMPLETION, cancelChecker) { + analyzeMaybeDangling(completionKtFile, AnalysisPriority.INTERACTIVE, cancelChecker) { val ctx = resolveAnalysisContext( env = env, 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 index 3f1903b76c..f0a3081ad0 100644 --- 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 @@ -133,7 +133,7 @@ class AnalysisSerializationTest : KtLspTest() { fun `reentrant withAnalysisLock on the same thread does not deadlock`() { var innerRan = false withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { - withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { innerRan = true } } @@ -166,7 +166,7 @@ class AnalysisSerializationTest : KtLspTest() { // A completion request must preempt the in-progress indexing. val higher = Thread { - withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { higherRan.set(true) } } @@ -211,7 +211,7 @@ class AnalysisSerializationTest : KtLspTest() { // A completion request preempts the in-progress (indexing) analysis. val higher = Thread { - withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) {} + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) {} } higher.start() higher.join(5_000) @@ -242,7 +242,7 @@ class AnalysisSerializationTest : KtLspTest() { env.project.read { analyzeMaybeDangling( file, - AnalysisPriority.COMPLETION, + AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(delegate), ) { holding.countDown() @@ -293,7 +293,7 @@ class AnalysisSerializationTest : KtLspTest() { // A completion holder keeps the lock until released. val holder = Thread { - withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { holding.countDown() release.await() } @@ -338,7 +338,7 @@ class AnalysisSerializationTest : KtLspTest() { // High-priority (completion) holder holds the lock until released. val higher = Thread { - withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { holding.countDown() release.await() } @@ -376,7 +376,7 @@ class AnalysisSerializationTest : KtLspTest() { // An in-flight completion runs a long, cooperatively-cancellable analysis. val older = Thread { try { - withAnalysisLock(AnalysisPriority.COMPLETION, holderChecker) { + withAnalysisLock(AnalysisPriority.INTERACTIVE, holderChecker) { holding.countDown() repeat(2_000) { holderChecker.abortIfCancelled() @@ -392,7 +392,7 @@ class AnalysisSerializationTest : KtLspTest() { // A newer completion request (user typed on) must supersede the in-flight one. val newer = Thread { - withAnalysisLock(AnalysisPriority.COMPLETION, ScheduledCancelChecker(ICancelChecker.NOOP)) { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { newerRan.set(true) } } From 3b688677c815c3733d84ca2c3a1194a4e36420ac Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Mon, 13 Jul 2026 17:33:04 +0000 Subject: [PATCH 13/18] ADFA-4174: Extract shared isAnalysisCancellation helper --- .../lsp/kotlin/compiler/modules/KtFileExts.kt | 15 +++++++++++++++ .../lsp/kotlin/completion/KotlinCompletions.kt | 17 ++++++----------- 2 files changed, 21 insertions(+), 11 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 61cd6a1367..200c230c1b 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 @@ -6,6 +6,7 @@ import org.jetbrains.kotlin.analysis.api.analyzeCopy import org.jetbrains.kotlin.analysis.api.projectStructure.KaDanglingFileResolutionMode import org.jetbrains.kotlin.analysis.api.projectStructure.copyOrigin import org.jetbrains.kotlin.analysis.api.projectStructure.isDangling +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Job import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager @@ -100,3 +101,17 @@ internal inline fun analyzeMaybeDangling( analyze(useSiteElement, action) } } + +/** + * True when [this] signals an analysis was cancelled or preempted rather than genuinely failing. + * + * A cancelled analysis surfaces as different types depending on where it was observed: a + * [CancellationException] (which also covers [AnalysisPreemptedException], thrown at a + * [ScheduledCancelChecker.abortIfCancelled] checkpoint), a [ProcessCanceledException] raised + * mid-`analyze`, or an [InterruptedException] on an interrupted worker thread. All mean + * "superseded/cancelled"; callers treat them uniformly so none is logged as a spurious error. + */ +internal fun Throwable.isAnalysisCancellation(): Boolean = + this is CancellationException || + this is ProcessCanceledException || + this is InterruptedException 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 08b6952d7a..7d2f646c38 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 @@ -7,6 +7,7 @@ import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPreemptedExcept import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling +import com.itsaky.androidide.lsp.kotlin.compiler.modules.isAnalysisCancellation import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.AnalysisContext import com.itsaky.androidide.lsp.kotlin.utils.ContextKeywords @@ -26,7 +27,6 @@ import com.itsaky.androidide.preferences.utils.indentationString import com.itsaky.androidide.progress.ICancelChecker import com.itsaky.androidide.progress.ProgressManager import io.github.rosemoe.sora.lang.completion.CompletionCancelledException -import kotlinx.coroutines.CancellationException import org.appdevforall.codeonthego.indexing.jvm.JvmClassInfo import org.appdevforall.codeonthego.indexing.jvm.JvmFunctionInfo import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol @@ -56,7 +56,6 @@ import org.jetbrains.kotlin.analysis.api.symbols.receiverType import org.jetbrains.kotlin.analysis.api.types.KaClassType import org.jetbrains.kotlin.analysis.api.types.KaType import org.jetbrains.kotlin.analysis.low.level.api.fir.util.originalKtFile -import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName @@ -101,17 +100,13 @@ private fun abortIfCancelled() { } /** - * A cancelled completion surfaces as different exception types depending on where it was observed - * ([CancellationException]/[AnalysisPreemptedException] at a checkpoint, [ProcessCanceledException] - * mid-`analyze`, [CompletionCancelledException] from the sora publisher, [InterruptedException] on - * the sora completion thread). All mean "superseded/cancelled"; treat them uniformly so none is - * logged as a spurious error. + * A cancelled completion surfaces as different exception types. [isAnalysisCancellation] covers the + * analysis-level ones (cancellation, preemption, process-cancellation, interruption); the + * sora-publisher-specific [CompletionCancelledException] is layered on here. All mean + * "superseded/cancelled"; treat them uniformly so none is logged as a spurious error. */ private fun Throwable.isCancellation(): Boolean = - this is CancellationException || - this is InterruptedException || - this is ProcessCanceledException || - this is CompletionCancelledException + isAnalysisCancellation() || this is CompletionCancelledException /** * Provide code completion for the given completion parameters. From 78d1a3c482f1cbc52397436deef5ec2ab6af10ce Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Mon, 13 Jul 2026 17:34:01 +0000 Subject: [PATCH 14/18] ADFA-4174: Run signature help at INTERACTIVE priority with cancellation --- .../signaturehelp/KotlinSignatureHelp.kt | 156 ++++++++++-------- 1 file changed, 90 insertions(+), 66 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt index ca92c09241..17a80cda13 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt @@ -1,12 +1,15 @@ package com.itsaky.androidide.lsp.kotlin.signaturehelp import com.itsaky.androidide.lsp.kotlin.compiler.CompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPreemptedException +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling +import com.itsaky.androidide.lsp.kotlin.compiler.modules.isAnalysisCancellation import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.models.SignatureHelp import com.itsaky.androidide.lsp.models.SignatureHelpParams import com.itsaky.androidide.lsp.models.SignatureInformation -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.future.await import org.jetbrains.kotlin.analysis.api.KaSession import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall @@ -23,46 +26,53 @@ import org.slf4j.LoggerFactory * active parameter is computed against that active overload. */ internal fun KaSession.buildSignatureHelp(call: KtCallElement, offset: Int): SignatureHelp { -val calleeText = call.calleeExpression?.text + val calleeText = call.calleeExpression?.text // (resolved function call, isBest) pairs, in candidate order. -val resolvedCandidates = call.resolveToCallCandidates() - .mapNotNull { info -> - (info.candidate as? KaFunctionCall<*>)?.let { it to info.isInBestCandidates } - } -logger.debug( - "resolveToCallCandidates() found {} candidate(s) for call '{}'", - resolvedCandidates.size, - calleeText -) + val resolvedCandidates = call.resolveToCallCandidates() + .mapNotNull { info -> + (info.candidate as? KaFunctionCall<*>)?.let { it to info.isInBestCandidates } + } + logger.debug( + "resolveToCallCandidates() found {} candidate(s) for call '{}'", + resolvedCandidates.size, + calleeText + ) -val candidates = resolvedCandidates.ifEmpty { - // Fallback: a single successfully-resolved function call. - logger.debug("No candidates from resolveToCallCandidates(); falling back to resolveToCall() for '{}'", calleeText) - call.resolveToCall()?.successfulFunctionCallOrNull()?.let { listOf(it to true) } ?: emptyList() - } + val candidates = resolvedCandidates.ifEmpty { + // Fallback: a single successfully-resolved function call. + logger.debug( + "No candidates from resolveToCallCandidates(); falling back to resolveToCall() for '{}'", + calleeText + ) + call.resolveToCall()?.successfulFunctionCallOrNull()?.let { listOf(it to true) } + ?: emptyList() + } -if (candidates.isEmpty()) { - logger.debug("No resolvable candidates for call '{}'; returning empty signature help", calleeText) - return SignatureHelp.empty() -} + if (candidates.isEmpty()) { + logger.debug( + "No resolvable candidates for call '{}'; returning empty signature help", + calleeText + ) + return SignatureHelp.empty() + } -val signatures: List = - candidates.map { (fnCall, _) -> buildSignatureInformation(fnCall.symbol) } + val signatures: List = + candidates.map { (fnCall, _) -> buildSignatureInformation(fnCall.symbol) } -val activeSignature = candidates.indexOfFirst { it.second }.let { if (it < 0) 0 else it } -val activeCall = candidates[activeSignature].first -val activeParameter = computeActiveParameter(call, activeCall, offset) + val activeSignature = candidates.indexOfFirst { it.second }.let { if (it < 0) 0 else it } + val activeCall = candidates[activeSignature].first + val activeParameter = computeActiveParameter(call, activeCall, offset) -logger.debug( - "buildSignatureHelp for '{}': {} signature(s), activeSignature={}, activeParameter={}", - calleeText, - signatures.size, - activeSignature, - activeParameter -) + logger.debug( + "buildSignatureHelp for '{}': {} signature(s), activeSignature={}, activeParameter={}", + calleeText, + signatures.size, + activeSignature, + activeParameter + ) -return SignatureHelp(signatures, activeSignature, activeParameter) + return SignatureHelp(signatures, activeSignature, activeParameter) } private val logger = LoggerFactory.getLogger("KotlinSignatureHelp") @@ -73,40 +83,54 @@ private val logger = LoggerFactory.getLogger("KotlinSignatureHelp") */ context(env: CompilationEnvironment) internal suspend fun doSignatureHelp(params: SignatureHelpParams): SignatureHelp { -logger.debug("doSignatureHelp requested for file={} position={}", params.file, params.position) + logger.debug("doSignatureHelp requested for file={} position={}", params.file, params.position) -if (params.cancelChecker.isCancelled()) { - logger.debug("Signature help request for {} was cancelled before processing", params.file) - return SignatureHelp.empty() -} + if (params.cancelChecker.isCancelled()) { + logger.debug("Signature help request for {} was cancelled before processing", params.file) + return SignatureHelp.empty() + } -// Safe to await a (possibly blocking) refresh here: this runs outside any project.read/write -// block, so it can't deadlock against the refresh's project.write (unlike KtSymbolIndex.getKtFile). -val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() -if (ktFile == null) { - logger.warn("File {} is not open", params.file) - return SignatureHelp.empty() -} + // Safe to await a (possibly blocking) refresh here: this runs outside any project.read/write + // block, so it can't deadlock against the refresh's project.write (unlike KtSymbolIndex.getKtFile). + val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() + if (ktFile == null) { + logger.warn("File {} is not open", params.file) + return SignatureHelp.empty() + } -return try { - val offset = params.position.requireIndex() - val result = env.project.read { - val call = findEnclosingCall(ktFile, offset) ?: return@read SignatureHelp.empty() - analyzeMaybeDangling(ktFile) { - buildSignatureHelp(call, offset) - } - } - logger.debug( - "Signature help result for {}: {} signature(s), activeSignature={}, activeParameter={}", - params.file, - result.signatures.size, - result.activeSignature, - result.activeParameter - ) - result -} catch (e: Throwable) { - if (e is CancellationException) throw e - logger.warn("Signature help computation failed for {}", params.file, e) - SignatureHelp.empty() -} + // Signature help is interactive (the user is typing arguments): run at INTERACTIVE priority so it + // preempts background diagnostics/indexing and is discarded when a newer interactive request wins. + // params.cancelChecker is request-scoped (CancellableRequestParams), so wrap it directly — no + // global Lookup fallback needed. + val cancelChecker = ScheduledCancelChecker(params.cancelChecker) + + return try { + val offset = params.position.requireIndex() + cancelChecker.abortIfCancelled() + val result = env.project.read { + val call = findEnclosingCall(ktFile, offset) ?: return@read SignatureHelp.empty() + analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { + buildSignatureHelp(call, offset) + } + } + logger.debug( + "Signature help result for {}: {} signature(s), activeSignature={}, activeParameter={}", + params.file, + result.signatures.size, + result.activeSignature, + result.activeParameter + ) + result + } catch (e: Throwable) { + if (e.isAnalysisCancellation()) { + logger.debug( + "Signature help for {} cancelled (preempted={})", + params.file, + e is AnalysisPreemptedException + ) + return SignatureHelp.empty() + } + logger.warn("Signature help computation failed for {}", params.file, e) + SignatureHelp.empty() + } } From 66da411e3ee169f9f23bf00d9246485dcd248796 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Mon, 13 Jul 2026 17:37:50 +0000 Subject: [PATCH 15/18] ADFA-4174: Migrate signature-help test fixture to priority-aware analysis lock --- .../com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt index 435d984664..43a701939f 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt @@ -1,8 +1,11 @@ package com.itsaky.androidide.lsp.kotlin.fixtures import com.itsaky.androidide.lsp.kotlin.compiler.index.toMetadata +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.read +import com.itsaky.androidide.progress.ICancelChecker import kotlinx.coroutines.runBlocking import org.jetbrains.kotlin.analysis.api.KaSession import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall @@ -63,7 +66,7 @@ abstract class KtLspTest { val ktFile = call.containingKtFile runBlocking { env.ktSymbolIndex.fileIndex.upsert(ktFile.toMetadata(env.project, isIndexed = false)) } return env.project.read { - analyzeMaybeDangling(ktFile) { + analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { val resolved = call.resolveToCall()?.successfulFunctionCallOrNull() action(resolved) } From 3616f37781e040214e8d8d4a49d3481c414ede11 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Tue, 14 Jul 2026 20:52:46 +0530 Subject: [PATCH 16/18] fix: resolve review comments Signed-off-by: Akash Yadav --- .../compiler/modules/AnalysisScheduler.kt | 6 +++++ .../lsp/kotlin/compiler/modules/KtFileExts.kt | 9 +++++-- .../signaturehelp/KotlinSignatureHelp.kt | 2 +- .../androidide/progress/ICancelChecker.kt | 14 ++++++++++ .../androidide/progress/ProgressManager.kt | 6 +++++ .../androidide/progress/ICancelCheckerTest.kt | 27 +++++++++++++++++++ 6 files changed, 61 insertions(+), 3 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index b35c948823..18330719b4 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -88,6 +88,12 @@ internal class ScheduledCancelChecker( listener() } } + + override fun removeOnCancel(listener: () -> Unit) { + // Mirror invokeOnCancel: drop from both the local (preemption) list and the delegate. + onCancelListeners.remove(listener) + delegate.removeOnCancel(listener) + } } /** 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 200c230c1b..dba236f521 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 @@ -65,11 +65,14 @@ internal inline fun withAnalysisLock( }, ) { // Single push path for both preemption and editor cancellation: fires immediately, no polling - // and so unaffected by GC pauses that would stall a poll thread. - cancelChecker.invokeOnCancel { + // and so unaffected by GC pauses that would stall a poll thread. Removed on the way out so a + // checker outliving this call (reuse across analyze calls, or a reentrant lock) doesn't retain + // a listener capturing this now-dead job/indicator. + val onCancel: () -> Unit = { indicator.cancel() job.cancel() } + cancelChecker.invokeOnCancel(onCancel) val holder = arrayOfNulls(1) try { AnalysisThreadContext.installJob(job).use { @@ -82,6 +85,8 @@ internal inline fun withAnalysisLock( // AnalysisPreemptedException when preempted, or the delegate's CancellationException). cancelChecker.abortIfCancelled() throw e + } finally { + cancelChecker.removeOnCancel(onCancel) } @Suppress("UNCHECKED_CAST") holder[0] as R diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt index 17a80cda13..650e033bda 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt @@ -120,7 +120,7 @@ internal suspend fun doSignatureHelp(params: SignatureHelpParams): SignatureHelp result.activeSignature, result.activeParameter ) - result + result } catch (e: Throwable) { if (e.isAnalysisCancellation()) { logger.debug( diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt index 99f2da692a..5b80266169 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt @@ -59,6 +59,16 @@ interface ICancelChecker { } } + /** + * Unregister a [listener] previously passed to [invokeOnCancel]. Removal is by reference identity, + * so callers must pass the *same* lambda instance. No-op if the listener was never registered or + * has already fired (listeners fire at most once and are dropped on firing). + * + * The default retains no listeners, so this does nothing; an implementation that stores listeners + * (e.g. [Default]) must override to drop [listener]. + */ + fun removeOnCancel(listener: () -> Unit) {} + open class Default(cancelled: Boolean = false) : ICancelChecker { private val cancelled = AtomicBoolean(cancelled) @@ -93,6 +103,10 @@ interface ICancelChecker { listener() } } + + override fun removeOnCancel(listener: () -> Unit) { + onCancelListeners.remove(listener) + } } companion object { diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt index 80d976bdf8..a0032fa631 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt @@ -44,6 +44,12 @@ class ProgressManager private constructor() { * Associate an existing [checker] with [thread] so a later [cancel] of [thread] flips *this* * checker (not a throwaway [Default]), letting a caller that polls [checker] observe the * cancellation. Pair with [unregister]. + * + * **Contract:** at most one live registration per thread. A caller must [unregister] its checker + * before registering another on the same thread. Any existing registration is overwritten and + * discarded, *including a cancelled one*: a [cancel] that arrived while nothing was registered + * targeted prior work on this thread, so it is not carried forward to the incoming [checker]. A + * caller that needs a cancel-before-register signal to survive must not rely on this method. */ fun register(thread: Thread, checker: ICancelChecker) { synchronized(threads) { diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt index 00f8c75200..9612f41cfc 100644 --- a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt +++ b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt @@ -75,6 +75,33 @@ class ICancelCheckerTest { assertThat(fired.get()).isEqualTo(2) } + @Test + fun `removeOnCancel drops the listener so it does not fire`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + val listener: () -> Unit = { fired.incrementAndGet() } + + checker.invokeOnCancel(listener) + checker.removeOnCancel(listener) + checker.cancel() + + assertThat(fired.get()).isEqualTo(0) + } + + @Test + fun `removeOnCancel only drops the given listener`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + val removed: () -> Unit = { fired.incrementAndGet() } + + checker.invokeOnCancel(removed) + checker.invokeOnCancel { fired.incrementAndGet() } + checker.removeOnCancel(removed) + checker.cancel() + + assertThat(fired.get()).isEqualTo(1) + } + @Test fun `NOOP invokeOnCancel is a no-op`() { val fired = AtomicInteger(0) From ab67790aa483c59829c9c2ddcf3858698eccac9f Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Mon, 27 Jul 2026 21:51:08 +0530 Subject: [PATCH 17/18] refactor: reformat Signed-off-by: Akash Yadav --- build.gradle.kts | 18 +- .../androidide/editor/language/IDELanguage.kt | 43 +- .../editor/ui/EditorCompletionWindow.kt | 387 +++++++------- .../kotlin/compiler/CompilationEnvironment.kt | 478 +++++++++--------- .../lsp/kotlin/compiler/index/IndexWorker.kt | 206 ++++---- .../compiler/modules/AnalysisScheduler.kt | 21 +- .../modules/AnalysisThreadContext.java | 31 +- .../modules/CancelCheckerProgressIndicator.kt | 1 - .../lsp/kotlin/compiler/modules/KtFileExts.kt | 7 +- .../kotlin/completion/KotlinCompletions.kt | 423 +++++++++------- .../diagnostic/KotlinDiagnosticProvider.kt | 12 +- .../signaturehelp/KotlinSignatureHelp.kt | 179 +++---- .../modules/AnalysisSerializationTest.kt | 340 +++++++------ .../androidide/progress/ICancelChecker.kt | 196 ++++--- .../androidide/progress/ProgressManager.kt | 9 +- .../androidide/progress/ICancelCheckerTest.kt | 143 +++--- .../progress/ProgressManagerTest.kt | 65 ++- 17 files changed, 1324 insertions(+), 1235 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 073a99e026..c909aac72c 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -97,15 +97,15 @@ subprojects { // - java.base/java.io, java.util: needed by Robolectric/Gradle worker // reflection in the same test JVM. // - java.base/java.util.concurrent: the embedded IntelliJ scheduler - // reflectively reads FutureTask.callable; without this its periodic - // thread dies, disabling the cancellation poll that makes the Kotlin - // Analysis API interruptible mid-`analyze`. - jvmArgs( - "--add-opens=java.base/java.lang=ALL-UNNAMED", - "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", - "--add-opens=java.base/java.io=ALL-UNNAMED", - "--add-opens=java.base/java.util=ALL-UNNAMED", - "--add-opens=java.base/java.util.concurrent=ALL-UNNAMED", + // reflectively reads FutureTask.callable; without this its periodic + // thread dies, disabling the cancellation poll that makes the Kotlin + // Analysis API interruptible mid-`analyze`. + jvmArgs( + "--add-opens=java.base/java.lang=ALL-UNNAMED", + "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", + "--add-opens=java.base/java.io=ALL-UNNAMED", + "--add-opens=java.base/java.util=ALL-UNNAMED", + "--add-opens=java.base/java.util.concurrent=ALL-UNNAMED", "--add-opens=jdk.unsupported/sun.misc=ALL-UNNAMED", ) diff --git a/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt b/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt index a47e08d86c..ec948cb24a 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kt @@ -42,23 +42,27 @@ import java.nio.file.Paths * @author Akash Yadav */ abstract class IDELanguage : Language { - private var formatter: Formatter? = null protected open val languageServer: ILanguageServer? get() = null - open fun getTabSize(): Int { - return EditorPreferences.tabSize - } + open fun getTabSize(): Int = EditorPreferences.tabSize open fun addBreakpoint(line: Int) {} + open fun addBreakpoints(lines: Iterable) = lines.forEach(::addBreakpoint) + open fun removeBreakpoint(line: Int) {} + open fun removeBreakpoints(lines: Iterable) = lines.forEach(::removeBreakpoint) + open fun removeAllBreakpoints() {} + open fun toggleBreakpoint(line: Int) {} + open fun highlightLine(line: Int) {} + open fun unhighlightLines() {} @Throws(CompletionCancelledException::class) @@ -66,7 +70,7 @@ abstract class IDELanguage : Language { content: ContentReference, position: CharPosition, publisher: CompletionPublisher, - extraArguments: Bundle + extraArguments: Bundle, ) { val completionThread = Thread.currentThread() try { @@ -80,7 +84,7 @@ abstract class IDELanguage : Language { } finally { ProgressManager.instance.unregister(completionThread) Lookup.getDefault().unregister( - ICancelChecker::class.java + ICancelChecker::class.java, ) } } @@ -90,7 +94,7 @@ abstract class IDELanguage : Language { position: CharPosition, publisher: CompletionPublisher, cancelChecker: CompletionCancelChecker, - extraArguments: Bundle + extraArguments: Bundle, ) { val server = languageServer ?: return val path = extraArguments.getString(IEditor.KEY_FILE, null) @@ -114,32 +118,21 @@ abstract class IDELanguage : Language { * @param c The character to check. * @return `true` if the character is completion char, `false` otherwise. */ - protected open fun checkIsCompletionChar(c: Char): Boolean { - return false - } + protected open fun checkIsCompletionChar(c: Char): Boolean = false - override fun useTab(): Boolean { - return !EditorPreferences.useSoftTab - } + override fun useTab(): Boolean = !EditorPreferences.useSoftTab - override fun getFormatter(): Formatter { - return formatter ?: LSPFormatter(languageServer).also { formatter = it } - } + override fun getFormatter(): Formatter = formatter ?: LSPFormatter(languageServer).also { formatter = it } override fun getIndentAdvance( content: ContentReference, line: Int, - column: Int - ): Int { - return getIndentAdvance(content.getLine(line).substring(0, column)) - } + column: Int, + ): Int = getIndentAdvance(content.getLine(line).substring(0, column)) - open fun getIndentAdvance(line: String): Int { - return 0 - } + open fun getIndentAdvance(line: String): Int = 0 companion object { - private val log = LoggerFactory.getLogger(IDELanguage::class.java) } -} \ No newline at end of file +} diff --git a/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt b/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt index 227915c782..b870e6e0bb 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/ui/EditorCompletionWindow.kt @@ -36,197 +36,198 @@ import kotlin.math.min * * @author Akash Yadav */ -class EditorCompletionWindow(val editor: IDEEditor) : EditorAutoCompletion(editor) { - - private var listView: ListView? = null - private val items: MutableList = mutableListOf() - - /** - * A scheduled-but-not-yet-started completion request, kept so a newer keystroke can cancel it. - * See [requireCompletion]. - */ - private var pendingCompletion: Runnable? = null - - companion object { - - private val log = LoggerFactory.getLogger(EditorCompletionWindow::class.java) - - /** Quiet period for coalescing a keystroke burst: analysis runs only after typing pauses this long. */ - private const val COMPLETION_DEBOUNCE_MS = 80L - } - - init { - setLayout(EditorCompletionLayout()) - setEnabledAnimation(true) - } - - override fun isShowing(): Boolean { - @Suppress("UNNECESSARY_SAFE_CALL", "USELESS_ELVIS") - return popup?.isShowing ?: false - } - - override fun setLayout(layout: CompletionLayout) { - super.setLayout(layout) - (layout.completionList as? ListView)?.let { - listView = it - it.adapter = this.adapter - it.setOnItemLongClickListener { _, view, position, _ -> - - val category = when (editor.file?.extension) { - "java" -> TooltipCategory.CATEGORY_JAVA - "kt" -> TooltipCategory.CATEGORY_KOTLIN - "xml" -> TooltipCategory.CATEGORY_XML - else -> TooltipCategory.CATEGORY_IDE - } - - val completionItem = - items[position] as? com.itsaky.androidide.lsp.models.CompletionItem - val completionData = completionItem?.data - - val tag = if (completionData == null) { - val label = completionItem?.ideLabel ?: "" - val attrName = if (label.contains(':')) label.substringAfterLast(':') else label - attrName.ifEmpty { null } - } else { - DocumentationReferenceProvider.getTag(completionData) - } - - // Dismiss the completion window before showing tooltip - hide() - - Log.d("EditorCompletionWindow", "Showing tooltip for tag: $tag category: $category") - TooltipManager.showTooltip( - context = editor.context, - anchorView = editor, - category = category, - tag = tag ?: "", - ) - true - } - } - } - - override fun select(pos: Int): Boolean { - if (pos > adapter!!.count) { - return false - } - return try { - super.select(pos) - } catch (e: Throwable) { - log.warn("Unable to select completion item at {}", pos, e) - false - } - } - - override fun select(): Boolean { - return try { - super.select() - } catch (e: Throwable) { - log.warn("Unable to select completion item", e) - false - } - } - - override fun cancelCompletion() { - // Drop any request that was scheduled but hasn't started yet. - pendingCompletion?.let { editor.handler.removeCallbacks(it) } - pendingCompletion = null - if (completionThread != null) { - ProgressManager.instance.cancel(completionThread) - } - super.cancelCompletion() - } - - /** Whether completion may run now; hides the window when the cursor is selected or otherwise not applicable. */ - private fun canStartCompletion(): Boolean { - if (cancelShowUp || !isEnabled || !editor.isAttachedToWindow) { - return false - } - if (editor.text.cursor.isSelected || checkNoCompletion()) { - hide() - return false - } - return true - } - - /** - * Coalesces a keystroke burst into one completion for the latest cursor position, keeping at most one - * analysis in flight. This prevents the CompletionThread/allocation pile-up that saturated the heap and - * froze the editor during fast typing. - */ - override fun requireCompletion() { - if (!canStartCompletion()) { - return - } - - // cancelCompletion() clears any in-flight and pending request, so we then schedule exactly one. - cancelCompletion() - - val request = Runnable { startCompletion() } - pendingCompletion = request - editor.handler.postDelayed(request, COMPLETION_DEBOUNCE_MS) - } - - /** Runs on the UI thread. */ - private fun startCompletion() { - pendingCompletion = null - - // Editor state may have changed during the debounce delay; re-check the guards. - if (!canStartCompletion()) { - return - } - - requestTime = System.nanoTime() - currentSelection = -1 - - publisher = - IDECompletionPublisher( - editor.handler, - { - val items = publisher.items - - this.items.apply { - clear() - addAll(items) - } - - if (lastAttachedItems == null || lastAttachedItems.get() != items) { - adapter.attachValues(this, items) - adapter.notifyDataSetInvalidated() - lastAttachedItems = WeakReference(items) - } else { - adapter.notifyDataSetChanged() - } - - val newHeight = (adapter!!.itemHeight * adapter!!.count).toFloat() - if (newHeight == 0F) { - hide() - } - - editor.getComponent(EditorAutoCompletion::class.java) - .updateCompletionWindowPosition() - setSize(width, min(newHeight, maxHeight.toFloat()).toInt()) - if (!isShowing) { - show() - } - - if (adapter!!.count >= 1 - && KeyboardUtils.isHardKeyboardConnected(context) - ) { - currentSelection = 0 - } - }, - editor.editorLanguage.interruptionLevel - ) - - publisher.setUpdateThreshold(1) - - completionThread = CompletionThread(requestTime, publisher) - completionThread.name = "CompletionThread-$requestTime" - - setLoading(true) - - completionThread.start() - } - +class EditorCompletionWindow( + val editor: IDEEditor, +) : EditorAutoCompletion(editor) { + private var listView: ListView? = null + private val items: MutableList = mutableListOf() + + /** + * A scheduled-but-not-yet-started completion request, kept so a newer keystroke can cancel it. + * See [requireCompletion]. + */ + private var pendingCompletion: Runnable? = null + + companion object { + private val log = LoggerFactory.getLogger(EditorCompletionWindow::class.java) + + /** Quiet period for coalescing a keystroke burst: analysis runs only after typing pauses this long. */ + private const val COMPLETION_DEBOUNCE_MS = 80L + } + + init { + setLayout(EditorCompletionLayout()) + setEnabledAnimation(true) + } + + override fun isShowing(): Boolean { + @Suppress("UNNECESSARY_SAFE_CALL", "USELESS_ELVIS") + return popup?.isShowing ?: false + } + + override fun setLayout(layout: CompletionLayout) { + super.setLayout(layout) + (layout.completionList as? ListView)?.let { + listView = it + it.adapter = this.adapter + it.setOnItemLongClickListener { _, view, position, _ -> + + val category = + when (editor.file?.extension) { + "java" -> TooltipCategory.CATEGORY_JAVA + "kt" -> TooltipCategory.CATEGORY_KOTLIN + "xml" -> TooltipCategory.CATEGORY_XML + else -> TooltipCategory.CATEGORY_IDE + } + + val completionItem = + items[position] as? com.itsaky.androidide.lsp.models.CompletionItem + val completionData = completionItem?.data + + val tag = + if (completionData == null) { + val label = completionItem?.ideLabel ?: "" + val attrName = if (label.contains(':')) label.substringAfterLast(':') else label + attrName.ifEmpty { null } + } else { + DocumentationReferenceProvider.getTag(completionData) + } + + // Dismiss the completion window before showing tooltip + hide() + + Log.d("EditorCompletionWindow", "Showing tooltip for tag: $tag category: $category") + TooltipManager.showTooltip( + context = editor.context, + anchorView = editor, + category = category, + tag = tag ?: "", + ) + true + } + } + } + + override fun select(pos: Int): Boolean { + if (pos > adapter!!.count) { + return false + } + return try { + super.select(pos) + } catch (e: Throwable) { + log.warn("Unable to select completion item at {}", pos, e) + false + } + } + + override fun select(): Boolean = + try { + super.select() + } catch (e: Throwable) { + log.warn("Unable to select completion item", e) + false + } + + override fun cancelCompletion() { + // Drop any request that was scheduled but hasn't started yet. + pendingCompletion?.let { editor.handler.removeCallbacks(it) } + pendingCompletion = null + if (completionThread != null) { + ProgressManager.instance.cancel(completionThread) + } + super.cancelCompletion() + } + + /** Whether completion may run now; hides the window when the cursor is selected or otherwise not applicable. */ + private fun canStartCompletion(): Boolean { + if (cancelShowUp || !isEnabled || !editor.isAttachedToWindow) { + return false + } + if (editor.text.cursor.isSelected || checkNoCompletion()) { + hide() + return false + } + return true + } + + /** + * Coalesces a keystroke burst into one completion for the latest cursor position, keeping at most one + * analysis in flight. This prevents the CompletionThread/allocation pile-up that saturated the heap and + * froze the editor during fast typing. + */ + override fun requireCompletion() { + if (!canStartCompletion()) { + return + } + + // cancelCompletion() clears any in-flight and pending request, so we then schedule exactly one. + cancelCompletion() + + val request = Runnable { startCompletion() } + pendingCompletion = request + editor.handler.postDelayed(request, COMPLETION_DEBOUNCE_MS) + } + + /** Runs on the UI thread. */ + private fun startCompletion() { + pendingCompletion = null + + // Editor state may have changed during the debounce delay; re-check the guards. + if (!canStartCompletion()) { + return + } + + requestTime = System.nanoTime() + currentSelection = -1 + + publisher = + IDECompletionPublisher( + editor.handler, + { + val items = publisher.items + + this.items.apply { + clear() + addAll(items) + } + + if (lastAttachedItems == null || lastAttachedItems.get() != items) { + adapter.attachValues(this, items) + adapter.notifyDataSetInvalidated() + lastAttachedItems = WeakReference(items) + } else { + adapter.notifyDataSetChanged() + } + + val newHeight = (adapter!!.itemHeight * adapter!!.count).toFloat() + if (newHeight == 0F) { + hide() + } + + editor + .getComponent(EditorAutoCompletion::class.java) + .updateCompletionWindowPosition() + setSize(width, min(newHeight, maxHeight.toFloat()).toInt()) + if (!isShowing) { + show() + } + + if (adapter!!.count >= 1 && + KeyboardUtils.isHardKeyboardConnected(context) + ) { + currentSelection = 0 + } + }, + editor.editorLanguage.interruptionLevel, + ) + + publisher.setUpdateThreshold(1) + + completionThread = CompletionThread(requestTime, publisher) + completionThread.name = "CompletionThread-$requestTime" + + setLoading(true) + + completionThread.start() + } } 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 50790cf8ff..84079924fa 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 @@ -64,192 +64,192 @@ import kotlin.time.Duration.Companion.milliseconds * @param jdkRelease The JDK release version at [jdkHome]. */ internal class CompilationEnvironment( - name: String, - kind: CompilationKind, - private val workspace: Workspace, - val ktProject: KotlinProjectModel, - intellijPluginRoot: Path, - jdkHome: Path, - jdkRelease: Int, - languageVersion: LanguageVersion = DEFAULT_LANGUAGE_VERSION, - enableParserEventSystem: Boolean = true, - val coroutineScope: CoroutineScope = - CoroutineScope( - SupervisorJob() + CoroutineName("CompilationEnv[$name]") + - CoroutineExceptionHandler { _, t -> - // Defense in depth: swallow (but log) non-cancellation failures from the - // debounce worker so a ClosedReceiveChannelException can never crash the app. - if (t !is CancellationException) { - logger.warn( - "Uncaught exception in compilation environment coroutine", - t - ) - } - }, - ), + name: String, + kind: CompilationKind, + private val workspace: Workspace, + val ktProject: KotlinProjectModel, + intellijPluginRoot: Path, + jdkHome: Path, + jdkRelease: Int, + languageVersion: LanguageVersion = DEFAULT_LANGUAGE_VERSION, + enableParserEventSystem: Boolean = true, + val coroutineScope: CoroutineScope = + CoroutineScope( + SupervisorJob() + CoroutineName("CompilationEnv[$name]") + + CoroutineExceptionHandler { _, t -> + // Defense in depth: swallow (but log) non-cancellation failures from the + // debounce worker so a ClosedReceiveChannelException can never crash the app. + if (t !is CancellationException) { + logger.warn( + "Uncaught exception in compilation environment coroutine", + t, + ) + } + }, + ), ) : AbstractCompilationEnvironment( - name = name, - kind = kind, - intellijPluginRoot = intellijPluginRoot, - jdkHome = jdkHome, - jdkRelease = jdkRelease, - languageVersion = languageVersion, - applicationEnvironmentMode = KotlinCoreApplicationEnvironmentMode.Production, - enableParserEventSystem = enableParserEventSystem, -), - KotlinProjectModel.ProjectModelListener { - companion object { - val DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION = 400.milliseconds - private val logger = LoggerFactory.getLogger(CompilationEnvironment::class.java) - } - - private var _languageClient: ILanguageClient? = null - - val fileAnalyzer: KeyedDebouncingAction - - val refreshScheduler: KeyedDebouncingAction - - val libraryIndex: JvmSymbolIndex? - get() = ktProject.libraryIndex - - val requireLibraryIndex: JvmSymbolIndex - get() = checkNotNull(libraryIndex) - - val sourceIndex: JvmSymbolIndex? - get() = ktProject.sourceIndex - - val requireSourceIndex: JvmSymbolIndex - get() = checkNotNull(sourceIndex) - - val fileIndex: KtFileMetadataIndex? - get() = ktProject.fileIndex - - val requireFileIndex: KtFileMetadataIndex - get() = checkNotNull(fileIndex) - - val generatedIndex: JvmSymbolIndex? - get() = ktProject.generatedIndex - - val symbolVisibilityChecker: SymbolVisibilityChecker by lazy { - SymbolVisibilityChecker(ProjectStructureProvider.getInstance(project)) - } - - var languageClient: ILanguageClient? - get() = _languageClient - set(value) { - _languageClient = value - } - - init { - initialize(::buildModules, ::buildKtSymbolIndex) - } - - @OptIn(KaImplementationDetail::class) - @Suppress("UNUSED_PARAMETER") - private fun buildKtSymbolIndex( - modules: List, - libraryRoots: List, - ): KtSymbolIndex = - KtSymbolIndex( - kind = kind, - project = project, - modules = modules, - fileIndex = requireFileIndex, - sourceIndex = requireSourceIndex, - libraryIndex = requireLibraryIndex, - ) - - private fun buildModules( - project: MockProject, - applicationEnv: KotlinCoreApplicationEnvironment, - ): List = workspace.collectKtModules(project, applicationEnv) - - override fun createServiceRegistrars() = - listOf(LspAnalysisApiServiceRegistrar(AnalysisApiServiceProviders.Production)) - - override fun createMessageCollector(): MessageCollector = - object : MessageCollector { - override fun clear() {} - - override fun hasErrors() = false - - override fun report( - severity: CompilerMessageSeverity, - message: String, - location: CompilerMessageSourceLocation?, - ) { - logger.info("[{}] {} ({})", severity.name, message, location) - } - } - - override fun postInit(libraryRoots: List) { - ktSymbolIndex.syncIndexInBackground() - } - - init { - fileAnalyzer = KeyedDebouncingAction( - scope = coroutineScope, - debounceDuration = DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION, - ) { path, cancelChecker -> - try { - val result = collectDiagnosticsFor(path, cancelChecker) - withContext(Dispatchers.Main.immediate) { - languageClient?.publishDiagnostics(result) - } - } catch (e: AnalysisPreemptedException) { - // Preempted by completion; re-schedule so diagnostics still run once it finishes. - logger.debug("diagnostics for {} preempted; rescheduling", path) - fileAnalyzer.schedule(path) - } - } - - refreshScheduler = - KeyedDebouncingAction( - scope = coroutineScope, - debounceDuration = DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION, - ) { path, _ -> - // Pull through the cache so a refresh (and its reindex) happens after every edit, - // independent of whether diagnostics run. - ktSymbolIndex.getCurrentKtFile(path).await() - } - } - - fun refreshSources() { + name = name, + kind = kind, + intellijPluginRoot = intellijPluginRoot, + jdkHome = jdkHome, + jdkRelease = jdkRelease, + languageVersion = languageVersion, + applicationEnvironmentMode = KotlinCoreApplicationEnvironmentMode.Production, + enableParserEventSystem = enableParserEventSystem, + ), + KotlinProjectModel.ProjectModelListener { + companion object { + val DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION = 400.milliseconds + private val logger = LoggerFactory.getLogger(CompilationEnvironment::class.java) + } + + private var _languageClient: ILanguageClient? = null + + val fileAnalyzer: KeyedDebouncingAction + + val refreshScheduler: KeyedDebouncingAction + + val libraryIndex: JvmSymbolIndex? + get() = ktProject.libraryIndex + + val requireLibraryIndex: JvmSymbolIndex + get() = checkNotNull(libraryIndex) + + val sourceIndex: JvmSymbolIndex? + get() = ktProject.sourceIndex + + val requireSourceIndex: JvmSymbolIndex + get() = checkNotNull(sourceIndex) + + val fileIndex: KtFileMetadataIndex? + get() = ktProject.fileIndex + + val requireFileIndex: KtFileMetadataIndex + get() = checkNotNull(fileIndex) + + val generatedIndex: JvmSymbolIndex? + get() = ktProject.generatedIndex + + val symbolVisibilityChecker: SymbolVisibilityChecker by lazy { + SymbolVisibilityChecker(ProjectStructureProvider.getInstance(project)) + } + + var languageClient: ILanguageClient? + get() = _languageClient + set(value) { + _languageClient = value + } + + init { + initialize(::buildModules, ::buildKtSymbolIndex) + } + + @OptIn(KaImplementationDetail::class) + @Suppress("UNUSED_PARAMETER") + private fun buildKtSymbolIndex( + modules: List, + libraryRoots: List, + ): KtSymbolIndex = + KtSymbolIndex( + kind = kind, + project = project, + modules = modules, + fileIndex = requireFileIndex, + sourceIndex = requireSourceIndex, + libraryIndex = requireLibraryIndex, + ) + + private fun buildModules( + project: MockProject, + applicationEnv: KotlinCoreApplicationEnvironment, + ): List = workspace.collectKtModules(project, applicationEnv) + + override fun createServiceRegistrars() = listOf(LspAnalysisApiServiceRegistrar(AnalysisApiServiceProviders.Production)) + + override fun createMessageCollector(): MessageCollector = + object : MessageCollector { + override fun clear() {} + + override fun hasErrors() = false + + override fun report( + severity: CompilerMessageSeverity, + message: String, + location: CompilerMessageSourceLocation?, + ) { + logger.info("[{}] {} ({})", severity.name, message, location) + } + } + + override fun postInit(libraryRoots: List) { + ktSymbolIndex.syncIndexInBackground() + } + + init { + fileAnalyzer = + KeyedDebouncingAction( + scope = coroutineScope, + debounceDuration = DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION, + ) { path, cancelChecker -> + try { + val result = collectDiagnosticsFor(path, cancelChecker) + withContext(Dispatchers.Main.immediate) { + languageClient?.publishDiagnostics(result) + } + } catch (e: AnalysisPreemptedException) { + // Preempted by completion; re-schedule so diagnostics still run once it finishes. + logger.debug("diagnostics for {} preempted; rescheduling", path) + fileAnalyzer.schedule(path) + } + } + + refreshScheduler = + KeyedDebouncingAction( + scope = coroutineScope, + debounceDuration = DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION, + ) { path, _ -> + // Pull through the cache so a refresh (and its reindex) happens after every edit, + // independent of whether diagnostics run. + ktSymbolIndex.getCurrentKtFile(path).await() + } + } + + fun refreshSources() { Sentry.addBreadcrumb("refreshSources (env=$name, modules=${modules.size})") - project.write { + project.write { Sentry.addBreadcrumb("refreshSources(env=$name): in-progress") - ResolutionScopeProvider.getInstance(project).invalidateAll() + ResolutionScopeProvider.getInstance(project).invalidateAll() modules .asFlatSequence() - .filterIsInstance() - .forEach { it.invalidateSearchScope() } - } - ktSymbolIndex.refreshSources() - } + .filterIsInstance() + .forEach { it.invalidateSearchScope() } + } + ktSymbolIndex.refreshSources() + } - fun openFileIfNeeded(path: Path) { + fun openFileIfNeeded(path: Path) { fileAnalyzer.schedule(path) - } + } - fun onFileOpen(path: Path) { - fileAnalyzer.schedule(path) - } + fun onFileOpen(path: Path) { + fileAnalyzer.schedule(path) + } - fun onFileSaved(path: Path) { - fileAnalyzer.schedule(path) - } + fun onFileSaved(path: Path) { + fileAnalyzer.schedule(path) + } - fun onFileClosed(path: Path) { - fileAnalyzer.cancelPending(path) + fun onFileClosed(path: Path) { + fileAnalyzer.cancelPending(path) refreshScheduler.cancelPending(path) ktSymbolIndex.invalidateCurrent(path) - } + } - @OptIn(KaImplementationDetail::class) - private inline fun notifyElementModifiedForPath( - path: Path, - crossinline typeProvider: (KtFile) -> KaElementModificationType, - ) { + @OptIn(KaImplementationDetail::class) + private inline fun notifyElementModifiedForPath( + path: Path, + crossinline typeProvider: (KtFile) -> KaElementModificationType, + ) { // Resolve PSI/module structure under the read lock; driving psiManager.findFile / // structureProvider concurrently with an `analyze` read section otherwise races. val (ktFile, module) = @@ -269,84 +269,84 @@ internal class CompilationEnvironment( ktFile to module } - project.write { - // Must run under the write lock so the session mutation can't race a concurrent + 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 KtSymbolIndex.refreshToCurrent. - if (ktFile != null) { + if (ktFile != null) { KaSourceModificationService .getInstance(project) - .handleElementModification(ktFile, typeProvider(ktFile)) - } - - if (module != null) { - module.invalidateSearchScope() - project.publishModificationEvent( - KotlinModuleStateModificationEvent( - module, - KotlinModuleStateModificationKind.UPDATE, + .handleElementModification(ktFile, typeProvider(ktFile)) + } + + if (module != null) { + module.invalidateSearchScope() + project.publishModificationEvent( + KotlinModuleStateModificationEvent( + module, + KotlinModuleStateModificationKind.UPDATE, ), - ) - project.analysisMessageBus - .syncPublisher(LLFirSessionInvalidationTopics.SESSION_INVALIDATION) - .afterInvalidation(setOf(module)) - ResolutionScopeProvider.getInstance(project).invalidate(module) - } else { - project.analysisMessageBus - .syncPublisher(LLFirSessionInvalidationTopics.SESSION_INVALIDATION) - .afterGlobalInvalidation() - ResolutionScopeProvider.getInstance(project).invalidateAll() - } - } - } - - suspend fun onFileCreated(path: Path) { - notifyElementModifiedForPath(path) { KaElementModificationType.ElementAdded } - ktSymbolIndex.submitForIndexing(path) - } - - suspend fun onFileRemoved(path: Path) { - notifyElementModifiedForPath(path) { ktFile -> - KaElementModificationType.ElementRemoved(ktFile) - } - ProjectStructureProvider.getInstance(project).unregisterInMemoryFile(path.pathString) - ktSymbolIndex.removeFromIndex(path) - } + ) + project.analysisMessageBus + .syncPublisher(LLFirSessionInvalidationTopics.SESSION_INVALIDATION) + .afterInvalidation(setOf(module)) + ResolutionScopeProvider.getInstance(project).invalidate(module) + } else { + project.analysisMessageBus + .syncPublisher(LLFirSessionInvalidationTopics.SESSION_INVALIDATION) + .afterGlobalInvalidation() + ResolutionScopeProvider.getInstance(project).invalidateAll() + } + } + } + + suspend fun onFileCreated(path: Path) { + notifyElementModifiedForPath(path) { KaElementModificationType.ElementAdded } + ktSymbolIndex.submitForIndexing(path) + } + + suspend fun onFileRemoved(path: Path) { + notifyElementModifiedForPath(path) { ktFile -> + KaElementModificationType.ElementRemoved(ktFile) + } + ProjectStructureProvider.getInstance(project).unregisterInMemoryFile(path.pathString) + ktSymbolIndex.removeFromIndex(path) + } suspend fun onFileMoved( fromPath: Path, toPath: Path, ) { val isFileOpen = FileManager.isActive(fromPath) - onFileRemoved(fromPath) - onFileCreated(toPath) - if (isFileOpen) { + onFileRemoved(fromPath) + onFileCreated(toPath) + if (isFileOpen) { ktSymbolIndex.invalidateCurrent(fromPath) - onFileOpen(toPath) - } - } + onFileOpen(toPath) + } + } - fun onFileContentChanged(path: Path) { + fun onFileContentChanged(path: Path) { refreshScheduler.schedule(path) fileAnalyzer.schedule(path) - } - - override fun close() { - ktProject.removeListener(this) - - // fileAnalyzer reads the project (collectDiagnosticsFor). Cancel AND join it before - // super.close() disposes the project, so an in-flight read can't touch a disposed project - // (APPDEVFORALL-17R / ADFA-4384). Bounded so a slow read can't block shutdown indefinitely. - runBlocking { - withTimeoutOrNull(CLOSE_DRAIN_TIMEOUT) { - coroutineScope.coroutineContext[Job]?.cancelAndJoin() - } - } - - super.close() - } - - override fun onProjectModelChanged( - model: KotlinProjectModel, - changeKind: KotlinProjectModel.ChangeKind, - ) = Unit + } + + override fun close() { + ktProject.removeListener(this) + + // fileAnalyzer reads the project (collectDiagnosticsFor). Cancel AND join it before + // super.close() disposes the project, so an in-flight read can't touch a disposed project + // (APPDEVFORALL-17R / ADFA-4384). Bounded so a slow read can't block shutdown indefinitely. + runBlocking { + withTimeoutOrNull(CLOSE_DRAIN_TIMEOUT) { + coroutineScope.coroutineContext[Job]?.cancelAndJoin() + } + } + + super.close() + } + + override fun onProjectModelChanged( + model: KotlinProjectModel, + changeKind: KotlinProjectModel.ChangeKind, + ) = Unit } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt index b5c7aefba0..6f6099b20c 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt @@ -35,134 +35,138 @@ internal class IndexWorker( val path: Path, val ktFile: KtFile, ) { - override fun equals(other: Any?): Boolean { - return path == (other as? ModFileIndexKey)?.path - } + override fun equals(other: Any?): Boolean = path == (other as? ModFileIndexKey)?.path - override fun hashCode(): Int { - return path.hashCode() - } + override fun hashCode(): Int = path.hashCode() operator fun component1() = path + operator fun component2() = ktFile } - suspend fun start() = coroutineScope { - var scanCount = 0 - var sourceIndexCount = 0 - - val modifiedFileIndexer = KeyedDebouncingAction( - scope = scope, - debounceDuration = CompilationEnvironment.DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION - ) { (path, ktFile), cancelChecker -> - logger.debug("Indexing modified file: {}", path) - try { - indexSourceFile(project, ktFile, fileIndex, sourceIndex, cancelChecker) - sourceIndexCount++ - } catch (e: AnalysisPreemptedException) { - // Preempted by higher-priority analysis; re-queue so the edit still gets indexed. - logger.debug("Indexing of modified file {} preempted; re-queueing", path) - scope.launch { submitCommand(IndexCommand.IndexModifiedFile(ktFile)) } - } - } - - while (isActive) { - // Defensive guard: if the project was disposed out from under us (e.g. a disposal - // path that didn't first drain this worker), stop instead of calling PsiManager on a - // disposed project, which throws "Project is already disposed" (APPDEVFORALL-17R). - if (project.isDisposed) break - - when (val cmd = queue.take()) { - is IndexCommand.RemoveFromIndex -> { - applyRemovals( - first = cmd, - fileIndex = fileIndex, - sourceIndex = sourceIndex, - pollNext = { queue.pollIndexQueue() }, - pushBack = { queue.pushBackIndexQueue(it) }, - ) - } - - is IndexCommand.IndexSourceFile -> { - if (cmd.vf.fileSystem.protocol != "file") { - logger.warn("Unknown source file protocol: {}", cmd.vf.path) - continue + suspend fun start() = + coroutineScope { + var scanCount = 0 + var sourceIndexCount = 0 + + val modifiedFileIndexer = + KeyedDebouncingAction( + scope = scope, + debounceDuration = CompilationEnvironment.DEFAULT_FILE_MOD_EVENT_DEBOUNCE_DURATION, + ) { (path, ktFile), cancelChecker -> + logger.debug("Indexing modified file: {}", path) + try { + indexSourceFile(project, ktFile, fileIndex, sourceIndex, cancelChecker) + sourceIndexCount++ + } catch (e: AnalysisPreemptedException) { + // Preempted by higher-priority analysis; re-queue so the edit still gets indexed. + logger.debug("Indexing of modified file {} preempted; re-queueing", path) + scope.launch { submitCommand(IndexCommand.IndexModifiedFile(ktFile)) } } + } - if (project.isDisposed) break + while (isActive) { + // Defensive guard: if the project was disposed out from under us (e.g. a disposal + // path that didn't first drain this worker), stop instead of calling PsiManager on a + // disposed project, which throws "Project is already disposed" (APPDEVFORALL-17R). + if (project.isDisposed) break - val ktFile = project.read { - PsiManager.getInstance(project) - .findFile(cmd.vf) as? KtFile + when (val cmd = queue.take()) { + is IndexCommand.RemoveFromIndex -> { + applyRemovals( + first = cmd, + fileIndex = fileIndex, + sourceIndex = sourceIndex, + pollNext = { queue.pollIndexQueue() }, + pushBack = { queue.pushBackIndexQueue(it) }, + ) } - if (ktFile == null) { - // probably a non-kotlin file - continue + is IndexCommand.IndexSourceFile -> { + if (cmd.vf.fileSystem.protocol != "file") { + logger.warn("Unknown source file protocol: {}", cmd.vf.path) + continue + } + + if (project.isDisposed) break + + val ktFile = + project.read { + PsiManager + .getInstance(project) + .findFile(cmd.vf) as? KtFile + } + + if (ktFile == null) { + // probably a non-kotlin file + continue + } + + try { + indexSourceFile( + project = project, + ktFile = ktFile, + fileIndex = fileIndex, + symbolsIndex = sourceIndex, + // A real (cancellable) checker so the scheduler can preempt this pass + // in favour of completion/diagnostics. + cancelChecker = ICancelChecker.Default(), + ) + + sourceIndexCount++ + } catch (e: AnalysisPreemptedException) { + // Preempted by higher-priority analysis; re-queue so the file still gets indexed. + logger.debug("Indexing of {} preempted; re-queueing", cmd.vf.path) + scope.launch { submitCommand(cmd) } + } } - try { - indexSourceFile( - project = project, - ktFile = ktFile, - fileIndex = fileIndex, - symbolsIndex = sourceIndex, - // A real (cancellable) checker so the scheduler can preempt this pass - // in favour of completion/diagnostics. - cancelChecker = ICancelChecker.Default() + is IndexCommand.IndexModifiedFile -> { + modifiedFileIndexer.schedule( + ModFileIndexKey( + cmd.ktFile.backingFilePath!!, + cmd.ktFile, + ), ) - - sourceIndexCount++ - } catch (e: AnalysisPreemptedException) { - // Preempted by higher-priority analysis; re-queue so the file still gets indexed. - logger.debug("Indexing of {} preempted; re-queueing", cmd.vf.path) - scope.launch { submitCommand(cmd) } } - } - is IndexCommand.IndexModifiedFile -> { - modifiedFileIndexer.schedule( - ModFileIndexKey( - cmd.ktFile.backingFilePath!!, - cmd.ktFile + IndexCommand.IndexingComplete -> { + logger.info( + "Indexing complete: scanned={}, sourceIndexCount={}", + scanCount, + sourceIndexCount, ) - ) - } + } - IndexCommand.IndexingComplete -> { - logger.info( - "Indexing complete: scanned={}, sourceIndexCount={}", - scanCount, - sourceIndexCount, - ) - } + is IndexCommand.ScanSourceFile -> { + if (project.isDisposed) break - is IndexCommand.ScanSourceFile -> { - if (project.isDisposed) break + val ktFile = + project.read { + PsiManager.getInstance(project).findFile(cmd.vf) as? KtFile + } + ?: continue - val ktFile = project.read { - PsiManager.getInstance(project).findFile(cmd.vf) as? KtFile - } - ?: continue + val newFile = ktFile.toMetadata(project, isIndexed = false) + val existingFile = fileIndex.get(newFile.filePath) + if (KtFileMetadata.shouldBeSkipped(existingFile, newFile)) { + continue + } - val newFile = ktFile.toMetadata(project, isIndexed = false) - val existingFile = fileIndex.get(newFile.filePath) - if (KtFileMetadata.shouldBeSkipped(existingFile, newFile)) { - continue + fileIndex.upsert(newFile) + scanCount++ } - fileIndex.upsert(newFile) - scanCount++ - } + IndexCommand.SourceScanningComplete -> { + logger.info("Scanning complete. Found {} files to index.", scanCount) + } - IndexCommand.SourceScanningComplete -> { - logger.info("Scanning complete. Found {} files to index.", scanCount) + IndexCommand.Stop -> { + break + } } - - IndexCommand.Stop -> break } } - } suspend fun submitCommand(cmd: IndexCommand) { when (cmd) { diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index 18330719b4..a4f3afef95 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -21,7 +21,9 @@ import kotlin.concurrent.withLock * [DIAGNOSTICS]/[INDEXING], whose preempted work is re-queued — there same-priority preemption would * livelock, two contenders endlessly re-queuing and re-preempting each other. */ -internal enum class AnalysisPriority(val supersedesSamePriority: Boolean) { +internal enum class AnalysisPriority( + val supersedesSamePriority: Boolean, +) { INDEXING(supersedesSamePriority = false), DIAGNOSTICS(supersedesSamePriority = false), INTERACTIVE(supersedesSamePriority = true), @@ -33,8 +35,7 @@ internal enum class AnalysisPriority(val supersedesSamePriority: Boolean) { * cleanly through the existing cancellation-aware `catch` blocks; callers that want the preempted work * to run later catch this specific type and re-schedule it. */ -internal class AnalysisPreemptedException : - CancellationException("analysis preempted by a higher-priority request") +internal class AnalysisPreemptedException : CancellationException("analysis preempted by a higher-priority request") /** * An [ICancelChecker] that adds a cooperative *preemption* signal on top of an existing [delegate] @@ -50,7 +51,6 @@ internal class AnalysisPreemptedException : internal class ScheduledCancelChecker( private val delegate: ICancelChecker, ) : ICancelChecker { - @Volatile private var preempted = false @@ -112,7 +112,6 @@ internal class ScheduledCancelChecker( * Access it through [withAnalysisLock] / [analyzeMaybeDangling] rather than directly. */ internal object AnalysisScheduler { - /** Upper bound on how long a queued requester waits before re-checking its cancellation. */ private const val WAIT_POLL_MILLIS = 25L @@ -139,7 +138,11 @@ internal object AnalysisScheduler { * this completion. This stops superseded completions from piling up holding heavy state (KtFile copies, * symbol lists), which on-device saturated the heap and triggered multi-second GC stalls. */ - fun acquire(priority: AnalysisPriority, cancelChecker: ICancelChecker, onPreempt: () -> Unit) { + fun acquire( + priority: AnalysisPriority, + cancelChecker: ICancelChecker, + onPreempt: () -> Unit, + ) { mutex.withLock { val me = Thread.currentThread() if (holderThread === me) { @@ -156,8 +159,10 @@ internal object AnalysisScheduler { val hp = holderPriority if (holderThread != null && hp != null && !holderPreempted && - (hp.ordinal < priority.ordinal || - (hp == priority && priority.supersedesSamePriority)) + ( + hp.ordinal < priority.ordinal || + (hp == priority && priority.supersedesSamePriority) + ) ) { // Signal the holder to bail (once): either it is strictly lower priority, or a // newer same-priority request supersedes it (completion only). diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java index ddf18687e9..730a9e586c 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisThreadContext.java @@ -8,26 +8,21 @@ /** * Java bridge to the embeddable IntelliJ {@link ThreadContext} coroutine-context API. * - *

Exists in Java because {@code currentThreadContext}/{@code installThreadContext} live in a - * Kotlin file-facade whose metadata this module's Kotlin compiler cannot resolve ("unresolved - * reference"), yet at the bytecode level they are plain {@code public static} methods Java can call. + *

+ * Exists in Java because {@code currentThreadContext}/{@code installThreadContext} live in a Kotlin file-facade whose metadata this module's Kotlin compiler cannot resolve ("unresolved reference"), yet at the bytecode level they are plain {@code public static} methods Java can call. */ public final class AnalysisThreadContext { - private AnalysisThreadContext() { - } + /** + * Installs {@code job} into the current thread's IntelliJ coroutine context (preserving any existing context) and returns a token that restores the previous context when closed. Cancelling {@code job} then aborts the running analysis mid-{@code analyze}, since the embeddable {@code CoreProgressManager.checkCanceled()} throws once the installed Job is cancelled. + * + *

+ * Public (not package-private) because the {@code internal inline} {@code withAnalysisLock} references it: an inline function may only reference declarations at least as accessible as itself. + */ + public static AccessToken installJob(Job job) { + CoroutineContext context = ThreadContext.currentThreadContext().plus(job); + return ThreadContext.installThreadContext(context, true); + } - /** - * Installs {@code job} into the current thread's IntelliJ coroutine context (preserving any - * existing context) and returns a token that restores the previous context when closed. Cancelling - * {@code job} then aborts the running analysis mid-{@code analyze}, since the embeddable - * {@code CoreProgressManager.checkCanceled()} throws once the installed Job is cancelled. - * - *

Public (not package-private) because the {@code internal inline} {@code withAnalysisLock} - * references it: an inline function may only reference declarations at least as accessible as itself. - */ - public static AccessToken installJob(Job job) { - CoroutineContext context = ThreadContext.currentThreadContext().plus(job); - return ThreadContext.installThreadContext(context, true); - } + private AnalysisThreadContext() {} } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt index 8b12678094..5c78877e27 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/CancelCheckerProgressIndicator.kt @@ -18,7 +18,6 @@ import org.jetbrains.kotlin.com.intellij.openapi.progress.util.AbstractProgressI internal class CancelCheckerProgressIndicator( private val checker: ICancelChecker, ) : AbstractProgressIndicatorBase() { - override fun isCanceled(): Boolean = super.isCanceled() || checker.isCancelled() override fun checkCanceled() { 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 dba236f521..7bf3f7f473 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 @@ -1,13 +1,13 @@ package com.itsaky.androidide.lsp.kotlin.compiler.modules +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Job import org.jetbrains.kotlin.analysis.api.KaSession import org.jetbrains.kotlin.analysis.api.analyze import org.jetbrains.kotlin.analysis.api.analyzeCopy import org.jetbrains.kotlin.analysis.api.projectStructure.KaDanglingFileResolutionMode import org.jetbrains.kotlin.analysis.api.projectStructure.copyOrigin import org.jetbrains.kotlin.analysis.api.projectStructure.isDangling -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.Job import org.jetbrains.kotlin.com.intellij.openapi.progress.ProcessCanceledException import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager import org.jetbrains.kotlin.com.intellij.openapi.util.Key @@ -76,7 +76,8 @@ internal inline fun withAnalysisLock( val holder = arrayOfNulls(1) try { AnalysisThreadContext.installJob(job).use { - ProgressManager.getInstance() + ProgressManager + .getInstance() .executeProcessUnderProgress({ holder[0] = action() }, indicator) } } catch (e: ProcessCanceledException) { 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 7d2f646c38..74e11ba22a 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 @@ -1,3 +1,5 @@ +@file:Suppress("ktlint:standard:max-line-length") + package com.itsaky.androidide.lsp.kotlin.completion import com.itsaky.androidide.lookup.Lookup @@ -93,7 +95,8 @@ private fun abortIfCancelled() { if (checker != null) { checker.abortIfCancelled() } else { - Lookup.getDefault() + Lookup + .getDefault() .lookup(ICancelChecker::class.java) ?.abortIfCancelled() } @@ -105,8 +108,7 @@ private fun abortIfCancelled() { * sora-publisher-specific [CompletionCancelledException] is layered on here. All mean * "superseded/cancelled"; treat them uniformly so none is logged as a spurious error. */ -private fun Throwable.isCancellation(): Boolean = - isAnalysisCancellation() || this is CompletionCancelledException +private fun Throwable.isCancellation(): Boolean = isAnalysisCancellation() || this is CompletionCancelledException /** * Provide code completion for the given completion parameters. @@ -158,30 +160,34 @@ internal fun doComplete(params: CompletionParams): CompletionResult { abortIfCancelled() // insert placeholder to fix broken trees - val textWithPlaceholder = buildString { - append(originalText, 0, completionOffset) - append(KT_COMPLETION_PLACEHOLDER) - append(originalText, completionOffset, originalText.length) - } + val textWithPlaceholder = + buildString { + append(originalText, 0, completionOffset) + append(KT_COMPLETION_PLACEHOLDER) + append(originalText, completionOffset, originalText.length) + } - val completionKtFile = env.project.read { - env.parser.createFile( - fileName = params.file.name, - text = textWithPlaceholder - ).apply { - originalFile = ktFile - originalKtFile = ktFile + val completionKtFile = + env.project.read { + env.parser + .createFile( + fileName = params.file.name, + text = textWithPlaceholder, + ).apply { + originalFile = ktFile + originalKtFile = ktFile + } } - } abortIfCancelled() // Use the request-scoped checker on params, not the global Lookup: Lookup holds one ICancelChecker // updated per request, so with concurrent completions an older request could read a newer request's // checker and never observe its own cancellation. Fall back to Lookup only for a NOOP checker (tests). - val delegate = params.cancelChecker.takeUnless { it === ICancelChecker.NOOP } - ?: Lookup.getDefault().lookup(ICancelChecker::class.java) - ?: ICancelChecker.NOOP + val delegate = + params.cancelChecker.takeUnless { it === ICancelChecker.NOOP } + ?: Lookup.getDefault().lookup(ICancelChecker::class.java) + ?: ICancelChecker.NOOP val cancelChecker = ScheduledCancelChecker(delegate) currentCancelChecker.set(cancelChecker) @@ -196,14 +202,14 @@ internal fun doComplete(params: CompletionParams): CompletionResult { file = params.file, ktFile = completionKtFile, offset = completionOffset, - partial = partial + partial = partial, ) if (ctx == null) { logger.error( "Unable to determine context at offset {} in file {}", completionOffset, - params.file + params.file, ) return@analyzeMaybeDangling CompletionResult.EMPTY } @@ -213,11 +219,13 @@ internal fun doComplete(params: CompletionParams): CompletionResult { val items = mutableListOf() val completionContext = determineCompletionContext(ctx.psiElement) when (completionContext) { - CompletionContext.Scope -> + CompletionContext.Scope -> { collectScopeCompletions(to = items) + } - CompletionContext.Member -> + CompletionContext.Member -> { collectMemberCompletions(to = items) + } } CompletionResult(items) @@ -237,9 +245,7 @@ internal fun doComplete(params: CompletionParams): CompletionResult { } context(ctx: AnalysisContext) -private fun KaSession.collectMemberCompletions( - to: MutableList -) { +private fun KaSession.collectMemberCompletions(to: MutableList) { abortIfCancelled() val qualifiedExpr = ctx.psiElement.getParentOfType(strict = false) if (qualifiedExpr == null) { @@ -260,7 +266,7 @@ private fun KaSession.collectMemberCompletions( receiver, receiverType, receiver.text, - ctx.partial + ctx.partial, ) collectMembersFromType(receiverType, to) @@ -273,18 +279,19 @@ private fun KaSession.collectMemberCompletions( collectExtensionFunctions(receiverType, to) } -context(ctx: AnalysisContext) @OptIn(KaExperimentalApi::class) +context(ctx: AnalysisContext) private fun KaSession.collectMembersFromType( receiverType: KaType, - to: MutableList + to: MutableList, ) { abortIfCancelled() val typeScope = receiverType.scope if (typeScope != null) { val callables = - typeScope.getCallableSignatures { name -> matchesFilter(name) } + typeScope + .getCallableSignatures { name -> matchesFilter(name) } .map { it.symbol } val classifiers = @@ -310,10 +317,11 @@ private fun KaSession.collectMembersFromType( context(ctx: AnalysisContext) private fun KaSession.collectExtensionFunctions( receiverType: KaType, - to: MutableList + to: MutableList, ) { val extensionSymbols = - ctx.scope.callables { name -> matchesFilter(name) } + ctx.scope + .callables { name -> matchesFilter(name) } .filter { symbol -> if (!symbol.isExtension) return@filter false @@ -325,9 +333,7 @@ private fun KaSession.collectExtensionFunctions( } context(env: CompilationEnvironment, ctx: AnalysisContext) -private fun KaSession.collectScopeCompletions( - to: MutableList, -) { +private fun KaSession.collectScopeCompletions(to: MutableList) { if (ctx.partial.isBlank()) { logger.warn("cannot complete for blank partial candidate") return @@ -342,11 +348,12 @@ private fun KaSession.collectScopeCompletions( logger.info( "Complete scope members of {}: matching '{}'", ktElement, - ctx.partial + ctx.partial, ) val callables = - scope.callables { name -> matchesFilter(name) } + scope + .callables { name -> matchesFilter(name) } .filter { symbol -> abortIfCancelled() @@ -372,10 +379,11 @@ private fun KaSession.collectScopeCompletions( } context(env: CompilationEnvironment, ctx: AnalysisContext) -private fun KaSession.collectUnimportedSymbols( - to: MutableList -) { - val currentPackage = ctx.ktElement.containingKtFile.packageDirective?.fqName?.asString() +private fun KaSession.collectUnimportedSymbols(to: MutableList) { + val currentPackage = + ctx.ktElement.containingKtFile.packageDirective + ?.fqName + ?.asString() val useSiteModule = this.useSiteModule val visibilityChecker = env.symbolVisibilityChecker @@ -384,24 +392,28 @@ private fun KaSession.collectUnimportedSymbols( if (symbol.packageName == currentPackage) return - val isVisible = visibilityChecker.isVisible( - symbol = symbol, - useSiteModule = useSiteModule, - useSitePackage = currentPackage, - ) + val isVisible = + visibilityChecker.isVisible( + symbol = symbol, + useSiteModule = useSiteModule, + useSitePackage = currentPackage, + ) if (!isVisible) return buildUnimportedSymbolItem(symbol)?.let { to += it } } - env.libraryIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) + env.libraryIndex + ?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) - env.sourceIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) + env.sourceIndex + ?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) - env.generatedIndex?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) + env.generatedIndex + ?.findByPrefix(ctx.partial, limit = UNIMPORTED_SYMBOL_LIMIT) ?.forEach(::addCompletionItem) } @@ -428,16 +440,19 @@ private fun KaSession.buildUnimportedSymbolItem(symbol: JvmSymbol): CompletionIt // the extension property/function's receiver type // is not available in current context, so ignore this sym if (!satisfiesImplicitReceivers) return null - } else return null + } else { + return null + } } abortIfCancelled() } - val item = ktCompletionItem( - name = symbol.shortName, - kind = kindOf(symbol), - ) + val item = + ktCompletionItem( + name = symbol.shortName, + kind = kindOf(symbol), + ) item.overrideTypeText = symbol.returnTypeDisplay when (symbol.kind) { @@ -449,10 +464,11 @@ private fun KaSession.buildUnimportedSymbolItem(symbol: JvmSymbol): CompletionIt hasParams = data.parameterCount > 0, ) - item.additionalEditHandler = KotlinAutoImportEditHandler( - analysisContext = ctx, - symbolToImport = symbol - ) + item.additionalEditHandler = + KotlinAutoImportEditHandler( + analysisContext = ctx, + symbolToImport = symbol, + ) if (symbol.kind == JvmSymbolKind.CONSTRUCTOR) { item.overrideTypeText = symbol.shortName @@ -460,10 +476,11 @@ private fun KaSession.buildUnimportedSymbolItem(symbol: JvmSymbol): CompletionIt } in JvmSymbolKind.CALLABLE_KINDS -> { - item.additionalEditHandler = KotlinAutoImportEditHandler( - analysisContext = ctx, - symbolToImport = symbol - ) + item.additionalEditHandler = + KotlinAutoImportEditHandler( + analysisContext = ctx, + symbolToImport = symbol, + ) } JvmSymbolKind.TYPE_ALIAS -> { @@ -493,14 +510,12 @@ private fun internalNameToClassId(internalName: String): ClassId { return ClassId( packageFqName = FqName.fromSegments(packageName.split('/')), relativeClassName = FqName.fromSegments(relativeName.split('$')), - isLocal = isLocal + isLocal = isLocal, ) } context(ctx: AnalysisContext) -private fun KaSession.collectKeywordCompletions( - to: MutableList, -) { +private fun KaSession.collectKeywordCompletions(to: MutableList) { fun kwItem(name: String) = ktCompletionItem( name = name, @@ -520,54 +535,83 @@ private fun KaSession.collectKeywordCompletions( context(ctx: AnalysisContext) private fun KaSession.collectSnippetCompletions(to: MutableList) { - val snippets = buildList { - // add global snippets, if any - KotlinSnippetRepository.snippets[KotlinSnippetScope.GLOBAL]?.also { addAll(it) } + val snippets = + buildList { + // add global snippets, if any + KotlinSnippetRepository.snippets[KotlinSnippetScope.GLOBAL]?.also { addAll(it) } + + val snippetScope = + when (ctx.declarationKind) { + DeclarationKind.CLASS, + DeclarationKind.INTERFACE, + DeclarationKind.OBJECT, + DeclarationKind.ENUM_CLASS, + DeclarationKind.ANNOTATION_CLASS, + -> { + KotlinSnippetScope.MEMBER + } - val snippetScope = when (ctx.declarationKind) { - DeclarationKind.CLASS, - DeclarationKind.INTERFACE, - DeclarationKind.OBJECT, - DeclarationKind.ENUM_CLASS, - DeclarationKind.ANNOTATION_CLASS -> KotlinSnippetScope.MEMBER + DeclarationKind.CONSTRUCTOR, + DeclarationKind.FUN, + -> { + KotlinSnippetScope.LOCAL + } - DeclarationKind.CONSTRUCTOR, - DeclarationKind.FUN -> KotlinSnippetScope.LOCAL + DeclarationKind.UNKNOWN -> { + KotlinSnippetScope.TOP_LEVEL.takeIf { + ctx.declarationContext == DeclarationContext.TOP_LEVEL + } + } - DeclarationKind.UNKNOWN -> KotlinSnippetScope.TOP_LEVEL.takeIf { ctx.declarationContext == DeclarationContext.TOP_LEVEL } + DeclarationKind.PROPERTY_VAL -> { + null + } - DeclarationKind.PROPERTY_VAL -> null - DeclarationKind.PROPERTY_VAR -> null - DeclarationKind.TYPEALIAS -> null - } + DeclarationKind.PROPERTY_VAR -> { + null + } - logger.info( - "Adding completions for snippet scope: {} (context: {}, kind: {})", - snippetScope, - ctx.declarationContext, - ctx.declarationKind - ) + DeclarationKind.TYPEALIAS -> { + null + } + } - snippetScope?.let { scope -> KotlinSnippetRepository.snippets[scope]?.also { snippets -> addAll(snippets) } } - } + logger.info( + "Adding completions for snippet scope: {} (context: {}, kind: {})", + snippetScope, + ctx.declarationContext, + ctx.declarationKind, + ) + + snippetScope?.let { scope -> + KotlinSnippetRepository.snippets[scope]?.also { snippets -> + addAll( + snippets, + ) + } + } + } abortIfCancelled() val indent = computeIndentLevelAt(ctx.ktElement) for (snippet in snippets) { abortIfCancelled() - to += ktCompletionItem(snippet.prefix, CompletionItemKind.SNIPPET).apply { - detail = snippet.description - ideSortText = "00000${snippet.prefix}" - snippetDescription = describeSnippet(ctx.partial) - - val indentation = indentationString(indent) - insertTextFormat = InsertTextFormat.SNIPPET - insertText = snippet.body.joinToString(separator = System.lineSeparator()) { - it.replace("\t", indentation) - .replace("\n", "\n${indentation}") + to += + ktCompletionItem(snippet.prefix, CompletionItemKind.SNIPPET).apply { + detail = snippet.description + ideSortText = "00000${snippet.prefix}" + snippetDescription = describeSnippet(ctx.partial) + + val indentation = indentationString(indent) + insertTextFormat = InsertTextFormat.SNIPPET + insertText = + snippet.body.joinToString(separator = System.lineSeparator()) { + it + .replace("\t", indentation) + .replace("\n", "\n$indentation") + } } - } } } @@ -589,51 +633,50 @@ private fun computeIndentLevelAt(ktElement: KtElement): Int { return indentLevel } -context(ctx: AnalysisContext) @JvmName("callablesToCompletionItems") -private fun KaSession.toCompletionItems( - callables: Sequence, -): Sequence = +context(ctx: AnalysisContext) +private fun KaSession.toCompletionItems(callables: Sequence): Sequence = callables.mapNotNull { callableSymbolToCompletionItem(it) } -context(ctx: AnalysisContext) @JvmName("classifiersToCompletionItems") -private fun KaSession.toCompletionItems( - classifiers: Sequence, -): Sequence = +context(ctx: AnalysisContext) +private fun KaSession.toCompletionItems(classifiers: Sequence): Sequence = classifiers.mapNotNull { classifierSymbolToCompletionItem(it) } -context(ctx: AnalysisContext) @OptIn(KaExperimentalApi::class) -private fun KaSession.callableSymbolToCompletionItem( - symbol: KaCallableSymbol, -): CompletionItem? { +context(ctx: AnalysisContext) +private fun KaSession.callableSymbolToCompletionItem(symbol: KaCallableSymbol): CompletionItem? { val item = createSymbolCompletionItem(symbol) ?: return null val name = item.ideLabel item.overrideTypeText = renderName(symbol.returnType) when (symbol) { is KaNamedFunctionSymbol -> { - val params = symbol.valueParameters.joinToString(", ") { param -> - "${param.name.asString()}: ${renderName(param.returnType)}" - } + val params = + symbol.valueParameters.joinToString(", ") { param -> + "${param.name.asString()}: ${renderName(param.returnType)}" + } val hasParams = symbol.valueParameters.isNotEmpty() - item.detail = "${name}($params)" + item.detail = "$name($params)" item.setInsertTextForFunction(name, hasParams) - // TODO(itsaky): provide method completion data in order to show API info - // in completion items + /* + TODO(itsaky): provide method completion data in order to show API info + in completion items + */ } - // TODO: For properties, we can check if they're a compile-time constant - // and include that constant value in the "detail" field of the - // completion item + /* + TODO: For properties, we can check if they're a compile-time constant + and include that constant value in the "detail" field of the + completion item + */ else -> {} } @@ -647,11 +690,12 @@ private fun CompletionItem.setInsertTextForFunction( hasParams: Boolean, ) { insertTextFormat = InsertTextFormat.SNIPPET - insertText = if (hasParams) { - "${name}($0)" - } else { - "${name}()$0" - } + insertText = + if (hasParams) { + "$name($0)" + } else { + "$name()$0" + } snippetDescription = describeSnippet(prefix = ctx.partial, allowCommandExecution = true) @@ -660,21 +704,27 @@ private fun CompletionItem.setInsertTextForFunction( } } -context(ctx: AnalysisContext) @OptIn(KaExperimentalApi::class, KaIdeApi::class) -private fun KaSession.classifierSymbolToCompletionItem( - symbol: KaClassifierSymbol, -): CompletionItem? { +context(ctx: AnalysisContext) +private fun KaSession.classifierSymbolToCompletionItem(symbol: KaClassifierSymbol): CompletionItem? { val item = createSymbolCompletionItem(symbol) ?: return null - item.detail = when (symbol) { - is KaClassSymbol -> symbol.classId?.asFqNameString() ?: "" - is KaTypeAliasSymbol -> renderName( - symbol.expandedType, - KaTypeRendererForSource.WITH_QUALIFIED_NAMES - ) + item.detail = + when (symbol) { + is KaClassSymbol -> { + symbol.classId?.asFqNameString() ?: "" + } - is KaTypeParameterSymbol -> item.ideLabel - } + is KaTypeAliasSymbol -> { + renderName( + symbol.expandedType, + KaTypeRendererForSource.WITH_QUALIFIED_NAMES, + ) + } + + is KaTypeParameterSymbol -> { + item.ideLabel + } + } if (symbol is KaClassLikeSymbol) { val classFqn = symbol.classId?.asFqNameString() @@ -682,8 +732,9 @@ private fun KaSession.classifierSymbolToCompletionItem( item.setClassCompletionData( className = classFqn, isNested = symbol.classId?.isNestedClass ?: false, - topLevelClass = symbol.containingTopLevelClassDeclaration?.classId?.asFqNameString() - ?: "" + topLevelClass = + symbol.containingTopLevelClassDeclaration?.classId?.asFqNameString() + ?: "", ) } } @@ -699,19 +750,18 @@ private fun CompletionItem.setClassCompletionData( ) { abortIfCancelled() - data = ClassCompletionData( - className, - isNested, - topLevelClass - ) + data = + ClassCompletionData( + className, + isNested, + topLevelClass, + ) additionalEditHandler = KotlinAutoImportEditHandler(analysisContext = ctx) } context(ctx: AnalysisContext) -private fun KaSession.createSymbolCompletionItem( - symbol: KaSymbol, -): CompletionItem? { +private fun KaSession.createSymbolCompletionItem(symbol: KaSymbol): CompletionItem? { abortIfCancelled() return ktCompletionItem( @@ -733,32 +783,55 @@ private fun KaSession.ktCompletionItem( return item } -private fun KaSession.kindOf(symbol: KaSymbol): CompletionItemKind { - return when (symbol) { - is KaClassSymbol -> when (symbol.classKind) { - KaClassKind.CLASS -> CompletionItemKind.CLASS - KaClassKind.ENUM_CLASS -> CompletionItemKind.ENUM - KaClassKind.ANNOTATION_CLASS -> CompletionItemKind.ANNOTATION_TYPE - KaClassKind.OBJECT -> CompletionItemKind.CLASS - KaClassKind.COMPANION_OBJECT -> CompletionItemKind.CLASS - KaClassKind.INTERFACE -> CompletionItemKind.INTERFACE - KaClassKind.ANONYMOUS_OBJECT -> CompletionItemKind.CLASS +private fun KaSession.kindOf(symbol: KaSymbol): CompletionItemKind = + when (symbol) { + is KaClassSymbol -> { + when (symbol.classKind) { + KaClassKind.CLASS -> CompletionItemKind.CLASS + KaClassKind.ENUM_CLASS -> CompletionItemKind.ENUM + KaClassKind.ANNOTATION_CLASS -> CompletionItemKind.ANNOTATION_TYPE + KaClassKind.OBJECT -> CompletionItemKind.CLASS + KaClassKind.COMPANION_OBJECT -> CompletionItemKind.CLASS + KaClassKind.INTERFACE -> CompletionItemKind.INTERFACE + KaClassKind.ANONYMOUS_OBJECT -> CompletionItemKind.CLASS + } } - is KaTypeParameterSymbol -> CompletionItemKind.TYPE_PARAMETER - is KaTypeAliasSymbol -> CompletionItemKind.CLASS - is KaFunctionSymbol -> when (symbol) { - is KaConstructorSymbol -> CompletionItemKind.CONSTRUCTOR - else -> CompletionItemKind.METHOD + is KaTypeParameterSymbol -> { + CompletionItemKind.TYPE_PARAMETER } - is KaPropertySymbol -> CompletionItemKind.PROPERTY - is KaLocalVariableSymbol -> CompletionItemKind.VARIABLE - is KaValueParameterSymbol -> CompletionItemKind.VARIABLE - is KaEnumEntrySymbol -> CompletionItemKind.ENUM_MEMBER - else -> CompletionItemKind.NONE + is KaTypeAliasSymbol -> { + CompletionItemKind.CLASS + } + + is KaFunctionSymbol -> { + when (symbol) { + is KaConstructorSymbol -> CompletionItemKind.CONSTRUCTOR + else -> CompletionItemKind.METHOD + } + } + + is KaPropertySymbol -> { + CompletionItemKind.PROPERTY + } + + is KaLocalVariableSymbol -> { + CompletionItemKind.VARIABLE + } + + is KaValueParameterSymbol -> { + CompletionItemKind.VARIABLE + } + + is KaEnumEntrySymbol -> { + CompletionItemKind.ENUM_MEMBER + } + + else -> { + CompletionItemKind.NONE + } } -} private fun KaSession.kindOf(symbol: JvmSymbol): CompletionItemKind = when (symbol.kind) { @@ -782,9 +855,7 @@ private fun KaSession.kindOf(symbol: JvmSymbol): CompletionItemKind = JvmSymbolKind.TYPE_ALIAS -> CompletionItemKind.CLASS } -private fun partialIdentifier(prefix: String): String { - return prefix.takeLastWhile { char -> Character.isJavaIdentifierPart(char) } -} +private fun partialIdentifier(prefix: String): String = prefix.takeLastWhile { char -> Character.isJavaIdentifierPart(char) } /** * Returns the [MatchLevel] of [name] against [partial], memoized in [cache]. @@ -802,12 +873,10 @@ internal fun memoizedMatchLevel( ): MatchLevel = cache.getOrPut(name) { CompletionItem.matchLevel(name, partial) } context(ctx: AnalysisContext) -private fun matchLevelFor(name: String): MatchLevel = - memoizedMatchLevel(ctx.matchLevelCache, name, ctx.partial) +private fun matchLevelFor(name: String): MatchLevel = memoizedMatchLevel(ctx.matchLevelCache, name, ctx.partial) context(ctx: AnalysisContext) -private fun matchesFilter(name: Name): Boolean = - matchLevelFor(name.asString()) != MatchLevel.NO_MATCH +private fun matchesFilter(name: Name): Boolean = matchLevelFor(name.asString()) != MatchLevel.NO_MATCH private fun determineCompletionContext(element: PsiElement): CompletionContext { // Walk up to find a qualified expression where we're the selector 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 68d76e0376..901b5c0c04 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 @@ -79,11 +79,13 @@ private fun doAnalyze( // to re-schedule this run once the higher-priority work finishes. val checker = ScheduledCancelChecker(cancelChecker) - val diagnostics = env.project.read { - buildList { - PsiTreeUtil.collectElementsOfType(ktFile, PsiErrorElement::class.java) - .forEach { errorElement -> - checker.abortIfCancelled() + val diagnostics = + env.project.read { + buildList { + PsiTreeUtil + .collectElementsOfType(ktFile, PsiErrorElement::class.java) + .forEach { errorElement -> + checker.abortIfCancelled() add( diagnosticItem( file = ktFile, diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt index 650e033bda..e2d151cdeb 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt @@ -25,54 +25,60 @@ import org.slf4j.LoggerFactory * the compiler marks as best (falling back to the resolved call, then to the first candidate). The * active parameter is computed against that active overload. */ -internal fun KaSession.buildSignatureHelp(call: KtCallElement, offset: Int): SignatureHelp { - val calleeText = call.calleeExpression?.text +internal fun KaSession.buildSignatureHelp( + call: KtCallElement, + offset: Int, +): SignatureHelp { + val calleeText = call.calleeExpression?.text // (resolved function call, isBest) pairs, in candidate order. - val resolvedCandidates = call.resolveToCallCandidates() - .mapNotNull { info -> - (info.candidate as? KaFunctionCall<*>)?.let { it to info.isInBestCandidates } - } - logger.debug( - "resolveToCallCandidates() found {} candidate(s) for call '{}'", - resolvedCandidates.size, - calleeText - ) + val resolvedCandidates = + call + .resolveToCallCandidates() + .mapNotNull { info -> + (info.candidate as? KaFunctionCall<*>)?.let { it to info.isInBestCandidates } + } + logger.debug( + "resolveToCallCandidates() found {} candidate(s) for call '{}'", + resolvedCandidates.size, + calleeText, + ) - val candidates = resolvedCandidates.ifEmpty { - // Fallback: a single successfully-resolved function call. - logger.debug( - "No candidates from resolveToCallCandidates(); falling back to resolveToCall() for '{}'", - calleeText - ) - call.resolveToCall()?.successfulFunctionCallOrNull()?.let { listOf(it to true) } - ?: emptyList() - } + val candidates = + resolvedCandidates.ifEmpty { + // Fallback: a single successfully-resolved function call. + logger.debug( + "No candidates from resolveToCallCandidates(); falling back to resolveToCall() for '{}'", + calleeText, + ) + call.resolveToCall()?.successfulFunctionCallOrNull()?.let { listOf(it to true) } + ?: emptyList() + } - if (candidates.isEmpty()) { - logger.debug( - "No resolvable candidates for call '{}'; returning empty signature help", - calleeText - ) - return SignatureHelp.empty() - } + if (candidates.isEmpty()) { + logger.debug( + "No resolvable candidates for call '{}'; returning empty signature help", + calleeText, + ) + return SignatureHelp.empty() + } - val signatures: List = - candidates.map { (fnCall, _) -> buildSignatureInformation(fnCall.symbol) } + val signatures: List = + candidates.map { (fnCall, _) -> buildSignatureInformation(fnCall.symbol) } - val activeSignature = candidates.indexOfFirst { it.second }.let { if (it < 0) 0 else it } - val activeCall = candidates[activeSignature].first - val activeParameter = computeActiveParameter(call, activeCall, offset) + val activeSignature = candidates.indexOfFirst { it.second }.let { if (it < 0) 0 else it } + val activeCall = candidates[activeSignature].first + val activeParameter = computeActiveParameter(call, activeCall, offset) - logger.debug( - "buildSignatureHelp for '{}': {} signature(s), activeSignature={}, activeParameter={}", - calleeText, - signatures.size, - activeSignature, - activeParameter - ) + logger.debug( + "buildSignatureHelp for '{}': {} signature(s), activeSignature={}, activeParameter={}", + calleeText, + signatures.size, + activeSignature, + activeParameter, + ) - return SignatureHelp(signatures, activeSignature, activeParameter) + return SignatureHelp(signatures, activeSignature, activeParameter) } private val logger = LoggerFactory.getLogger("KotlinSignatureHelp") @@ -83,54 +89,55 @@ private val logger = LoggerFactory.getLogger("KotlinSignatureHelp") */ context(env: CompilationEnvironment) internal suspend fun doSignatureHelp(params: SignatureHelpParams): SignatureHelp { - logger.debug("doSignatureHelp requested for file={} position={}", params.file, params.position) + logger.debug("doSignatureHelp requested for file={} position={}", params.file, params.position) - if (params.cancelChecker.isCancelled()) { - logger.debug("Signature help request for {} was cancelled before processing", params.file) - return SignatureHelp.empty() - } + if (params.cancelChecker.isCancelled()) { + logger.debug("Signature help request for {} was cancelled before processing", params.file) + return SignatureHelp.empty() + } - // Safe to await a (possibly blocking) refresh here: this runs outside any project.read/write - // block, so it can't deadlock against the refresh's project.write (unlike KtSymbolIndex.getKtFile). - val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() - if (ktFile == null) { - logger.warn("File {} is not open", params.file) - return SignatureHelp.empty() - } + // Safe to await a (possibly blocking) refresh here: this runs outside any project.read/write + // block, so it can't deadlock against the refresh's project.write (unlike KtSymbolIndex.getKtFile). + val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() + if (ktFile == null) { + logger.warn("File {} is not open", params.file) + return SignatureHelp.empty() + } - // Signature help is interactive (the user is typing arguments): run at INTERACTIVE priority so it - // preempts background diagnostics/indexing and is discarded when a newer interactive request wins. - // params.cancelChecker is request-scoped (CancellableRequestParams), so wrap it directly — no - // global Lookup fallback needed. - val cancelChecker = ScheduledCancelChecker(params.cancelChecker) + // Signature help is interactive (the user is typing arguments): run at INTERACTIVE priority so it + // preempts background diagnostics/indexing and is discarded when a newer interactive request wins. + // params.cancelChecker is request-scoped (CancellableRequestParams), so wrap it directly — no + // global Lookup fallback needed. + val cancelChecker = ScheduledCancelChecker(params.cancelChecker) - return try { - val offset = params.position.requireIndex() - cancelChecker.abortIfCancelled() - val result = env.project.read { - val call = findEnclosingCall(ktFile, offset) ?: return@read SignatureHelp.empty() - analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { - buildSignatureHelp(call, offset) - } - } - logger.debug( - "Signature help result for {}: {} signature(s), activeSignature={}, activeParameter={}", - params.file, - result.signatures.size, - result.activeSignature, - result.activeParameter - ) - result - } catch (e: Throwable) { - if (e.isAnalysisCancellation()) { - logger.debug( - "Signature help for {} cancelled (preempted={})", - params.file, - e is AnalysisPreemptedException - ) - return SignatureHelp.empty() - } - logger.warn("Signature help computation failed for {}", params.file, e) - SignatureHelp.empty() - } + return try { + val offset = params.position.requireIndex() + cancelChecker.abortIfCancelled() + val result = + env.project.read { + val call = findEnclosingCall(ktFile, offset) ?: return@read SignatureHelp.empty() + analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { + buildSignatureHelp(call, offset) + } + } + logger.debug( + "Signature help result for {}: {} signature(s), activeSignature={}, activeParameter={}", + params.file, + result.signatures.size, + result.activeSignature, + result.activeParameter, + ) + result + } catch (e: Throwable) { + if (e.isAnalysisCancellation()) { + logger.debug( + "Signature help for {} cancelled (preempted={})", + params.file, + e is AnalysisPreemptedException, + ) + return SignatureHelp.empty() + } + logger.warn("Signature help computation failed for {}", params.file, e) + SignatureHelp.empty() + } } 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 index f0a3081ad0..fc96fc014b 100644 --- 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 @@ -4,11 +4,11 @@ import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import com.itsaky.androidide.progress.ICancelChecker -import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import org.jetbrains.kotlin.com.intellij.openapi.progress.ProgressManager import org.junit.Test import java.util.Collections import java.util.concurrent.CancellationException @@ -38,96 +38,99 @@ import kotlin.math.max * preemption, and reentrancy behaviour. */ 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 `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() - ) - } + 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, - AnalysisPriority.DIAGNOSTICS, - ScheduledCancelChecker(ICancelChecker.NOOP), - ) { - // Touching declaration symbols is what triggered the lifetime check. - file.declarations.forEach { dcl -> - dcl.symbol + 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, + AnalysisPriority.DIAGNOSTICS, + ScheduledCancelChecker(ICancelChecker.NOOP), + ) { + // Touching declaration symbols is what triggered the lifetime check. + file.declarations.forEach { dcl -> + dcl.symbol + } } } + } catch (t: Throwable) { + errors.add(t) } - } catch (t: Throwable) { - errors.add(t) } } } - } - assertThat(errors).isEmpty() - } + 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 }") - } + 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, - AnalysisPriority.DIAGNOSTICS, - ScheduledCancelChecker(ICancelChecker.NOOP), - ) { - 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() + 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, + AnalysisPriority.DIAGNOSTICS, + ScheduledCancelChecker(ICancelChecker.NOOP), + ) { + 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) } - } 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) - } + assertThat(errors).isEmpty() + // The shared analysis lock must prevent two analyses from running at once. + assertThat(maxObserved.get()).isEqualTo(1) + } @Test(timeout = 10_000) fun `reentrant withAnalysisLock on the same thread does not deadlock`() { @@ -148,28 +151,30 @@ class AnalysisSerializationTest : KtLspTest() { val higherRan = AtomicBoolean(false) // Low-priority (indexing) holder runs a long, cooperatively-cancellable analysis. - val lower = Thread { - try { - withAnalysisLock(AnalysisPriority.INDEXING, holderChecker) { - holding.countDown() - repeat(2_000) { - holderChecker.abortIfCancelled() - Thread.sleep(5) + val lower = + Thread { + try { + withAnalysisLock(AnalysisPriority.INDEXING, holderChecker) { + holding.countDown() + repeat(2_000) { + holderChecker.abortIfCancelled() + Thread.sleep(5) + } } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) } - } catch (e: AnalysisPreemptedException) { - preempted.set(true) } - } lower.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() // A completion request must preempt the in-progress indexing. - val higher = Thread { - withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { - higherRan.set(true) + val higher = + Thread { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { + higherRan.set(true) + } } - } higher.start() higher.join(5_000) lower.join(5_000) @@ -191,28 +196,30 @@ class AnalysisSerializationTest : KtLspTest() { val preempted = AtomicBoolean(false) val ranToCompletion = AtomicBoolean(false) - val lower = Thread { - try { - withAnalysisLock(AnalysisPriority.INDEXING, holderChecker) { - holding.countDown() - repeat(2_000) { - // Compiler-level checkpoint only — no abortIfCancelled() here. - ProgressManager.checkCanceled() - Thread.sleep(5) + val lower = + Thread { + try { + withAnalysisLock(AnalysisPriority.INDEXING, holderChecker) { + holding.countDown() + repeat(2_000) { + // Compiler-level checkpoint only — no abortIfCancelled() here. + ProgressManager.checkCanceled() + Thread.sleep(5) + } + ranToCompletion.set(true) } - ranToCompletion.set(true) + } catch (e: AnalysisPreemptedException) { + preempted.set(true) } - } catch (e: AnalysisPreemptedException) { - preempted.set(true) } - } lower.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() // A completion request preempts the in-progress (indexing) analysis. - val higher = Thread { - withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) {} - } + val higher = + Thread { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) {} + } higher.start() higher.join(5_000) lower.join(5_000) @@ -237,28 +244,29 @@ class AnalysisSerializationTest : KtLspTest() { // Run inside a real analyze under the read lock: this also guards against a read/write-lock // upgrade deadlock regression (withAnalysisLock runs under the shared read lock). - val worker = Thread { - try { - env.project.read { - analyzeMaybeDangling( - file, - AnalysisPriority.INTERACTIVE, - ScheduledCancelChecker(delegate), - ) { - holding.countDown() - repeat(2_000) { - // Compiler-level checkpoint only — mirrors FIR resolution, which never calls - // the LSP-level abortIfCancelled(). - ProgressManager.checkCanceled() - Thread.sleep(5) + val worker = + Thread { + try { + env.project.read { + analyzeMaybeDangling( + file, + AnalysisPriority.INTERACTIVE, + ScheduledCancelChecker(delegate), + ) { + holding.countDown() + repeat(2_000) { + // Compiler-level checkpoint only — mirrors FIR resolution, which never calls + // the LSP-level abortIfCancelled(). + ProgressManager.checkCanceled() + Thread.sleep(5) + } + ranToCompletion.set(true) } - ranToCompletion.set(true) } + } catch (t: Throwable) { + caught.set(t) } - } catch (t: Throwable) { - caught.set(t) } - } worker.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() @@ -292,26 +300,28 @@ class AnalysisSerializationTest : KtLspTest() { val waiterEntered = AtomicBoolean(false) // A completion holder keeps the lock until released. - val holder = Thread { - withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { - holding.countDown() - release.await() + val holder = + Thread { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { + holding.countDown() + release.await() + } } - } holder.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() // A lower-priority (diagnostics) requester must wait behind the completion holder (it does not // preempt). It is cancelled while waiting and must bail from acquire() promptly. - val waiter = Thread { - try { - withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(waiterDelegate)) { - waiterEntered.set(true) + val waiter = + Thread { + try { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(waiterDelegate)) { + waiterEntered.set(true) + } + } catch (t: Throwable) { + waiterThrew.set(t) } - } catch (t: Throwable) { - waiterThrew.set(t) } - } waiter.start() // Let it enter the wait loop, then cancel it. @@ -337,21 +347,23 @@ class AnalysisSerializationTest : KtLspTest() { val lowerEntered = AtomicBoolean(false) // High-priority (completion) holder holds the lock until released. - val higher = Thread { - withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { - holding.countDown() - release.await() + val higher = + Thread { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { + holding.countDown() + release.await() + } } - } higher.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() // A diagnostics request is strictly lower priority: it must not preempt completion. - val lower = Thread { - withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { - lowerEntered.set(true) + val lower = + Thread { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + lowerEntered.set(true) + } } - } lower.start() // Give the lower-priority request time to (incorrectly) barge in. @@ -374,28 +386,30 @@ class AnalysisSerializationTest : KtLspTest() { val newerRan = AtomicBoolean(false) // An in-flight completion runs a long, cooperatively-cancellable analysis. - val older = Thread { - try { - withAnalysisLock(AnalysisPriority.INTERACTIVE, holderChecker) { - holding.countDown() - repeat(2_000) { - holderChecker.abortIfCancelled() - Thread.sleep(5) + val older = + Thread { + try { + withAnalysisLock(AnalysisPriority.INTERACTIVE, holderChecker) { + holding.countDown() + repeat(2_000) { + holderChecker.abortIfCancelled() + Thread.sleep(5) + } } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) } - } catch (e: AnalysisPreemptedException) { - preempted.set(true) } - } older.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() // A newer completion request (user typed on) must supersede the in-flight one. - val newer = Thread { - withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { - newerRan.set(true) + val newer = + Thread { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { + newerRan.set(true) + } } - } newer.start() newer.join(5_000) older.join(5_000) @@ -411,22 +425,24 @@ class AnalysisSerializationTest : KtLspTest() { val secondEntered = AtomicBoolean(false) // A diagnostics holder holds the lock until released. - val first = Thread { - withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { - holding.countDown() - release.await() + val first = + Thread { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + holding.countDown() + release.await() + } } - } first.start() assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() // A second diagnostics request is the same priority but must NOT supersede the holder // (only completion supersedes same-priority work); it waits until the holder releases. - val second = Thread { - withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { - secondEntered.set(true) + val second = + Thread { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, ScheduledCancelChecker(ICancelChecker.NOOP)) { + secondEntered.set(true) + } } - } second.start() // Give the second request time to (incorrectly) barge in. diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt index 5b80266169..80b2a78cfd 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ICancelChecker.kt @@ -27,103 +27,101 @@ import java.util.concurrent.atomic.AtomicBoolean * @author Akash Yadav */ interface ICancelChecker { - - /** - * Cancel this process. - */ - fun cancel() - - /** - * Check whether this process has been cancelled or not. - * - * @return Whether the process has been cancelled. - */ - fun isCancelled(): Boolean - - /** - * Throw [CancellationException] if this process has been cancelled. - */ - @Throws(CancellationException::class) - fun abortIfCancelled() - - /** - * Register [listener] to fire when this process is cancelled, so a consumer can react immediately - * instead of polling [isCancelled]. Fires synchronously now if already cancelled, and at most once. - * - * This default only fires when already cancelled; an implementation that can transition to cancelled - * after registration (e.g. [Default]) must override to fire on the transition. - */ - fun invokeOnCancel(listener: () -> Unit) { - if (isCancelled()) { - listener() - } - } - - /** - * Unregister a [listener] previously passed to [invokeOnCancel]. Removal is by reference identity, - * so callers must pass the *same* lambda instance. No-op if the listener was never registered or - * has already fired (listeners fire at most once and are dropped on firing). - * - * The default retains no listeners, so this does nothing; an implementation that stores listeners - * (e.g. [Default]) must override to drop [listener]. - */ - fun removeOnCancel(listener: () -> Unit) {} - - open class Default(cancelled: Boolean = false) : ICancelChecker { - - private val cancelled = AtomicBoolean(cancelled) - private val onCancelListeners = CopyOnWriteArrayList<() -> Unit>() - - override fun cancel() { - if (cancelled.compareAndSet(false, true)) { - onCancelListeners.forEach { it() } - onCancelListeners.clear() - } - } - - override fun isCancelled(): Boolean { - return cancelled.get() - } - - override fun abortIfCancelled() { - if (isCancelled()) { - throw CancellationException() - } - } - - override fun invokeOnCancel(listener: () -> Unit) { - if (isCancelled()) { - listener() - return - } - onCancelListeners.add(listener) - // Guard the race where cancel() ran between the check above and the add: if we now observe - // cancellation, run the listener ourselves (removing it so cancel() can't also run it). - if (isCancelled() && onCancelListeners.remove(listener)) { - listener() - } - } - - override fun removeOnCancel(listener: () -> Unit) { - onCancelListeners.remove(listener) - } - } - - companion object { - - /** - * A no-op cancel checker. The task is never cancelled. - */ - @JvmField - val NOOP = object : Default(false) { - // Never transitions to cancelled, so retaining listeners would only leak them. - override fun invokeOnCancel(listener: () -> Unit) = Unit - } - - /** - * An already cancelled cancel checker. - */ - @JvmField - val CANCELLED = Default(true) - } + /** + * Cancel this process. + */ + fun cancel() + + /** + * Check whether this process has been cancelled or not. + * + * @return Whether the process has been cancelled. + */ + fun isCancelled(): Boolean + + /** + * Throw [CancellationException] if this process has been cancelled. + */ + @Throws(CancellationException::class) + fun abortIfCancelled() + + /** + * Register [listener] to fire when this process is cancelled, so a consumer can react immediately + * instead of polling [isCancelled]. Fires synchronously now if already cancelled, and at most once. + * + * This default only fires when already cancelled; an implementation that can transition to cancelled + * after registration (e.g. [Default]) must override to fire on the transition. + */ + fun invokeOnCancel(listener: () -> Unit) { + if (isCancelled()) { + listener() + } + } + + /** + * Unregister a [listener] previously passed to [invokeOnCancel]. Removal is by reference identity, + * so callers must pass the *same* lambda instance. No-op if the listener was never registered or + * has already fired (listeners fire at most once and are dropped on firing). + * + * The default retains no listeners, so this does nothing; an implementation that stores listeners + * (e.g. [Default]) must override to drop [listener]. + */ + fun removeOnCancel(listener: () -> Unit) {} + + open class Default( + cancelled: Boolean = false, + ) : ICancelChecker { + private val cancelled = AtomicBoolean(cancelled) + private val onCancelListeners = CopyOnWriteArrayList<() -> Unit>() + + override fun cancel() { + if (cancelled.compareAndSet(false, true)) { + onCancelListeners.forEach { it() } + onCancelListeners.clear() + } + } + + override fun isCancelled(): Boolean = cancelled.get() + + override fun abortIfCancelled() { + if (isCancelled()) { + throw CancellationException() + } + } + + override fun invokeOnCancel(listener: () -> Unit) { + if (isCancelled()) { + listener() + return + } + onCancelListeners.add(listener) + // Guard the race where cancel() ran between the check above and the add: if we now observe + // cancellation, run the listener ourselves (removing it so cancel() can't also run it). + if (isCancelled() && onCancelListeners.remove(listener)) { + listener() + } + } + + override fun removeOnCancel(listener: () -> Unit) { + onCancelListeners.remove(listener) + } + } + + companion object { + /** + * A no-op cancel checker. The task is never cancelled. + */ + @JvmField + val NOOP = + object : Default(false) { + // Never transitions to cancelled, so retaining listeners would only leak them. + override fun invokeOnCancel(listener: () -> Unit) = Unit + } + + /** + * An already cancelled cancel checker. + */ + @JvmField + val CANCELLED = Default(true) + } } diff --git a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt index a0032fa631..3ed8e58068 100644 --- a/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt +++ b/shared/src/main/java/com/itsaky/androidide/progress/ProgressManager.kt @@ -25,11 +25,9 @@ import java.util.concurrent.CancellationException * @author Akash Yadav */ class ProgressManager private constructor() { - private val threads = WeakHashMap() companion object { - val instance by lazy { ProgressManager() } @@ -51,7 +49,10 @@ class ProgressManager private constructor() { * targeted prior work on this thread, so it is not carried forward to the incoming [checker]. A * caller that needs a cancel-before-register signal to survive must not rely on this method. */ - fun register(thread: Thread, checker: ICancelChecker) { + fun register( + thread: Thread, + checker: ICancelChecker, + ) { synchronized(threads) { threads[thread] = checker } @@ -88,4 +89,4 @@ class ProgressManager private constructor() { } } } -} \ No newline at end of file +} diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt index 9612f41cfc..ee761e63d8 100644 --- a/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt +++ b/shared/src/test/java/com/itsaky/androidide/progress/ICancelCheckerTest.kt @@ -26,100 +26,99 @@ import java.util.concurrent.atomic.AtomicInteger * abort an in-flight `analyze` the moment cancellation happens instead of polling. */ class ICancelCheckerTest { + @Test + fun `invokeOnCancel fires when cancel is called`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) - @Test - fun `invokeOnCancel fires when cancel is called`() { - val checker = ICancelChecker.Default() - val fired = AtomicInteger(0) + checker.invokeOnCancel { fired.incrementAndGet() } + assertThat(fired.get()).isEqualTo(0) - checker.invokeOnCancel { fired.incrementAndGet() } - assertThat(fired.get()).isEqualTo(0) + checker.cancel() + assertThat(fired.get()).isEqualTo(1) + } - checker.cancel() - assertThat(fired.get()).isEqualTo(1) - } + @Test + fun `invokeOnCancel fires immediately when already cancelled`() { + val checker = ICancelChecker.Default(cancelled = true) + val fired = AtomicInteger(0) - @Test - fun `invokeOnCancel fires immediately when already cancelled`() { - val checker = ICancelChecker.Default(cancelled = true) - val fired = AtomicInteger(0) + checker.invokeOnCancel { fired.incrementAndGet() } - checker.invokeOnCancel { fired.incrementAndGet() } + assertThat(fired.get()).isEqualTo(1) + } - assertThat(fired.get()).isEqualTo(1) - } + @Test + fun `invokeOnCancel fires at most once across repeated cancel calls`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) - @Test - fun `invokeOnCancel fires at most once across repeated cancel calls`() { - val checker = ICancelChecker.Default() - val fired = AtomicInteger(0) + checker.invokeOnCancel { fired.incrementAndGet() } + checker.cancel() + checker.cancel() + checker.cancel() - checker.invokeOnCancel { fired.incrementAndGet() } - checker.cancel() - checker.cancel() - checker.cancel() + assertThat(fired.get()).isEqualTo(1) + } - assertThat(fired.get()).isEqualTo(1) - } + @Test + fun `multiple listeners all fire on cancel`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) - @Test - fun `multiple listeners all fire on cancel`() { - val checker = ICancelChecker.Default() - val fired = AtomicInteger(0) + checker.invokeOnCancel { fired.incrementAndGet() } + checker.invokeOnCancel { fired.incrementAndGet() } - checker.invokeOnCancel { fired.incrementAndGet() } - checker.invokeOnCancel { fired.incrementAndGet() } + checker.cancel() - checker.cancel() + assertThat(fired.get()).isEqualTo(2) + } - assertThat(fired.get()).isEqualTo(2) - } + @Test + fun `removeOnCancel drops the listener so it does not fire`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + val listener: () -> Unit = { fired.incrementAndGet() } - @Test - fun `removeOnCancel drops the listener so it does not fire`() { - val checker = ICancelChecker.Default() - val fired = AtomicInteger(0) - val listener: () -> Unit = { fired.incrementAndGet() } + checker.invokeOnCancel(listener) + checker.removeOnCancel(listener) + checker.cancel() - checker.invokeOnCancel(listener) - checker.removeOnCancel(listener) - checker.cancel() + assertThat(fired.get()).isEqualTo(0) + } - assertThat(fired.get()).isEqualTo(0) - } + @Test + fun `removeOnCancel only drops the given listener`() { + val checker = ICancelChecker.Default() + val fired = AtomicInteger(0) + val removed: () -> Unit = { fired.incrementAndGet() } - @Test - fun `removeOnCancel only drops the given listener`() { - val checker = ICancelChecker.Default() - val fired = AtomicInteger(0) - val removed: () -> Unit = { fired.incrementAndGet() } + checker.invokeOnCancel(removed) + checker.invokeOnCancel { fired.incrementAndGet() } + checker.removeOnCancel(removed) + checker.cancel() - checker.invokeOnCancel(removed) - checker.invokeOnCancel { fired.incrementAndGet() } - checker.removeOnCancel(removed) - checker.cancel() + assertThat(fired.get()).isEqualTo(1) + } - assertThat(fired.get()).isEqualTo(1) - } + @Test + fun `NOOP invokeOnCancel is a no-op`() { + val fired = AtomicInteger(0) - @Test - fun `NOOP invokeOnCancel is a no-op`() { - val fired = AtomicInteger(0) + // NOOP is a shared singleton that never cancels: registering must be a no-op so captured listeners + // don't accumulate forever. We deliberately don't call NOOP.cancel() — flipping the shared + // singleton would corrupt every other user of it. + ICancelChecker.NOOP.invokeOnCancel { fired.incrementAndGet() } - // NOOP is a shared singleton that never cancels: registering must be a no-op so captured listeners - // don't accumulate forever. We deliberately don't call NOOP.cancel() — flipping the shared - // singleton would corrupt every other user of it. - ICancelChecker.NOOP.invokeOnCancel { fired.incrementAndGet() } + assertThat(fired.get()).isEqualTo(0) + } - assertThat(fired.get()).isEqualTo(0) - } + @Test + fun `CANCELLED fires immediately`() { + val fired = AtomicInteger(0) - @Test - fun `CANCELLED fires immediately`() { - val fired = AtomicInteger(0) + ICancelChecker.CANCELLED.invokeOnCancel { fired.incrementAndGet() } - ICancelChecker.CANCELLED.invokeOnCancel { fired.incrementAndGet() } - - assertThat(fired.get()).isEqualTo(1) - } + assertThat(fired.get()).isEqualTo(1) + } } diff --git a/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt b/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt index c782fc6133..941ed02564 100644 --- a/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt +++ b/shared/src/test/java/com/itsaky/androidide/progress/ProgressManagerTest.kt @@ -21,37 +21,36 @@ import com.google.common.truth.Truth.assertThat import org.junit.Test class ProgressManagerTest { - - @Test - fun `cancel flips a registered checker`() { - // Regression for ADFA-4174: cancel(thread) must flip the *registered* checker so a caller polling - // it observes the cancellation. Previously cancel() stored a throwaway Default and the registered - // checker never became cancelled. - val checker = ICancelChecker.Default() - val thread = Thread.currentThread() - - ProgressManager.instance.register(thread, checker) - try { - assertThat(checker.isCancelled()).isFalse() - - ProgressManager.instance.cancel(thread) - - assertThat(checker.isCancelled()).isTrue() - } finally { - ProgressManager.instance.unregister(thread) - } - } - - @Test - fun `unregister detaches the checker so cancel no longer affects it`() { - val checker = ICancelChecker.Default() - val thread = Thread.currentThread() - - ProgressManager.instance.register(thread, checker) - ProgressManager.instance.unregister(thread) - - ProgressManager.instance.cancel(thread) - - assertThat(checker.isCancelled()).isFalse() - } + @Test + fun `cancel flips a registered checker`() { + // Regression for ADFA-4174: cancel(thread) must flip the *registered* checker so a caller polling + // it observes the cancellation. Previously cancel() stored a throwaway Default and the registered + // checker never became cancelled. + val checker = ICancelChecker.Default() + val thread = Thread.currentThread() + + ProgressManager.instance.register(thread, checker) + try { + assertThat(checker.isCancelled()).isFalse() + + ProgressManager.instance.cancel(thread) + + assertThat(checker.isCancelled()).isTrue() + } finally { + ProgressManager.instance.unregister(thread) + } + } + + @Test + fun `unregister detaches the checker so cancel no longer affects it`() { + val checker = ICancelChecker.Default() + val thread = Thread.currentThread() + + ProgressManager.instance.register(thread, checker) + ProgressManager.instance.unregister(thread) + + ProgressManager.instance.cancel(thread) + + assertThat(checker.isCancelled()).isFalse() + } } From 86f430d67d41b056f7129852a30970958b6e7724 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Tue, 28 Jul 2026 12:51:17 +0000 Subject: [PATCH 18/18] fix: use updated analyzeMaybeDangling for actions Signed-off-by: Akash Yadav --- .../kotlin/actions/ImplementMembersAction.kt | 9 +++++-- .../kotlin/actions/OrganizeImportsAction.kt | 9 +++++-- .../lsp/kotlin/fixtures/KtLspTest.kt | 23 +++++++++++++---- .../kotlin/utils/AbstractMemberStubsTest.kt | 18 ++++++------- .../utils/ImplementMembersEndToEndTest.kt | 2 +- .../kotlin/utils/ImportUsageCollectorTest.kt | 4 +-- .../lsp/kotlin/utils/NullSafetyFixTest.kt | 25 ++++++++----------- .../utils/OrganizeImportsEndToEndTest.kt | 8 +++--- 8 files changed, 55 insertions(+), 43 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt index bdc1eb2c07..69a5673035 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt @@ -7,6 +7,8 @@ import com.itsaky.androidide.actions.requireFile import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.lsp.kotlin.KotlinLanguageServer import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.membersToImplement @@ -19,6 +21,7 @@ import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.models.Range import com.itsaky.androidide.resources.R +import com.itsaky.androidide.tasks.createJobCancelChecker import org.jetbrains.kotlin.analysis.api.symbols.KaClassKind import org.jetbrains.kotlin.analysis.api.symbols.KaClassSymbol import org.jetbrains.kotlin.analysis.api.symbols.KaSymbolModality @@ -48,7 +51,8 @@ class ImplementMembersAction : BaseKotlinCodeAction() { val nioPath = data.requireFile().toPath() val offset = data.requireEditor().cursor.left val env = server.compilationEnvironmentFor(nioPath) ?: return emptyList() - return computeImplementMembersEdit(env, nioPath, offset) + // Ties the analysis to this action's coroutine: cancelling the action aborts the queued analysis. + return computeImplementMembersEdit(env, nioPath, offset, ScheduledCancelChecker(createJobCancelChecker())) } /** @@ -66,12 +70,13 @@ class ImplementMembersAction : BaseKotlinCodeAction() { env: AbstractCompilationEnvironment, nioPath: Path, offset: Int, + cancelChecker: ScheduledCancelChecker, ): List = runCatching { val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return emptyList() env.project.read { val classOrObject = findEnclosingClassOrObject(ktFile, offset) ?: return@read emptyList() - analyzeMaybeDangling(ktFile) { + analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { val classSymbol = classOrObject.symbol as? KaClassSymbol ?: return@analyzeMaybeDangling emptyList() if (!isImplementable(classSymbol)) return@analyzeMaybeDangling emptyList() diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt index ce5dfafb97..1294b3259d 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt @@ -6,6 +6,8 @@ import com.itsaky.androidide.actions.requireFile import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.lsp.kotlin.KotlinLanguageServer import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.collectImportUsage @@ -18,6 +20,7 @@ import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.models.Range import com.itsaky.androidide.resources.R +import com.itsaky.androidide.tasks.createJobCancelChecker import org.slf4j.LoggerFactory import java.nio.file.Path @@ -37,7 +40,8 @@ class OrganizeImportsAction : BaseKotlinCodeAction() { val server = data.get() ?: return emptyList() val nioPath = data.requireFile().toPath() val env = server.compilationEnvironmentFor(nioPath) ?: return emptyList() - return computeOrganizeEdit(env, nioPath) + // Ties the analysis to this action's coroutine: cancelling the action aborts the queued analysis. + return computeOrganizeEdit(env, nioPath, ScheduledCancelChecker(createJobCancelChecker())) } /** @@ -53,12 +57,13 @@ class OrganizeImportsAction : BaseKotlinCodeAction() { internal fun computeOrganizeEdit( env: AbstractCompilationEnvironment, nioPath: Path, + cancelChecker: ScheduledCancelChecker, ): List = runCatching { val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return emptyList() if (ktFile.importDirectives.isEmpty()) return emptyList() env.project.read { - val usage = analyzeMaybeDangling(ktFile) { collectImportUsage(ktFile) } + val usage = analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { collectImportUsage(ktFile) } val newText = organizedImportBlock(ktFile, usage) ?: return@read emptyList() val range = ktFile.importList?.textRange?.toRange(ktFile) ?: return@read emptyList() if (range == Range.NONE) return@read emptyList() diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt index 43a701939f..d9935d9c77 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt @@ -49,6 +49,21 @@ abstract class KtLspTest { action: KaSession.() -> R, ): R = env.analyze(file, action) + /** + * Runs [action] in a dangling-aware analysis session for [ktFile], the way an interactive request + * (completion, code action) does. Tests have no upstream cancellation source, hence [ICancelChecker.NOOP]. + */ + internal fun analyzeMaybeDanglingForTest( + ktFile: KtFile, + action: KaSession.() -> R, + ): R = + env.project.read { + analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, noopCancelChecker(), action) + } + + /** A fresh scheduler-aware checker with no upstream cancellation source, for call sites that require one. */ + internal fun noopCancelChecker(): ScheduledCancelChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + /** Resolves [call] to a function call and runs [action] inside the analyze block. */ protected fun analyzeMaybeDanglingForTest( call: KtCallElement, @@ -65,11 +80,9 @@ abstract class KtLspTest { // does in production. val ktFile = call.containingKtFile runBlocking { env.ktSymbolIndex.fileIndex.upsert(ktFile.toMetadata(env.project, isIndexed = false)) } - return env.project.read { - analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { - val resolved = call.resolveToCall()?.successfulFunctionCallOrNull() - action(resolved) - } + return analyzeMaybeDanglingForTest(ktFile) { + val resolved = call.resolveToCall()?.successfulFunctionCallOrNull() + action(resolved) } } } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/AbstractMemberStubsTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/AbstractMemberStubsTest.kt index be83489ffe..8c8e0a9211 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/AbstractMemberStubsTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/AbstractMemberStubsTest.kt @@ -1,7 +1,5 @@ package com.itsaky.androidide.lsp.kotlin.utils -import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling -import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import org.jetbrains.kotlin.analysis.api.symbols.KaClassSymbol import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil @@ -17,15 +15,13 @@ class AbstractMemberStubsTest : KtLspTest() { className: String, ): List { val ktFile = createSourceFile("Sample.kt", content) - return env.project.read { - analyzeMaybeDangling(ktFile) { - val decl = - PsiTreeUtil - .collectElementsOfType(ktFile, KtClassOrObject::class.java) - .first { it.name == className } - val symbol = decl.symbol as KaClassSymbol - membersToImplement(symbol).mapNotNull { renderOverrideStub(it, "\t", "\t") } - } + return analyzeMaybeDanglingForTest(ktFile) { + val decl = + PsiTreeUtil + .collectElementsOfType(ktFile, KtClassOrObject::class.java) + .first { it.name == className } + val symbol = decl.symbol as KaClassSymbol + membersToImplement(symbol).mapNotNull { renderOverrideStub(it, "\t", "\t") } } } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt index a067ab4236..f3d017163f 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt @@ -15,7 +15,7 @@ class ImplementMembersEndToEndTest : KtLspTest() { ): List { createSourceFile("Main.kt", content) val mainPath = env.sourceRoots.first().resolve("Main.kt") - return ImplementMembersAction().computeImplementMembersEdit(env, mainPath, caret) + return ImplementMembersAction().computeImplementMembersEdit(env, mainPath, caret, noopCancelChecker()) } /** Applies a single edit's newText over its [TextEdit.range] index span, returning the resulting text. */ diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.kt index c63f8ba098..0461dabf36 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.kt @@ -1,7 +1,5 @@ package com.itsaky.androidide.lsp.kotlin.utils -import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling -import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import org.jetbrains.kotlin.psi.KtFile import org.junit.Assert.assertFalse @@ -9,7 +7,7 @@ import org.junit.Assert.assertTrue import org.junit.Test class ImportUsageCollectorTest : KtLspTest() { - private fun usageOf(ktFile: KtFile): ImportUsage = env.project.read { analyzeMaybeDangling(ktFile) { collectImportUsage(ktFile) } } + private fun usageOf(ktFile: KtFile): ImportUsage = analyzeMaybeDanglingForTest(ktFile) { collectImportUsage(ktFile) } @Test fun `type reference is recorded as used`() { diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/NullSafetyFixTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/NullSafetyFixTest.kt index 012a0d101e..3845531410 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/NullSafetyFixTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/NullSafetyFixTest.kt @@ -1,6 +1,5 @@ package com.itsaky.androidide.lsp.kotlin.utils -import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import com.itsaky.androidide.lsp.models.TextEdit @@ -14,24 +13,20 @@ import org.junit.Test class NullSafetyFixTest : KtLspTest() { /** The [start, end) source offsets of the sole UNSAFE_CALL diagnostic in [ktFile]. */ private fun unsafeCallRange(ktFile: KtFile): Pair = - env.project.read { - analyzeMaybeDangling(ktFile) { - ktFile - .collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) - .filter { it.factoryName == UNSAFE_CALL_FACTORY } - .map { it.psi.textRange.startOffset to it.psi.textRange.endOffset } - .single() - } + analyzeMaybeDanglingForTest(ktFile) { + ktFile + .collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) + .filter { it.factoryName == UNSAFE_CALL_FACTORY } + .map { it.psi.textRange.startOffset to it.psi.textRange.endOffset } + .single() } /** The null-safety marker the diagnostic provider would store for each diagnostic in [ktFile]. */ private fun nullSafetyMarkers(ktFile: KtFile): List = - env.project.read { - analyzeMaybeDangling(ktFile) { - ktFile - .collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) - .map { nullSafetyFactoryFor(it.factoryName) } - } + analyzeMaybeDanglingForTest(ktFile) { + ktFile + .collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS) + .map { nullSafetyFactoryFor(it.factoryName) } } /** Applies a single-edit variant to [source] and returns the rewritten text. */ diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt index b695c58dc9..535a4ae7b9 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt @@ -31,7 +31,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { val mainPath = env.sourceRoots.first().resolve("Main.kt") // Drive the action's real plumbing: fetch-before-read ordering + full guard chain. - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) assertEquals(1, edits.size) assertEquals("import lib.Used", edits.single().newText) @@ -63,7 +63,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { """.trimIndent(), ) val mainPath = env.sourceRoots.first().resolve("Main.kt") - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) // Already organized -> no edit. A dropped import would produce a rewrite that removes it. assertTrue("constructor-only import must survive", edits.isEmpty()) } @@ -86,7 +86,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { """.trimIndent(), ) val mainPath = env.sourceRoots.first().resolve("Main.kt") - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) assertTrue("annotation-only import must survive", edits.isEmpty()) } @@ -109,7 +109,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { """.trimIndent(), ) val mainPath = env.sourceRoots.first().resolve("Main.kt") - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) assertTrue("typealias-only import used as constructor must survive", edits.isEmpty()) } }