diff --git a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt
index fc94ebbc84..d48c53b05a 100644
--- a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt
+++ b/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,10 +149,10 @@ 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()
@@ -134,10 +160,18 @@ class JavaLanguageServer : ILanguageServer {
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,6 +365,22 @@ 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 {
@@ -265,8 +388,7 @@ class JavaLanguageServer : ILanguageServer {
}
}
- 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()
}
diff --git a/lsp/java/src/test/java/com/itsaky/androidide/lsp/java/JavaLanguageServerLifecycleTest.kt b/lsp/java/src/test/java/com/itsaky/androidide/lsp/java/JavaLanguageServerLifecycleTest.kt
new file mode 100644
index 0000000000..d7addc4625
--- /dev/null
+++ b/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 .
+ */
+
+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()
+
+ @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()
+ }
+}