Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,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()
Expand All @@ -96,6 +98,20 @@ 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
Comment on lines +109 to +113

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Keep compiler use inside the lifecycle contract.

ensureProjectReset() releases compilerLifecycleLock before getCompiler() returns a JavaCompilerService. shutdown() can then acquire the lock and destroy that service while completion, navigation, or document-change work still uses it. After shutdown, getCompiler() and onContentChange() also continue to access compiler state, and setupWithProject() can change SHUTDOWN back to PENDING.

Use an operation-scoped lifecycle lease or equivalent guarded execution API. Keep shutdown terminal. Route all compiler creation, use, and destruction through that API.

  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L109-L113: represent active compiler operations in the lifecycle mechanism.
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L153-L164: wait for active compiler operations before destruction.
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L203-L210: do not transition SHUTDOWN to PENDING.
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L355-L359: acquire and use the compiler through the lifecycle mechanism.
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L385-L396: execute Java document updates through the same mechanism.

This conflicts with the PR objective that lifecycle locking prevents concurrent compiler use during teardown.

📍 Affects 1 file
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L109-L113 (this comment)
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L153-L164
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L203-L210
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L355-L359
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt#L385-L396
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt`
around lines 109 - 113, Keep compiler lifecycle operations leased and
synchronized so shutdown cannot destroy a compiler while it is in use. In
JavaLanguageServer.kt:109-113, extend the lifecycle state with active-operation
tracking; at 153-164, make shutdown wait for active operations before destroying
the compiler and remain terminal; at 203-210, prevent setupWithProject() from
changing SHUTDOWN back to PENDING; at 355-359, acquire and use the compiler
through the lifecycle lease; and at 385-396, route onContentChange() through the
same guarded execution mechanism.


val settings: IServerSettings
get() {
return _settings ?: JavaServerSettings
Expand Down Expand Up @@ -123,21 +139,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()
}
Expand All @@ -163,41 +187,93 @@ 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()

// 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") }
(
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 {
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 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 {
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: Exception) {
// 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()
}
Comment on lines +214 to 272

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate lifecycle tests and their covered transitions.
rg -n \
  -g '*Test*.kt' -g '*Test*.java' -g '*test*.kt' -g '*test*.java' \
  '\b(ensureProjectReset|setupWithProject|getCompiler|onContentChange|shutdown|CompilerLifecycle)\b' \
  lsp/java

Repository: appdevforall/CodeOnTheGo

Length of output: 1679


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the Java server lifecycle implementation and existing Java tests without running repository code.
printf '== JavaLanguageServer outline ==\n'
ast-grep outline lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt --match JavaLanguageServer --view expanded || true

printf '\n== JavaLanguageServer lifecycle section ==\n'
sed -n '150,290p' lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt | cat -n

printf '\n== Existing Java LSP tests size/content ==\n'
wc -l lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt lsp/java/src/test/java/com/itsaky/androidide/lsp/java/JavaLSPTest.kt lsp/java/src/test/java/com/itsaky/androidide/lsp/java/compiler/CompilerTest.kt lsp/java/src/test/java/com/itsaky/androidide/lsp/java/partial/PartialReparserImplTest.kt
printf '\nJavaLSPTest relevant content:\n'
sed -n '1,180p' lsp/java/src/test/java/com/itsaky/androidide/lsp/java/JavaLSPTest.kt | cat -n
printf '\nCompilerTest relevant content:\n'
sed -n '1,130p' lsp/java/src/test/java/com/itsaky/androidide/lsp/java/compiler/CompilerTest.kt | cat -n
printf '\nPartialReparserImplTest relevant content:\n'
sed -n '1,120p' lsp/java/src/test/java/com/itsaky/androidide/lsp/java/partial/PartialReparserImplTest.kt | cat -n

printf '\n== Deterministic coverage probe: tests mentioning lifecycle methods/states ==\n'
python3 - <<'PY'
import pathlib,re
dirs = [pathlib.Path('lsp/java/src/test')]
terms = ['ensureProjectReset','setupWithProject','CompilerLifecycle','getCompiler','onContentChange','shutdown']
for path in sorted(p for d in dirs for p in d.rglob('*') if p.name.endswith(('.kt','.java')) and 'Test' in p.name.lower()):
    text = path.read_text(errors='ignore')
    hits = [t for t in terms if t in text]
    if hits:
        print(path)
        print('\n'.join(f'  {t}' for t in hits))
PY

Repository: appdevforall/CodeOnTheGo

Length of output: 20074


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect lock usage and public lifecycle entry points to clarify the required test scenarios.
printf '== All JavaLanguageServer lifecycle symbols ==\n'
rg -n '\b(ensureProjectReset|setupWithProject|getCompiler|onContentChange|shutdown|compilerLifecycle|pendingWorkspace|compilerLifecycleLock|CompilerLifecycle)\b' lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt

printf '\n== Lock implementation nearby ==\n'
rg -n 'compilerLifecycleLock|lock\(|withLock\(' lsp/java/src/main/java/com/itsaky/androidide/lsp/java

Repository: appdevforall/CodeOnTheGo

Length of output: 5311


Add lifecycle transition coverage.

The new setupWithProject() / ensureProjectReset() path and lifecycle guards are only exercised indirectly through compile tests. Add tests for the initial deferred reset, reset failure recovery, a new project enqueued during reset, and shutdown() called before the first Java-file interaction.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt`
around lines 214 - 264, Add focused tests covering the compiler lifecycle around
setupWithProject() and ensureProjectReset(): verify the initial reset is
deferred until Java interaction, a reset failure restores a recoverable
lifecycle state, a project queued during RESETTING becomes pending for a
subsequent reset, and shutdown() safely handles a pending reset before any Java
file interaction. Use existing test seams and lifecycle symbols rather than
relying only on compile tests.

Source: Coding guidelines


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
}

Expand Down Expand Up @@ -258,15 +334,21 @@ class JavaLanguageServer : ILanguageServer {
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) {
Expand All @@ -285,10 +367,17 @@ 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 {
ensureProjectReset()
val module =
ProjectManagerImpl.getInstance().findModuleForFile(file!!)
?: return@withLock JavaCompilerService.NO_MODULE_COMPILER
JavaCompilerProvider.get(module)
}
}

private fun updateCachedCompletion(cachedCompletion: CachedCompletion) {
Expand All @@ -314,14 +403,20 @@ 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 {
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()
}
Expand Down
Loading