ADFA-5052: Defer eager JavaCompilerService construction until a real .java file is touched

lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt

@@ -17,6 +17,7 @@
 package com.itsaky.androidide.lsp.java
 
 import androidx.annotation.RestrictTo
+import androidx.annotation.VisibleForTesting
 import com.itsaky.androidide.app.BaseApplication
 import com.itsaky.androidide.eventbus.events.editor.DocumentChangeEvent
 import com.itsaky.androidide.eventbus.events.editor.DocumentCloseEvent
@@ -84,6 +85,8 @@ import org.slf4j.LoggerFactory
 import java.nio.file.Files
 import java.nio.file.Path
 import java.util.Objects
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
 
 class JavaLanguageServer : ILanguageServer {
 	private val completionProvider: CompletionProvider = CompletionProvider()
@@ -96,6 +99,29 @@ class JavaLanguageServer : ILanguageServer {
 	private val timer = AnalyzeTimer { analyzeSelected() }
 	private var cachedCompletion: CachedCompletion
 
+	// Lifecycle of the javac-backed compiler state (NO_MODULE_COMPILER, SourceFileManager,
+	// JavaCompilerProvider), which setupWithProject() defers instead of building eagerly
+	// (ADFA-5052). All reads/writes of pendingWorkspace and compilerLifecycle go through
+	// compilerLifecycleLock, held for the *entire* reset/shutdown, not just the decision to
+	// run one -- otherwise a concurrent getCompiler()/onContentChange() could use a compiler
+	// mid-teardown, or shutdown() could destroy state a reset is still rebuilding.
+	private enum class CompilerLifecycle { PENDING, RESETTING, INITIALIZED, SHUTDOWN }
+
+	private val compilerLifecycleLock = ReentrantLock()
+
+	// Guarded by compilerLifecycleLock.
+	private var pendingWorkspace: Workspace? = null
+	private var compilerLifecycle = CompilerLifecycle.PENDING
+
+	/**
+	 * Whether [shutdown] has run. Exposed because the lifecycle is otherwise unobservable from
+	 * outside -- every path returns `NO_MODULE_COMPILER` for its own reasons, so a test cannot tell
+	 * "refused because shut down" from "no module for this file" without it.
+	 */
+	@VisibleForTesting
+	internal val isShutDown: Boolean
+		get() = compilerLifecycleLock.withLock { compilerLifecycle == CompilerLifecycle.SHUTDOWN }
+
 	val settings: IServerSettings
 		get() {
 			return _settings ?: JavaServerSettings
@@ -123,21 +149,29 @@ class JavaLanguageServer : ILanguageServer {
 
 		val projectManager = ProjectManagerImpl.getInstance()
 		projectManager.indexingServiceManager.register(
-			service = JvmLibraryIndexingService(context = BaseApplication.baseInstance)
+			service = JvmLibraryIndexingService(context = BaseApplication.baseInstance),
 		)
 		projectManager.indexingServiceManager.register(
-			service = JvmGeneratedIndexingService(context = BaseApplication.baseInstance)
+			service = JvmGeneratedIndexingService(context = BaseApplication.baseInstance),
 		)
 
 		JavaSnippetRepository.init()
 	}
 
 	override fun shutdown() {
 		(this.debugAdapter as? AutoCloseable?)?.close()
-		JavaCompilerProvider.getInstance().destroy()
-		SourceFileManager.clearCache()
-		CacheFSInfoSingleton.clearCache()
-		clearCache()
+		compilerLifecycleLock.withLock {
+			// Blocks here if a reset is in flight (RESETTING can only be observed by another
+			// thread while the lock is held, never by us once we've acquired it), so this never
+			// races ensureProjectReset()'s own destroy/rebuild.
+			if (compilerLifecycle == CompilerLifecycle.INITIALIZED) {
+				JavaCompilerProvider.getInstance().destroy()
+				SourceFileManager.clearCache()
+				CacheFSInfoSingleton.clearCache()
+				clearCache()
+			}
+			compilerLifecycle = CompilerLifecycle.SHUTDOWN
+		}
 		EventBus.getDefault().unregister(this)
 		timer.cancel()
 	}
@@ -163,41 +197,114 @@ class JavaLanguageServer : ILanguageServer {
 	override fun setupWithProject(workspace: Workspace) {
 		LSPEditorActions.ensureActionsMenuRegistered(JavaCodeActionsMenu)
 
-		(ProjectManagerImpl.getInstance()
-			.indexingServiceManager
-			.getService(JvmLibraryIndexingService.ID) as? JvmLibraryIndexingService?)
-			?.refresh()
-
-		// Once we have project initialized
-		// Destory the NO_MODULE_COMPILER instance
-		JavaCompilerService.NO_MODULE_COMPILER.destroy()
-
-		// Clear cached file managers
-		SourceFileManager.clearCache()
+		(
+			ProjectManagerImpl
+				.getInstance()
+				.indexingServiceManager
+				.getService(JvmLibraryIndexingService.ID) as? JvmLibraryIndexingService?
+		)?.refresh()
+
+		// Deferred to ensureProjectReset(), run on the first real .java-file interaction instead
+		// of here -- this method runs for every project open regardless of language
+		// (DefaultLanguageServerRegistry dispatches to all registered servers unconditionally),
+		// and JavaCompilerService.NO_MODULE_COMPILER / SourceFileManager.NO_MODULE eagerly
+		// construct real javac machinery plus a full android.jar scan at class-init, merely by
+		// being referenced (ADFA-5052, mirrors ADFA-5010's KotlinLanguageServer fix).
+		compilerLifecycleLock.withLock {
+			// SHUTDOWN is terminal. A server whose javac state has been destroyed does not come
+			// back because a project happened to open afterwards; reviving it here would rebuild
+			// compilers nothing is going to shut down again (found in review).
+			if (compilerLifecycle == CompilerLifecycle.SHUTDOWN) {
+				log.debug("setupWithProject() ignored: this server has been shut down.")
+				return
+			}
+			pendingWorkspace = workspace
+			// Leave RESETTING alone: ensureProjectReset()'s own finally block re-checks
+			// pendingWorkspace once it re-acquires the lock, so a project switch mid-reset is
+			// picked up as another PENDING round rather than raced here.
+			if (compilerLifecycle != CompilerLifecycle.RESETTING) {
+				compilerLifecycle = CompilerLifecycle.PENDING
+			}
+		}
 
-		// Clear cached JAR file system for R.jar
-		// Using the cached instance will result in completions not being updated for updated resources
-		// TODO Clearing caches for JAR files ending with '/R.jar' is probably not a good idea
-		//    Maybe this could be improved by using data from the AndroidModule project model
-		clearCachesForPaths { path: String -> path.endsWith("/R.jar") }
+		// Re-armed here as well as in ensureProjectReset(). A .java tab restored from the tab cache
+		// opens before the Gradle sync posts this event, and AnalyzeTimer fires once: that shot lands
+		// while pendingWorkspace is still null, finds no module, and returns NO_UPDATE. Without this
+		// the restored file then shows no diagnostics until the user types. Arming the timer builds
+		// no javac -- analyze() is gated on isJavaFile and now on the lifecycle too.
+		startOrRestartAnalyzeTimer()
+	}
 
-		// Clear cached module-specific compilers
-		JavaCompilerProvider.getInstance().destroy()
+	/**
+	 * Runs the javac-specific project reset deferred by [setupWithProject], for the most
+	 * recently opened project, the first time a real Java file is actually interacted with.
+	 * No-ops if already up to date. Blocks concurrent callers (and [shutdown]) for the entire
+	 * reset, not just the decision to run one.
+	 */
+	private fun ensureProjectReset() {
+		compilerLifecycleLock.withLock {
+			// PENDING is the only state a reset starts from; SHUTDOWN in particular is terminal.
+			if (compilerLifecycle != CompilerLifecycle.PENDING) return
+			val workspace = pendingWorkspace ?: return
+			pendingWorkspace = null
+			compilerLifecycle = CompilerLifecycle.RESETTING
+
+			try {
+				// Once we have project initialized
+				// Destory the NO_MODULE_COMPILER instance
+				JavaCompilerService.NO_MODULE_COMPILER.destroy()
+
+				// Clear cached file managers
+				SourceFileManager.clearCache()
+
+				// Clear cached JAR file system for R.jar
+				// Using the cached instance will result in completions not being updated for updated resources
+				// TODO Clearing caches for JAR files ending with '/R.jar' is probably not a good idea
+				//    Maybe this could be improved by using data from the AndroidModule project model
+				clearCachesForPaths { path: String -> path.endsWith("/R.jar") }
+
+				// Clear cached module-specific compilers
+				JavaCompilerProvider.getInstance().destroy()
 
-		// Cache classpath locations
-		for (subModule in workspace.subProjects) {
-			if (subModule !is ModuleProject || subModule.path == workspace.rootProject.path) {
-				continue
+				// Cache classpath locations
+				for (subModule in workspace.subProjects) {
+					if (subModule !is ModuleProject || subModule.path == workspace.rootProject.path) {
+						continue
+					}
+					SourceFileManager.forModule(subModule)
+				}
+				startOrRestartAnalyzeTimer()
+			} catch (e: Throwable) {
+				// Throwable, not Exception: the class-init this whole change defers is exactly what
+				// fails as an Error -- OutOfMemoryError, or ExceptionInInitializerError /
+				// NoClassDefFoundError out of the android.jar top-level scan. Catching only Exception
+				// left compilerLifecycle stuck at RESETTING with the workspace already discarded, so
+				// every later reset returned early and Java support stayed dead for the session.
+				//
+				// Re-queue the workspace so the next real .java-file interaction retries the
+				// reset, instead of a half-destroyed/half-rebuilt state being silently claimed as
+				// INITIALIZED (pendingWorkspace is already null by this point).
+				log.warn("Failed to reset javac project state; will retry on next interaction", e)
+				pendingWorkspace = workspace
+				compilerLifecycle = CompilerLifecycle.PENDING
+				throw e
 			}
-			SourceFileManager.forModule(subModule)
+
+			// A newer setupWithProject() may have queued another workspace while we were
+			// resetting (see the RESETTING guard above); if so, go back to PENDING instead of
+			// claiming INITIALIZED for a project we didn't actually reset for.
+			compilerLifecycle =
+				if (pendingWorkspace != null) {
+					CompilerLifecycle.PENDING
+				} else {
+					CompilerLifecycle.INITIALIZED
+				}
 		}
-		startOrRestartAnalyzeTimer()
 	}
 
 	override fun complete(params: CompletionParams?): CompletionResult {
 		val compiler = getCompiler(params!!.file)
-		if (!settings.completionsEnabled() || !completionProvider.canComplete(params.file)
-		) {
+		if (!settings.completionsEnabled() || !completionProvider.canComplete(params.file)) {
 			return CompletionResult.EMPTY
 		}
 
@@ -258,15 +365,30 @@ class JavaLanguageServer : ILanguageServer {
 			return DiagnosticResult.NO_UPDATE
 		}
 
+		// The third javac entry point, and the one that was missing this: diagnosticProvider.analyze()
+		// constructs its own JavaCompilerService, so an analysis already in flight when shutdown()
+		// lands would rebuild everything shutdown just destroyed -- and SHUTDOWN is terminal, so
+		// nothing would ever tear it down again. Reachable: analyzeSelected() launches on
+		// Dispatchers.Default and the timer callback cannot be recalled once dispatched.
+		if (isShutDown) {
+			return DiagnosticResult.NO_UPDATE
+		}
+
+		// diagnosticProvider.analyze() builds its own JavaCompilerService directly (bypassing
+		// getCompiler()), and analysis is often the first real .java-file interaction in a
+		// session (auto-triggered on file open, ahead of any completion request) -- without this,
+		// the R.jar/file-manager caches this reset clears would never get cleared for this
+		// project, and diagnostics could resolve against a stale previous project's classpath.
+		ensureProjectReset()
+
 		return if (!settings.codeAnalysisEnabled()) {
 			DiagnosticResult.NO_UPDATE
 		} else {
 			diagnosticProvider.analyze(file)
 		}
 	}
 
-	override fun formatCode(params: FormatCodeParams?): CodeFormatResult =
-		CodeFormatProvider(settings).format(params)
+	override fun formatCode(params: FormatCodeParams?): CodeFormatResult = CodeFormatProvider(settings).format(params)
 
 	override fun handleFailure(failure: LSPFailure?): Boolean {
 		return when (failure!!.type) {
@@ -285,10 +407,23 @@ class JavaLanguageServer : ILanguageServer {
 		if (!DocumentUtils.isJavaFile(file)) {
 			return JavaCompilerService.NO_MODULE_COMPILER
 		}
-		val module =
-			ProjectManagerImpl.getInstance().findModuleForFile(file!!)
-				?: return JavaCompilerService.NO_MODULE_COMPILER
-		return JavaCompilerProvider.get(module)
+		// Held across ensureProjectReset() *and* the provider lookup (ReentrantLock is
+		// reentrant, so ensureProjectReset()'s own withLock nests fine): otherwise a concurrent
+		// reset for a newer project could destroy() the provider's compilers in the gap between
+		// this thread's reset finishing and its JavaCompilerProvider.get() call.
+		return compilerLifecycleLock.withLock {
+			// Nothing to hand out once the javac state is gone: NO_MODULE_COMPILER is the same
+			// answer this returns for a non-Java file, and it does not resurrect what shutdown()
+			// destroyed.
+			if (compilerLifecycle == CompilerLifecycle.SHUTDOWN) {
+				return@withLock JavaCompilerService.NO_MODULE_COMPILER
+			}
+			ensureProjectReset()
+			val module =
+				ProjectManagerImpl.getInstance().findModuleForFile(file!!)
+					?: return@withLock JavaCompilerService.NO_MODULE_COMPILER
+			JavaCompilerProvider.get(module)
+		}
 	}
 
 	private fun updateCachedCompletion(cachedCompletion: CachedCompletion) {
@@ -314,14 +449,24 @@ class JavaLanguageServer : ILanguageServer {
 			return
 		}
 
-		// TODO Find an alternative to efficiently update changeDelta in JavaCompilerService instance
-		JavaCompilerService.NO_MODULE_COMPILER.onDocumentChange(event)
-		val module =
-			getInstance()
-				.findModuleForFile(event.changedFile)
-		if (module != null) {
-			val compiler = JavaCompilerProvider.get(module)
-			compiler.onDocumentChange(event)
+		// See getCompiler(): held across the reset *and* the provider lookup/use so a concurrent
+		// reset can't destroy() these compilers in between.
+		compilerLifecycleLock.withLock {
+			// A document change after shutdown has no compiler to tell, and must not rebuild one.
+			if (compilerLifecycle == CompilerLifecycle.SHUTDOWN) {
+				return
+			}
+			ensureProjectReset()
+
+			// TODO Find an alternative to efficiently update changeDelta in JavaCompilerService instance
+			JavaCompilerService.NO_MODULE_COMPILER.onDocumentChange(event)
+			val module =
+				getInstance()
+					.findModuleForFile(event.changedFile)
+			if (module != null) {
+				val compiler = JavaCompilerProvider.get(module)
+				compiler.onDocumentChange(event)
+			}
 		}
 		startOrRestartAnalyzeTimer()
 	}

lsp/java/src/test/java/com/itsaky/androidide/lsp/java/JavaLanguageServerLifecycleTest.kt

@@ -0,0 +1,77 @@
+/*
+ *  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 <https://www.gnu.org/licenses/>.
+ */
+
+package com.itsaky.androidide.lsp.java
+
+import com.google.common.truth.Truth.assertThat
+import io.mockk.mockk
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+/**
+ * ADFA-5052 defers the javac reset to the first real Java interaction, which makes the server's
+ * lifecycle something with states rather than a single construction. These cover the transitions
+ * that do not need a project fixture; the concurrent ones (an operation holding a compiler while
+ * shutdown lands) are ADFA-5261.
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(manifest = Config.DEFAULT_VALUE_STRING)
+class JavaLanguageServerLifecycleTest {
+	// Every server registers itself on the global EventBus and adds indexing services to the
+	// ProjectManagerImpl singleton in its init block, and Robolectric caches a sandbox per @Config --
+	// so a server left running here would keep receiving events posted by other test classes.
+	private val servers = mutableListOf<JavaLanguageServer>()
+
+	@After
+	fun tearDown() {
+		servers.forEach { it.shutdown() }
+		servers.clear()
+	}
+
+	private fun newServer() = JavaLanguageServer().also { servers += it }
+
+	// The whole point of the deferral: nothing javac-related is built until a Java file is touched,
+	// so shutting down before that must be a no-op rather than a teardown of things never made.
+	@Test
+	fun `shutdown before the first java interaction is safe`() {
+		val server = newServer()
+
+		server.shutdown()
+
+		assertThat(server.isShutDown).isTrue()
+	}
+
+	// A server whose javac state has been destroyed does not come back because a project opened
+	// afterwards -- otherwise it rebuilds compilers that nothing will shut down again.
+	@Test
+	fun `a project opened after shutdown does not revive the server`() {
+		val server = newServer()
+		server.shutdown()
+
+		server.setupWithProject(mockk(relaxed = true))
+
+		assertThat(server.isShutDown).isTrue()
+	}
+
+	@Test
+	fun `a fresh server is not shut down`() {
+		assertThat(newServer().isShutDown).isFalse()
+	}
+}