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).

Author: itsaky-adfa

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<JvmSymbol, List<TextEdit>> {
-		val (diagnostic, env) = data.require<DiagnosticItem>().extra as? KotlinDiagnosticExtra
+		val (reference, env) = data.require<DiagnosticItem>().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) }
 	}

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(

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()
 

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<CompletionItem>()
-						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<CompletionItem>()
+					val completionContext = determineCompletionContext(ctx.psiElement)
+					when (completionContext) {
+						CompletionContext.Scope ->
+							collectScopeCompletions(to = items)
+
+						CompletionContext.Member ->
+							collectMemberCompletions(to = items)
 					}
+
+					CompletionResult(items)
 				}
 			}
 		}

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)
 						})
 					}
 			}