Skip to content

ADFA-3863 Fix LayoutInflaterTest hang that wedged Jacoco/SonarQube CI #1371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hal-eisen-adfa wants to merge 1 commit into
base: stage
Choose a base branch
from
Open

ADFA-3863 Fix LayoutInflaterTest hang that wedged Jacoco/SonarQube CI #1371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .github/workflows/analyze.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ jobs:
restore-keys: ${{ runner.os }}-sonar

- name: Build and analyze
timeout-minutes: 60
env:
GRADLE_OPTS: "-Xmx10g -XX:MaxMetaspaceSize=512m"
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
Expand Down
5 changes: 5 additions & 0 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ subprojects {
// Continue even if tests fail, so coverage data is written
ignoreFailures = true

// Backstop: kill any individual Test task that runs longer than 10 minutes.
// Prevents a single hung test JVM (e.g. the Tooling API child) from burning
// the entire CI job budget.
timeout.set(java.time.Duration.ofMinutes(10))

// Attach jacoco agent
extensions.configure<JacocoTaskExtension> {
isIncludeNoLocationClasses = true
Expand Down
22 changes: 20 additions & 2 deletions ...ing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import java.nio.file.Path
import java.util.Collections
import java.util.UUID
import java.util.concurrent.CompletableFuture
import java.util.concurrent.TimeUnit
import kotlin.io.path.bufferedReader
import kotlin.io.path.bufferedWriter
import kotlin.io.path.deleteIfExists
Expand Down Expand Up @@ -193,11 +194,28 @@ object ToolingApiTestLauncher {
// perform the action
ToolingApiTestScope(server, gradleBuild, result).action()
} finally {
server.cancelCurrentBuild().get()
server.shutdown().get()
// Bound every shutdown step so a stalled child JVM or hung RPC can't wedge
// the calling test (and the whole Gradle worker) for hours.
try {
server.cancelCurrentBuild().get(SHUTDOWN_RPC_TIMEOUT_SECONDS, TimeUnit.SECONDS)
} catch (error: Throwable) {
println("[ToolingApiTestLauncher] cancelCurrentBuild failed or timed out: ${error.message}")
}
try {
server.shutdown().get(SHUTDOWN_RPC_TIMEOUT_SECONDS, TimeUnit.SECONDS)
} catch (error: Throwable) {
println("[ToolingApiTestLauncher] shutdown failed or timed out: ${error.message}")
}
if (!proc.waitFor(PROCESS_EXIT_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
println("[ToolingApiTestLauncher] child JVM still alive after shutdown RPC; destroying forcibly")
proc.destroyForcibly()
}
}
}

private const val SHUTDOWN_RPC_TIMEOUT_SECONDS = 30L
private const val PROCESS_EXIT_TIMEOUT_SECONDS = 30L

private fun createProcessCmd(
jar: String,
sysProps: Map<String, String> = emptyMap(),
Expand Down
9 changes: 9 additions & 0 deletions xml-inflater/src/test/java/com/itsaky/androidide/inflater/LayoutInflaterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,24 @@ import com.itsaky.androidide.projects.api.AndroidModule
import com.itsaky.androidide.projects.models.projectDir
import com.itsaky.androidide.projects.util.findAppModule
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.rules.Timeout
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import java.io.File
import java.util.concurrent.TimeUnit

/** @author Akash Yadav */
@RunWith(RobolectricTestRunner::class)
class LayoutInflaterTest {

// Fail any single test in this class after 2 minutes instead of hanging indefinitely.
// The Tooling API child JVM has been observed to wedge in CI; bounding the test here
// turns a 3-hour job timeout into a fast, attributable failure.
@get:Rule
val timeout: Timeout = Timeout(2, TimeUnit.MINUTES)

@Before
fun `setup project`() {
XmlInflaterTest.initIfNeeded()
Expand Down
19 changes: 13 additions & 6 deletions xml-inflater/src/test/java/com/itsaky/androidide/inflater/XmlInflaterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,25 @@ import java.util.concurrent.atomic.AtomicBoolean

@Ignore("Test utility provider")
object XmlInflaterTest {
private var init = AtomicBoolean(false)
private val init = AtomicBoolean(false)
internal val activity by lazy { Robolectric.buildActivity(AppCompatActivity::class.java).get() }

fun initIfNeeded() {
if (init.get()) {
// Atomic claim of the init slot. If another caller already won the race,
// they are responsible for completing setup; we just return.
if (!init.compareAndSet(false, true)) {
return
}
Comment on lines +40 to 44
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Non-winning callers can continue before initialization finishes.

compareAndSet prevents duplicate init, but losers return immediately and may use the inflater before setup completes. This creates a race/flaky path when tests run in parallel. Use a wait/notify (or latch/state-machine) so non-winning callers block until init reaches success/failure.

Suggested fix (state machine + wait/notify) 
 object XmlInflaterTest {
-	private val init = AtomicBoolean(false)
+	private enum class InitState { UNINITIALIZED, INITIALIZING, INITIALIZED }
+	private val initLock = Object()
+	`@Volatile` private var initState = InitState.UNINITIALIZED
 	internal val activity by lazy { Robolectric.buildActivity(AppCompatActivity::class.java).get() }

 	fun initIfNeeded() {
-		// Atomic claim of the init slot. If another caller already won the race,
-		// they are responsible for completing setup; we just return.
-		if (!init.compareAndSet(false, true)) {
-			return
-		}
+		synchronized(initLock) {
+			while (initState == InitState.INITIALIZING) initLock.wait()
+			if (initState == InitState.INITIALIZED) return
+			initState = InitState.INITIALIZING
+		}

 		try {
 			ToolingApiTestLauncher.launchServer {
 				assertThat(result is InitializeResult.Success).isTrue()
 				runBlocking { IProjectManager.getInstance().setup(gradleBuild.get()) }
 			}
+			synchronized(initLock) {
+				initState = InitState.INITIALIZED
+				initLock.notifyAll()
+			}
 		} catch (error: Throwable) {
-			// Roll back so subsequent tests can retry rather than silently skipping setup.
-			init.set(false)
+			synchronized(initLock) {
+				initState = InitState.UNINITIALIZED
+				initLock.notifyAll()
+			}
 			throw error
 		}
 	}
 }

Also applies to: 46-55

🤖 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 `@xml-inflater/src/test/java/com/itsaky/androidide/inflater/XmlInflaterTest.kt`
around lines 40 - 44, The current init.compareAndSet(false, true) lets
non-winning callers return immediately and use the inflater before setup
finishes; change the init logic so losers block until initialization completes
(success or failure) instead of returning. Replace the AtomicBoolean init with a
state machine or synchronization primitive (e.g., an enum/AtomicInteger states
like UNINITIALIZED/INITIALIZING/READY/FAILED plus wait/notify or a
CountDownLatch) and update the initialization block and all callers to: the
winning thread performs setup and sets the terminal state (READY or FAILED),
then notifies/writes the latch, while non-winning threads wait for that terminal
state and then proceed or throw based on success/failure; ensure you update the
same init usage shown in XmlInflaterTest.kt (the init variable and the init
compareAndSet block spanning the original lines and the related 46-55 section).


ToolingApiTestLauncher.launchServer {
assertThat(result is InitializeResult.Success).isTrue()
runBlocking { IProjectManager.getInstance().setup(gradleBuild.get()) }
init.set(true)
try {
ToolingApiTestLauncher.launchServer {
assertThat(result is InitializeResult.Success).isTrue()
runBlocking { IProjectManager.getInstance().setup(gradleBuild.get()) }
}
} catch (error: Throwable) {
// Roll back so subsequent tests can retry rather than silently skipping setup.
init.set(false)
throw error
}
}
}
Expand Down
Loading