fix(ADFA-4384): stop Kotlin LSP index workers before disposing project

docs/process/learnings.md

@@ -0,0 +1,5 @@
+# Learnings
+
+## Kotlin LSP test harness
+- Disposing the `KtLspTestEnvironment` in a unit test (`env.close()`, or `Disposer.dispose(env.project)`) throws `AssertionError: Write access is allowed inside write-action only`. IntelliJ requires model teardown to run inside a write action. This is why `KtLspTestRule`'s teardown has `env.close()` commented out as "fails in test cases". To dispose deterministically in a test, wrap it: `ApplicationManager.getApplication().runWriteAction { env.close() }`.
+- The index/compilation environment lifecycle is racy: background `IndexWorker` coroutines call `PsiManager.findFile(project)` and will crash with `Project is already disposed` if the project is disposed before the workers are stopped. Always stop & join `KtSymbolIndex.close()` (and cancel related scopes) before `Disposer.dispose(...)`.

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/AbstractCompilationEnvironment.kt

@@ -10,6 +10,8 @@ import com.itsaky.androidide.lsp.kotlin.compiler.services.JavaModuleAnnotationsP
 import com.itsaky.androidide.lsp.kotlin.compiler.services.KtLspService
 import com.itsaky.androidide.lsp.kotlin.compiler.services.WriteAccessGuard
 import com.itsaky.androidide.lsp.kotlin.compiler.services.latestLanguageVersionSettings
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeoutOrNull
 import org.jetbrains.kotlin.K1Deprecation
 import org.jetbrains.kotlin.analysis.api.platform.declarations.KotlinAnnotationsResolverFactory
 import org.jetbrains.kotlin.analysis.api.platform.declarations.KotlinDeclarationProviderFactory
@@ -96,6 +98,9 @@ internal abstract class AbstractCompilationEnvironment(
 ) : AutoCloseable {
 
 	companion object {
+		/** Max time close() will block the (main) thread draining background workers before disposal. */
+		const val CLOSE_DRAIN_TIMEOUT_MS = 2_000L
+
 		init {
 			System.setProperty("java.awt.headless", "true")
 			setupIdeaStandaloneExecution()
@@ -333,6 +338,16 @@ internal abstract class AbstractCompilationEnvironment(
 		}
 
 	override fun close() {
+		// Stop and join the background index workers *before* the project is disposed.
+		// Otherwise IndexWorker's coroutine keeps calling PsiManager.findFile(project) on a
+		// disposed project and crashes with "AssertionError: Project is already disposed"
+		// (Sentry APPDEVFORALL-17R / ADFA-4384). close() runs on the main thread during editor
+		// teardown, so the join is bounded by a timeout to avoid an ANR if a read is slow; the
+		// project.isDisposed guards cover the rare case where the timeout fires before draining.
+		if (::ktSymbolIndex.isInitialized) {
+			runBlocking { withTimeoutOrNull(CLOSE_DRAIN_TIMEOUT_MS) { ktSymbolIndex.close() } }
+		}
+
 		Disposer.dispose(disposable)
 	}
 }

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt

@@ -20,8 +20,12 @@ import io.sentry.Sentry
 import kotlinx.coroutines.CoroutineName
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withContext
+import kotlinx.coroutines.withTimeoutOrNull
 import org.appdevforall.codeonthego.indexing.jvm.JvmSymbolIndex
 import org.appdevforall.codeonthego.indexing.jvm.KtFileMetadataIndex
 import org.jetbrains.kotlin.analysis.api.KaImplementationDetail
@@ -298,6 +302,16 @@ internal class CompilationEnvironment(
 
 	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_MS) {
+				coroutineScope.coroutineContext[Job]?.cancelAndJoin()
+			}
+		}
+
 		super.close()
 	}
 

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt

@@ -59,6 +59,11 @@ internal class IndexWorker(
 		}
 
 		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 -> {
 					val filePath = cmd.path.pathString
@@ -72,6 +77,8 @@ internal class IndexWorker(
 						continue
 					}
 
+					if (project.isDisposed) break
+
 					val ktFile = project.read {
 						PsiManager.getInstance(project)
 							.findFile(cmd.vf) as? KtFile
@@ -111,6 +118,8 @@ internal class IndexWorker(
 				}
 
 				is IndexCommand.ScanSourceFile -> {
+					if (project.isDisposed) break
+
 					val ktFile = project.read {
 						PsiManager.getInstance(project).findFile(cmd.vf) as? KtFile
 					}

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt

@@ -202,6 +202,13 @@ internal class KtSymbolIndex(
 
 		scanningJob?.cancelAndJoin()
 		indexingJob?.join()
+
+		// Cancel AND JOIN the index's own scope. Beyond the main worker loop drained above, the
+		// debounced modifiedFileIndexer and queueOnFileChangedAsync coroutines also run
+		// project.read { PsiManager … }. Joining guarantees none survive into the caller's
+		// Disposer.dispose(...), which would otherwise crash with "Project is already disposed"
+		// (APPDEVFORALL-17R). This index owns `scope`.
+		scope.coroutineContext[Job]?.cancelAndJoin()
 	}
 }
 

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt

@@ -74,7 +74,17 @@ internal suspend fun indexSourceFile(
 	symbolsIndex: JvmSymbolIndex,
 	cancelChecker: ICancelChecker,
 ) {
-	val newFile = ktFile.toMetadata(project, isIndexed = true)
+	// Defensive backstop: this runs on the debounced/async index scope, so a disposal path that
+	// didn't first drain & join the workers could otherwise touch PSI on a disposed project and
+	// throw "Project is already disposed" (APPDEVFORALL-17R). Cheap fast-path before the reads below.
+	if (project.isDisposed) return
+
+	// Re-check disposal *inside* the read lock so it is atomic with toMetadata()'s PSI access:
+	// the fast-path check above can race a concurrent disposal before toMetadata enters its read.
+	val newFile = project.read {
+		if (project.isDisposed) return@read null
+		ktFile.toMetadata(project, isIndexed = true)
+	} ?: return
 	val existingFile = fileIndex.get(newFile.filePath)
 	cancelChecker.abortIfCancelled()
 
@@ -89,6 +99,9 @@ internal suspend fun indexSourceFile(
 	}
 
 	val symbols = project.read {
+		// Atomic w.r.t. the read lock: bail if the project was disposed before we acquired it.
+		if (project.isDisposed) return@read emptyList()
+
 		val list = mutableListOf<JvmSymbol>()
 		analyzeMaybeDangling(ktFile) {
 			val session = this

lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorkerDisposalTest.kt

@@ -0,0 +1,65 @@
+package com.itsaky.androidide.lsp.kotlin.compiler.index
+
+import com.google.common.truth.Truth.assertThat
+import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import org.appdevforall.codeonthego.indexing.InMemoryIndex
+import org.appdevforall.codeonthego.indexing.jvm.JvmSymbolDescriptor
+import org.appdevforall.codeonthego.indexing.jvm.JvmSymbolIndex
+import org.appdevforall.codeonthego.indexing.jvm.KtFileMetadataDescriptor
+import org.appdevforall.codeonthego.indexing.jvm.KtFileMetadataIndex
+import org.appdevforall.codeonthego.indexing.util.BackgroundIndexer
+import org.jetbrains.kotlin.com.intellij.openapi.application.ApplicationManager
+import org.junit.Test
+
+/**
+ * Regression tests for APPDEVFORALL-17R / ADFA-4384:
+ * `AssertionError: Project is already disposed` thrown by [IndexWorker] when the IntelliJ
+ * [org.jetbrains.kotlin.com.intellij.openapi.project.Project] is disposed while the background
+ * index worker is still running and about to call `PsiManager.findFile(project)`.
+ */
+class IndexWorkerDisposalTest : KtLspTest() {
+
+	private fun jvmIndex(): JvmSymbolIndex {
+		val backing = InMemoryIndex(JvmSymbolDescriptor)
+		return object : JvmSymbolIndex(backing, BackgroundIndexer(backing)) {
+			override fun isActive(sourceId: String) = true
+		}
+	}
+
+	private fun fileIndex(): KtFileMetadataIndex =
+		KtFileMetadataIndex(InMemoryIndex(KtFileMetadataDescriptor))
+
+	@Test
+	fun `worker stops without crashing when project is already disposed`(): Unit = runBlocking {
+		// Capture a real VirtualFile while the project is still alive.
+		val vf = createSourceFile("Sample.kt", "fun foo() = 1").virtualFile
+
+		val worker = IndexWorker(
+			project = env.project,
+			queue = WorkerQueue(),
+			fileIndex = fileIndex(),
+			sourceIndex = jvmIndex(),
+			scope = CoroutineScope(Dispatchers.Default + SupervisorJob()),
+		)
+
+		// Dispose the project out from under the worker, exactly as happens on LSP shutdown
+		// (env.close() disposes the project via its parent Disposable, the production path).
+		// IntelliJ requires model teardown to run inside a write action.
+		ApplicationManager.getApplication().runWriteAction { env.close() }
+		assertThat(env.project.isDisposed).isTrue()
+
+		// Pre-fix, processing either command calls PsiManager.findFile on the disposed project
+		// and throws AssertionError("Project is already disposed"). Post-fix, the disposal guard
+		// breaks the loop, so start() returns cleanly instead of throwing.
+		worker.submitCommand(IndexCommand.ScanSourceFile(vf))
+		worker.submitCommand(IndexCommand.IndexSourceFile(vf))
+		worker.submitCommand(IndexCommand.Stop)
+
+		withTimeout(5_000) { worker.start() }
+	}
+}