From deb93eebfbd8a72fe25b49c9e1f8a8f0f2cf0f6b Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Mon, 15 Jun 2026 21:40:49 -0400 Subject: [PATCH 1/5] ADFA-4329: add cycle guard to getCompileModuleProjects Sentry APPDEVFORALL-Z0. Thread a visited set through module/library traversal to stop StackOverflow on cyclic module graphs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../androidide/projects/api/AndroidModule.kt | 42 ++++++++- .../androidide/projects/api/JavaModule.kt | 8 +- .../androidide/projects/api/ModuleProject.kt | 13 ++- .../api/CompileModuleProjectsCycleTest.kt | 94 +++++++++++++++++++ 4 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt index 39a70b9cc6..a51d1b2f72 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt @@ -239,9 +239,30 @@ open class AndroidModule( libraries: List, result: MutableSet, excludeSourceGeneratedClassPath: Boolean = false, + ) { + collectLibraries( + root = root, + libraries = libraries, + result = result, + excludeSourceGeneratedClassPath = excludeSourceGeneratedClassPath, + visited = HashSet(), + ) + } + + private fun collectLibraries( + root: Workspace, + libraries: List, + result: MutableSet, + excludeSourceGeneratedClassPath: Boolean, + visited: MutableSet, ) { val libraryMap = variantDependencies.librariesMap for (library in libraries) { + // Guard against cyclic dependency graphs: expand each graph node at most once. + if (!visited.add(library.key)) { + continue + } + val lib = libraryMap[library.key] ?: continue if (lib.type == AndroidModels.LibraryType.Project) { val module = root.findByPath(lib.projectInfo!!.projectPath) ?: continue @@ -256,12 +277,27 @@ open class AndroidModule( result.add(lib.artifact) } - collectLibraries(root, library.dependencyList, result) + // Note: the recursive call intentionally keeps the original behavior of not propagating + // `excludeSourceGeneratedClassPath` (it defaults to false here); only the visited-set + // cycle guard is added. + collectLibraries( + root = root, + libraries = library.dependencyList, + result = result, + excludeSourceGeneratedClassPath = false, + visited = visited, + ) } } - override fun getCompileModuleProjects(): List { + override fun getCompileModuleProjects(visited: MutableSet): List { val root = IProjectManager.getInstance().workspace ?: return emptyList() + + // Guard against cyclic project-dependency graphs: expand each module at most once. + if (!visited.add(path)) { + return emptyList() + } + val result = mutableListOf() val libraries = variantDependencies.mainArtifact.compileDependencyList @@ -278,7 +314,7 @@ open class AndroidModule( } result.add(module) - result.addAll(module.getCompileModuleProjects()) + result.addAll(module.getCompileModuleProjects(visited)) } return result diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt index e24dc9079a..608a4dd7c5 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt @@ -121,8 +121,14 @@ class JavaModule( override fun getRuntimeDexFiles(): Set = emptySet() - override fun getCompileModuleProjects(): List { + override fun getCompileModuleProjects(visited: MutableSet): List { val root = IProjectManager.getInstance().workspace ?: return emptyList() + + // Guard against cyclic project-dependency graphs: expand each module at most once. + if (!visited.add(path)) { + return emptyList() + } + return this.dependencyList .filter { it.hasModule() && it.scope == SCOPE_COMPILE } .mapNotNull { root.findByPath(it.module.projectPath) } diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt index 5f45acc408..8c5e2195f8 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt @@ -127,7 +127,18 @@ abstract class ModuleProject( * Get the list of module projects with compile scope. This includes transitive module projects as * well. */ - abstract fun getCompileModuleProjects(): List + fun getCompileModuleProjects(): List = getCompileModuleProjects(HashSet()) + + /** + * Get the list of module projects with compile scope (including transitive ones), guarding against + * cyclic project-dependency graphs. + * + * @param visited The set of already-expanded module paths. Each module is expanded at most once, + * keyed by its [path], so a cyclic dependency graph terminates instead of recursing until the + * stack overflows. Implementations must add their own [path] before recursing into dependencies + * and must thread the same set through every recursive call. + */ + abstract fun getCompileModuleProjects(visited: MutableSet): List /** * Check if the given module is a dependency of this module. diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt new file mode 100644 index 0000000000..4f3025626e --- /dev/null +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt @@ -0,0 +1,94 @@ +/* + * 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.projects.api + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +/** + * Verifies that the visited-set guard used by + * [ModuleProject.getCompileModuleProjects] (see [AndroidModule] and [JavaModule]) terminates on a + * cyclic module-dependency graph instead of recursing until the stack overflows (Sentry issue Z0). + * + * Constructing real [AndroidModule]/[JavaModule] instances requires fully-wired protobuf project + * models, a registered workspace and variant dependencies, so this test exercises the exact + * guard pattern (`if (!visited.add(path)) return emptyList()`) over a minimal in-memory graph that + * mirrors the production traversal. + * + * @author Akash Yadav + */ +class CompileModuleProjectsCycleTest { + + /** Minimal stand-in for a module that mirrors the production traversal + cycle guard. */ + private class FakeModule(val path: String) { + val dependencies = mutableListOf() + + fun compileModuleProjects(visited: MutableSet = HashSet()): List { + // Same guard as AndroidModule/JavaModule.getCompileModuleProjects(visited). + if (!visited.add(path)) { + return emptyList() + } + + val result = mutableListOf() + for (dependency in dependencies) { + result.add(dependency) + result.addAll(dependency.compileModuleProjects(visited)) + } + return result + } + } + + @Test + fun `terminates on a two-node dependency cycle`() { + val a = FakeModule(":a") + val b = FakeModule(":b") + // a -> b -> a + a.dependencies.add(b) + b.dependencies.add(a) + + // Without the visited-set guard this would recurse until StackOverflowError. + val result = a.compileModuleProjects() + + assertThat(result.map { it.path }).containsExactly(":b", ":a") + } + + @Test + fun `terminates on a self dependency cycle`() { + val a = FakeModule(":a") + a.dependencies.add(a) + + val result = a.compileModuleProjects() + + assertThat(result.map { it.path }).containsExactly(":a") + } + + @Test + fun `terminates on a longer cycle and expands each module at most once`() { + val a = FakeModule(":a") + val b = FakeModule(":b") + val c = FakeModule(":c") + // a -> b -> c -> a + a.dependencies.add(b) + b.dependencies.add(c) + c.dependencies.add(a) + + val result = a.compileModuleProjects() + + assertThat(result.map { it.path }).containsExactly(":b", ":c", ":a") + } +} From 0103296592fd26d70811593c1eeaed99245d3e06 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Wed, 17 Jun 2026 05:02:12 -0700 Subject: [PATCH 2/5] ADFA-4329: replace FakeModule cycle test with real AndroidModule repro Rewrite CompileModuleProjectsCycleTest to build a cyclic graph of real AndroidModule instances (backed by protobuf VariantDependencies referencing each other as Project libraries) registered in a real Workspace on the production ProjectManagerImpl, instead of a hand-written FakeModule copy of the guard. This drives the actual AndroidModule.getCompileModuleProjects traversal: it throws StackOverflowError on the pre-fix baseline and terminates with the visited-set guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../api/CompileModuleProjectsCycleTest.kt | 143 ++++++++++++------ 1 file changed, 98 insertions(+), 45 deletions(-) diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt index 4f3025626e..440ad5748d 100644 --- a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt @@ -18,76 +18,129 @@ package com.itsaky.androidide.projects.api import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.project.AndroidModels +import com.itsaky.androidide.project.GradleModels +import com.itsaky.androidide.projects.ProjectManagerImpl +import org.junit.After import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config /** - * Verifies that the visited-set guard used by - * [ModuleProject.getCompileModuleProjects] (see [AndroidModule] and [JavaModule]) terminates on a - * cyclic module-dependency graph instead of recursing until the stack overflows (Sentry issue Z0). + * Verifies [AndroidModule.getCompileModuleProjects] (via [ModuleProject.getCompileModuleProjects]) + * terminates on a *cyclic* project-dependency graph instead of recursing until the stack overflows + * (ADFA-4329 / Sentry). * - * Constructing real [AndroidModule]/[JavaModule] instances requires fully-wired protobuf project - * models, a registered workspace and variant dependencies, so this test exercises the exact - * guard pattern (`if (!visited.add(path)) return emptyList()`) over a minimal in-memory graph that - * mirrors the production traversal. + * This builds a cyclic graph of *real* [AndroidModule] instances backed by protobuf project models + * (each module's [AndroidModels.VariantDependencies] references the next module as a `Project` + * library), registers them in a real [Workspace] on the production [ProjectManagerImpl], and drives + * the actual production traversal in [AndroidModule.getCompileModuleProjects]. * - * @author Akash Yadav + * On the pre-fix code (no visited-set guard) a cycle a -> b -> a recurses + * `a.getCompileModuleProjects -> b.getCompileModuleProjects -> a.getCompileModuleProjects -> ...` + * forever and blows the stack with [StackOverflowError]. With the fix it terminates and each module + * is expanded at most once. */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE) class CompileModuleProjectsCycleTest { - /** Minimal stand-in for a module that mirrors the production traversal + cycle guard. */ - private class FakeModule(val path: String) { - val dependencies = mutableListOf() - - fun compileModuleProjects(visited: MutableSet = HashSet()): List { - // Same guard as AndroidModule/JavaModule.getCompileModuleProjects(visited). - if (!visited.add(path)) { - return emptyList() - } - - val result = mutableListOf() - for (dependency in dependencies) { - result.add(dependency) - result.addAll(dependency.compileModuleProjects(visited)) - } - return result + @After + fun tearDown() { + runCatching { ProjectManagerImpl.getInstance().workspace = null } + } + + /** + * Build a real [AndroidModule] at Gradle [path] whose compile-scope project dependencies are + * [moduleDeps] (Gradle project paths). The dependency graph is encoded exactly the way the tooling + * layer encodes it: a [AndroidModels.GraphItem] in the main artifact's compile graph keyed to a + * [AndroidModels.Library] of type [AndroidModels.LibraryType.Project] that points at the dependency + * module via [AndroidModels.ProjectInfo.getProjectPath]. + */ + private fun androidModule(path: String, moduleDeps: List): AndroidModule { + val mainArtifact = AndroidModels.ArtifactDependencies.newBuilder() + val variantDeps = AndroidModels.VariantDependencies.newBuilder().setName("debug") + + for (depPath in moduleDeps) { + val key = "project$depPath" + mainArtifact.addCompileDependency( + AndroidModels.GraphItem.newBuilder().setKey(key).build(), + ) + variantDeps.putLibraries( + key, + AndroidModels.Library.newBuilder() + .setKey(key) + .setType(AndroidModels.LibraryType.Project) + .setProjectInfo( + AndroidModels.ProjectInfo.newBuilder() + .setBuildId(":") + .setProjectPath(depPath) + .build(), + ) + .build(), + ) } + variantDeps.setMainArtifact(mainArtifact.build()) + + val androidProject = AndroidModels.AndroidProject.newBuilder() + .setProjectType(AndroidModels.ProjectType.LibraryProject) + .setVariantDependencies(variantDeps.build()) + .build() + + val gradleProject = GradleModels.GradleProject.newBuilder() + .setName(path.trimStart(':')) + .setPath(path) + .setProjectDirPath("/tmp/cycle-test${path.replace(':', '/')}") + .setBuildDirPath("/tmp/cycle-test${path.replace(':', '/')}/build") + .setBuildScriptPath("/tmp/cycle-test${path.replace(':', '/')}/build.gradle") + .setAndroidProject(androidProject) + .build() + + return AndroidModule(gradleProject) + } + + private fun installWorkspace(vararg modules: ModuleProject) { + val root = GradleProject( + GradleModels.GradleProject.newBuilder().setName("root").setPath(":").build(), + ) + ProjectManagerImpl.getInstance().workspace = + Workspace(rootProject = root, subProjects = modules.toList(), syncIssues = emptyList()) } - @Test - fun `terminates on a two-node dependency cycle`() { - val a = FakeModule(":a") - val b = FakeModule(":b") - // a -> b -> a - a.dependencies.add(b) - b.dependencies.add(a) + @Test(timeout = 30_000) + fun `terminates on a two-node module dependency cycle`() { + // a -> b -> a (compile scope, real AndroidModule instances) + val a = androidModule(":a", listOf(":b")) + val b = androidModule(":b", listOf(":a")) + installWorkspace(a, b) - // Without the visited-set guard this would recurse until StackOverflowError. - val result = a.compileModuleProjects() + // Pre-fix: a -> b -> a -> b -> ... until StackOverflowError. + // With the fix: terminates; each module expanded at most once. + val result = a.getCompileModuleProjects() assertThat(result.map { it.path }).containsExactly(":b", ":a") } - @Test + @Test(timeout = 30_000) fun `terminates on a self dependency cycle`() { - val a = FakeModule(":a") - a.dependencies.add(a) + val a = androidModule(":a", listOf(":a")) + installWorkspace(a) - val result = a.compileModuleProjects() + val result = a.getCompileModuleProjects() assertThat(result.map { it.path }).containsExactly(":a") } - @Test + @Test(timeout = 30_000) fun `terminates on a longer cycle and expands each module at most once`() { - val a = FakeModule(":a") - val b = FakeModule(":b") - val c = FakeModule(":c") // a -> b -> c -> a - a.dependencies.add(b) - b.dependencies.add(c) - c.dependencies.add(a) + val a = androidModule(":a", listOf(":b")) + val b = androidModule(":b", listOf(":c")) + val c = androidModule(":c", listOf(":a")) + installWorkspace(a, b, c) - val result = a.compileModuleProjects() + val result = a.getCompileModuleProjects() assertThat(result.map { it.path }).containsExactly(":b", ":c", ":a") } From 3bc14fe33faee39b0b79a7e994fd21fcc6c2da16 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Wed, 17 Jun 2026 08:07:48 -0700 Subject: [PATCH 3/5] ADFA-4329: report dependency cycles as an actionable error (not silent) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the cycle guard, but distinguish a TRUE cycle (a module already on the DFS recursion path) from a harmless diamond/shared dependency, and log an actionable chain (e.g. ':a -> :b -> :a') when a real cycle is broken — instead of silently returning emptyList and leaving the user with a confusing downstream build error. Thread an ordered recursionPath alongside the existing visited set; add ModuleProject.reportDependencyCycle + the pure, testable formatDependencyCycle. Tests: a diamond/shared dep terminates and is not mis-reported as a cycle; the formatter renders the loop from the first occurrence of the repeated module. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../androidide/projects/api/AndroidModule.kt | 49 ++++++++++++------- .../androidide/projects/api/JavaModule.kt | 15 +++++- .../androidide/projects/api/ModuleProject.kt | 49 +++++++++++++++++-- .../api/CompileModuleProjectsCycleTest.kt | 30 ++++++++++++ 4 files changed, 119 insertions(+), 24 deletions(-) diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt index a51d1b2f72..6c16b8a7a9 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt @@ -290,34 +290,49 @@ open class AndroidModule( } } - override fun getCompileModuleProjects(visited: MutableSet): List { + override fun getCompileModuleProjects( + visited: MutableSet, + recursionPath: ArrayDeque, + ): List { val root = IProjectManager.getInstance().workspace ?: return emptyList() - // Guard against cyclic project-dependency graphs: expand each module at most once. + // True cycle: this module is already an ancestor on the current recursion path + // (e.g. :a -> :b -> :a). Report it and break the cycle. + if (recursionPath.contains(path)) { + reportDependencyCycle(recursionPath, path) + return emptyList() + } + + // Already fully expanded elsewhere (a diamond / shared dependency, not a cycle): dedup. if (!visited.add(path)) { return emptyList() } - val result = mutableListOf() + recursionPath.addLast(path) + try { + val result = mutableListOf() - val libraries = variantDependencies.mainArtifact.compileDependencyList - val libraryMap = variantDependencies.librariesMap - for (library in libraries) { - val lib = libraryMap[library.key] ?: continue - if (lib.type != AndroidModels.LibraryType.Project) { - continue - } + val libraries = variantDependencies.mainArtifact.compileDependencyList + val libraryMap = variantDependencies.librariesMap + for (library in libraries) { + val lib = libraryMap[library.key] ?: continue + if (lib.type != AndroidModels.LibraryType.Project) { + continue + } - val module = root.findByPath(lib.projectInfo!!.projectPath) ?: continue - if (module !is ModuleProject) { - continue + val module = root.findByPath(lib.projectInfo!!.projectPath) ?: continue + if (module !is ModuleProject) { + continue + } + + result.add(module) + result.addAll(module.getCompileModuleProjects(visited, recursionPath)) } - result.add(module) - result.addAll(module.getCompileModuleProjects(visited)) + return result + } finally { + recursionPath.removeLast() } - - return result } override fun hasExternalDependency( diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt index 608a4dd7c5..93c3d4e8b1 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt @@ -121,10 +121,21 @@ class JavaModule( override fun getRuntimeDexFiles(): Set = emptySet() - override fun getCompileModuleProjects(visited: MutableSet): List { + override fun getCompileModuleProjects( + visited: MutableSet, + recursionPath: ArrayDeque, + ): List { val root = IProjectManager.getInstance().workspace ?: return emptyList() - // Guard against cyclic project-dependency graphs: expand each module at most once. + // True cycle guard (defensive: this override is non-recursive — it returns only direct + // compile-scope module dependencies — so it cannot itself extend the recursion path, but the + // check keeps behavior consistent with AndroidModule if that ever changes). + if (recursionPath.contains(path)) { + reportDependencyCycle(recursionPath, path) + return emptyList() + } + + // Already fully expanded elsewhere (a diamond / shared dependency, not a cycle): dedup. if (!visited.add(path)) { return emptyList() } diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt index 8c5e2195f8..7beaddfb9f 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt @@ -56,6 +56,17 @@ abstract class ModuleProject( companion object { private val log = LoggerFactory.getLogger(ModuleProject::class.java) + /** + * Format a dependency cycle as a human-readable chain, e.g. `:a -> :b -> :a`. [recursionPath] + * is the current DFS path of module paths; [repeated] is the module that closes the cycle. The + * chain starts at the first occurrence of [repeated] on the path and ends with [repeated] + * again, so it shows exactly the loop the user needs to break. + */ + internal fun formatDependencyCycle( + recursionPath: Collection, + repeated: String, + ): String = (recursionPath.dropWhile { it != repeated } + repeated).joinToString(" -> ") + @JvmStatic val COMPLETION_MODULE_KEY = Lookup.Key() } @@ -127,18 +138,46 @@ abstract class ModuleProject( * Get the list of module projects with compile scope. This includes transitive module projects as * well. */ - fun getCompileModuleProjects(): List = getCompileModuleProjects(HashSet()) + fun getCompileModuleProjects(): List = + getCompileModuleProjects(HashSet(), ArrayDeque()) /** * Get the list of module projects with compile scope (including transitive ones), guarding against * cyclic project-dependency graphs. * * @param visited The set of already-expanded module paths. Each module is expanded at most once, - * keyed by its [path], so a cyclic dependency graph terminates instead of recursing until the - * stack overflows. Implementations must add their own [path] before recursing into dependencies - * and must thread the same set through every recursive call. + * keyed by its [path], so a diamond/shared dependency is not re-expanded and a cyclic graph + * terminates instead of recursing until the stack overflows. Implementations must add their own + * [path] before recursing into dependencies and must thread the same set through every recursive + * call. + * @param recursionPath The ordered stack of module paths currently being expanded (the DFS path + * from the root call down to this module). Implementations must push their own [path] before + * recursing and pop it afterwards. It distinguishes a *true cycle* (a module that is already an + * ancestor on this path) from a harmless diamond/shared dependency (a module merely seen before + * but not an ancestor), so only real cycles are reported via [reportDependencyCycle]. + */ + abstract fun getCompileModuleProjects( + visited: MutableSet, + recursionPath: ArrayDeque, + ): List + + /** + * Report a detected project-dependency cycle as an actionable error. Called when a module is found + * to be its own ancestor in the compile-dependency graph (e.g. `:a -> :b -> :a`). The traversal + * breaks the cycle (returns without recursing again) to keep the IDE responsive; reporting it + * surfaces the misconfiguration to the user instead of silently swallowing it. + * + * @param recursionPath The current DFS path of module paths. + * @param repeated The module path that was found to already be on [recursionPath]. */ - abstract fun getCompileModuleProjects(visited: MutableSet): List + protected fun reportDependencyCycle(recursionPath: ArrayDeque, repeated: String) { + log.error( + "Module dependency cycle detected: {}. Breaking the cycle to keep the IDE responsive; " + + "the project's module/classpath graph may be incomplete until you remove one of these " + + "dependencies (check the build.gradle of the modules in the cycle).", + formatDependencyCycle(recursionPath, repeated), + ) + } /** * Check if the given module is a dependency of this module. diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt index 440ad5748d..fed1269e66 100644 --- a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt @@ -144,4 +144,34 @@ class CompileModuleProjectsCycleTest { assertThat(result.map { it.path }).containsExactly(":b", ":c", ":a") } + + @Test(timeout = 30_000) + fun `diamond shared dependency terminates and is not treated as a cycle`() { + // a -> b -> d, a -> c -> d. `d` is a shared dependency reached by two paths, NOT a cycle: + // it must be expanded (no infinite loop) and must not trip the cycle guard. + val a = androidModule(":a", listOf(":b", ":c")) + val b = androidModule(":b", listOf(":d")) + val c = androidModule(":c", listOf(":d")) + val d = androidModule(":d", emptyList()) + installWorkspace(a, b, c, d) + + val result = a.getCompileModuleProjects() + + // Terminates (timeout would fail otherwise) and reaches every module in the diamond. + assertThat(result.map { it.path }.toSet()).containsExactly(":b", ":c", ":d") + } + + @Test + fun `formatDependencyCycle renders the loop from the first occurrence of the repeated module`() { + // Two-node, self, and longer cycles render as the chain the user must break. + assertThat(ModuleProject.formatDependencyCycle(listOf(":a", ":b"), ":a")) + .isEqualTo(":a -> :b -> :a") + assertThat(ModuleProject.formatDependencyCycle(listOf(":a"), ":a")) + .isEqualTo(":a -> :a") + assertThat(ModuleProject.formatDependencyCycle(listOf(":a", ":b", ":c"), ":a")) + .isEqualTo(":a -> :b -> :c -> :a") + // The chain starts at the FIRST occurrence of the repeated node, dropping any prefix before it. + assertThat(ModuleProject.formatDependencyCycle(listOf(":a", ":b", ":c"), ":b")) + .isEqualTo(":b -> :c -> :b") + } } From 8269ee963e78fde771d235218559d75f3c8c93ec Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Thu, 18 Jun 2026 03:52:48 -0700 Subject: [PATCH 4/5] ADFA-4329: also guard getCompileClasspaths against cyclic module graphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The StackOverflow fix only covered getCompileModuleProjects; the sibling classpath traversal had the same defect — AndroidModule.collectLibraries' cross-module 'module.getCompileClasspaths(...)' (and JavaModule's per-dependency call) recursed a cyclic PROJECT graph with no module-level visited set, so :a -> :b -> :a overflowed the stack on classpath resolution (e.g. LSP resolving a file's classpath), independent of getCompileModuleProjects. Thread a module-path 'visited' set through getCompileClasspaths in ModuleProject/AndroidModule/JavaModule (each module contributes its classpaths at most once; a revisit returns emptySet). The public 1-arg getCompileClasspaths is preserved (delegates with a fresh set) so external callers are unaffected. The user-facing cycle report still comes from getCompileModuleProjects; this only breaks the cycle. Tests: getCompileClasspaths terminates on two-node and self cycles. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../androidide/projects/api/AndroidModule.kt | 36 +++++++++---------- .../androidide/projects/api/JavaModule.kt | 14 +++++--- .../androidide/projects/api/ModuleProject.kt | 17 ++++++++- .../api/CompileModuleProjectsCycleTest.kt | 25 +++++++++++++ 4 files changed, 69 insertions(+), 23 deletions(-) diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt index 6c16b8a7a9..d2e3465a27 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt @@ -141,8 +141,18 @@ open class AndroidModule( addAll(getSelectedVariant()?.mainArtifact?.classJars ?: emptyList()) } - override fun getCompileClasspaths(excludeSourceGeneratedClassPath: Boolean): Set { + override fun getCompileClasspaths( + excludeSourceGeneratedClassPath: Boolean, + visited: MutableSet, + ): Set { val project = IProjectManager.getInstance().workspace ?: return emptySet() + + // Guard against cyclic project-dependency graphs: contribute each module's classpaths at most + // once so a cycle (:a -> :b -> :a) terminates instead of recursing until the stack overflows. + if (!visited.add(path)) { + return emptySet() + } + val result = mutableSetOf() if (excludeSourceGeneratedClassPath) { // TODO: The mainArtifact.classJars are technically generated from source files @@ -159,6 +169,8 @@ open class AndroidModule( libraries = variantDependencies.mainArtifact?.compileDependencyList ?: emptyList(), result = result, excludeSourceGeneratedClassPath = excludeSourceGeneratedClassPath, + visited = HashSet(), + moduleVisited = visited, ) return result } @@ -234,31 +246,17 @@ open class AndroidModule( return result } - private fun collectLibraries( - root: Workspace, - libraries: List, - result: MutableSet, - excludeSourceGeneratedClassPath: Boolean = false, - ) { - collectLibraries( - root = root, - libraries = libraries, - result = result, - excludeSourceGeneratedClassPath = excludeSourceGeneratedClassPath, - visited = HashSet(), - ) - } - private fun collectLibraries( root: Workspace, libraries: List, result: MutableSet, excludeSourceGeneratedClassPath: Boolean, visited: MutableSet, + moduleVisited: MutableSet, ) { val libraryMap = variantDependencies.librariesMap for (library in libraries) { - // Guard against cyclic dependency graphs: expand each graph node at most once. + // Guard against cyclic dependency graphs within this module: expand each graph node once. if (!visited.add(library.key)) { continue } @@ -270,7 +268,8 @@ open class AndroidModule( continue } - result.addAll(module.getCompileClasspaths(excludeSourceGeneratedClassPath)) + // Cross-module recursion threads moduleVisited so a cyclic PROJECT graph terminates. + result.addAll(module.getCompileClasspaths(excludeSourceGeneratedClassPath, moduleVisited)) } else if (lib.type == AndroidModels.LibraryType.ExternalAndroidLibrary && lib.hasAndroidLibraryData()) { result.addAll(lib.androidLibraryData.compileJarFiles) } else if (lib.type == AndroidModels.LibraryType.ExternalJavaLibrary && lib.hasArtifactPath()) { @@ -286,6 +285,7 @@ open class AndroidModule( result = result, excludeSourceGeneratedClassPath = false, visited = visited, + moduleVisited = moduleVisited, ) } } diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt index 93c3d4e8b1..8d11a09e2c 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt @@ -86,15 +86,21 @@ class JavaModule( override fun getModuleClasspaths(): Set = mutableSetOf(classesJar) - override fun getCompileClasspaths(excludeSourceGeneratedClassPath: Boolean): Set { + override fun getCompileClasspaths( + excludeSourceGeneratedClassPath: Boolean, + visited: MutableSet, + ): Set { + // Guard against cyclic project-dependency graphs: contribute each module's classpaths once. + if (!visited.add(path)) { + return emptySet() + } + val classpaths = if (excludeSourceGeneratedClassPath) mutableSetOf() else getModuleClasspaths().toMutableSet() getCompileModuleProjects().forEach { classpaths.addAll( - it.getCompileClasspaths( - excludeSourceGeneratedClassPath - ) + it.getCompileClasspaths(excludeSourceGeneratedClassPath, visited) ) } diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt index 7beaddfb9f..bbef5fcdaf 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt @@ -113,9 +113,24 @@ abstract class ModuleProject( * source files of this module or its dependencies. Defaults to `false`. * @return The source directories. */ - abstract fun getCompileClasspaths(excludeSourceGeneratedClassPath: Boolean): Set + fun getCompileClasspaths(excludeSourceGeneratedClassPath: Boolean): Set = + getCompileClasspaths(excludeSourceGeneratedClassPath, HashSet()) fun getCompileClasspaths() = getCompileClasspaths(false) + /** + * Variant of [getCompileClasspaths] that guards against cyclic project-dependency graphs. + * + * @param visited module paths already expanded in this traversal. Each module contributes its + * classpaths at most once; a module already in [visited] returns an empty set, so a cyclic graph + * (e.g. `:a -> :b -> :a`) terminates instead of recursing until the stack overflows. The + * user-facing cycle report is emitted once by [getCompileModuleProjects]; this method only breaks + * the cycle. + */ + abstract fun getCompileClasspaths( + excludeSourceGeneratedClassPath: Boolean, + visited: MutableSet, + ): Set + /** * Get the intermediate build output classpaths for this module. * This includes compiled .class files from the build directory that aren't packaged into JARs yet. diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt index fed1269e66..036957e079 100644 --- a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt @@ -161,6 +161,31 @@ class CompileModuleProjectsCycleTest { assertThat(result.map { it.path }.toSet()).containsExactly(":b", ":c", ":d") } + @Test(timeout = 30_000) + fun `getCompileClasspaths terminates on a cyclic module graph`() { + // a -> b -> a. Pre-fix the sibling classpath traversal had no cross-module guard, so + // a.getCompileClasspaths -> b.getCompileClasspaths -> a.getCompileClasspaths -> ... overflowed + // the stack just like getCompileModuleProjects did. With the fix it terminates. + val a = androidModule(":a", listOf(":b")) + val b = androidModule(":b", listOf(":a")) + installWorkspace(a, b) + + val result = a.getCompileClasspaths() + + // Terminates (timeout would fail otherwise) and still contributes this module's own classpaths. + assertThat(result).contains(a.getGeneratedJar()) + } + + @Test(timeout = 30_000) + fun `getCompileClasspaths terminates on a self dependency cycle`() { + val a = androidModule(":a", listOf(":a")) + installWorkspace(a) + + val result = a.getCompileClasspaths() + + assertThat(result).contains(a.getGeneratedJar()) + } + @Test fun `formatDependencyCycle renders the loop from the first occurrence of the repeated module`() { // Two-node, self, and longer cycles render as the chain the user must break. From b9b427967f92f5a89af37f563ac51857bccffc65 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 19 Jun 2026 04:48:57 -0700 Subject: [PATCH 5/5] ADFA-4329: add KDoc for docstring coverage (CodeRabbit) Co-Authored-By: Claude Opus 4.8 --- .../itsaky/androidide/projects/api/AndroidModule.kt | 13 +++++++++++++ .../projects/api/CompileModuleProjectsCycleTest.kt | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt index d2e3465a27..c12d27711d 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt @@ -246,6 +246,19 @@ open class AndroidModule( return result } + /** + * Recursively collect the compile classpath entries contributed by the given dependency-graph + * [libraries] into [result], guarding against cycles. + * + * @param root The workspace used to resolve project dependencies by path. + * @param libraries The dependency-graph nodes to expand at this level. + * @param result The accumulating set of classpath files. + * @param excludeSourceGeneratedClassPath Whether to exclude source-generated classpath entries. + * @param visited Keys of dependency-graph nodes already expanded within this module; each node is + * expanded at most once so a cyclic graph terminates. + * @param moduleVisited Module paths already expanded across modules; threaded into cross-module + * recursion so a cyclic PROJECT graph terminates. + */ private fun collectLibraries( root: Workspace, libraries: List, diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt index 036957e079..5e1cfacee0 100644 --- a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt @@ -100,6 +100,7 @@ class CompileModuleProjectsCycleTest { return AndroidModule(gradleProject) } + /** Install a real [Workspace] containing the given [modules] on the production [ProjectManagerImpl]. */ private fun installWorkspace(vararg modules: ModuleProject) { val root = GradleProject( GradleModels.GradleProject.newBuilder().setName("root").setPath(":").build(), @@ -108,6 +109,7 @@ class CompileModuleProjectsCycleTest { Workspace(rootProject = root, subProjects = modules.toList(), syncIssues = emptyList()) } + /** A two-node cycle (`:a -> :b -> :a`) terminates and expands each module exactly once. */ @Test(timeout = 30_000) fun `terminates on a two-node module dependency cycle`() { // a -> b -> a (compile scope, real AndroidModule instances) @@ -122,6 +124,7 @@ class CompileModuleProjectsCycleTest { assertThat(result.map { it.path }).containsExactly(":b", ":a") } + /** A self-dependency (`:a -> :a`) terminates and reports the module once. */ @Test(timeout = 30_000) fun `terminates on a self dependency cycle`() { val a = androidModule(":a", listOf(":a")) @@ -132,6 +135,7 @@ class CompileModuleProjectsCycleTest { assertThat(result.map { it.path }).containsExactly(":a") } + /** A longer cycle (`:a -> :b -> :c -> :a`) terminates and expands each module at most once. */ @Test(timeout = 30_000) fun `terminates on a longer cycle and expands each module at most once`() { // a -> b -> c -> a @@ -145,6 +149,7 @@ class CompileModuleProjectsCycleTest { assertThat(result.map { it.path }).containsExactly(":b", ":c", ":a") } + /** A diamond (shared dependency reached by two paths) terminates and is not flagged as a cycle. */ @Test(timeout = 30_000) fun `diamond shared dependency terminates and is not treated as a cycle`() { // a -> b -> d, a -> c -> d. `d` is a shared dependency reached by two paths, NOT a cycle: @@ -161,6 +166,7 @@ class CompileModuleProjectsCycleTest { assertThat(result.map { it.path }.toSet()).containsExactly(":b", ":c", ":d") } + /** [AndroidModule.getCompileClasspaths] terminates on a cyclic module graph (`:a -> :b -> :a`). */ @Test(timeout = 30_000) fun `getCompileClasspaths terminates on a cyclic module graph`() { // a -> b -> a. Pre-fix the sibling classpath traversal had no cross-module guard, so @@ -176,6 +182,7 @@ class CompileModuleProjectsCycleTest { assertThat(result).contains(a.getGeneratedJar()) } + /** [AndroidModule.getCompileClasspaths] terminates on a self-dependency cycle (`:a -> :a`). */ @Test(timeout = 30_000) fun `getCompileClasspaths terminates on a self dependency cycle`() { val a = androidModule(":a", listOf(":a")) @@ -186,6 +193,10 @@ class CompileModuleProjectsCycleTest { assertThat(result).contains(a.getGeneratedJar()) } + /** + * [ModuleProject.formatDependencyCycle] renders the loop starting at the first occurrence of the + * repeated module, for two-node, self, and longer cycles. + */ @Test fun `formatDependencyCycle renders the loop from the first occurrence of the repeated module`() { // Two-node, self, and longer cycles render as the chain the user must break.