From e71d9fad23c791c88adbe9b967fd9c3cc66a3eba Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Tue, 21 Jul 2026 16:03:44 -0500 Subject: [PATCH 1/3] Fix flaky RunningApplicationProcessSpec stop timeout "stop terminates a running process and removes the PID file" flakes (~2% of CI runs, apache/grails-core#16030) because it budgets only 15000ms for RunningApplicationProcess.stop() to observe the spawned process's exit. stop() destroys the process then blocks on ProcessHandle.onExit().get(timeoutMillis, MILLISECONDS); on a contended CI runner (parallel Gradle test forks each spawning subprocess children) the JVM's process-reaper notification can occasionally lag past that budget, causing stop() to fall through to STILL_RUNNING instead of STOPPED. This is a timing-budget issue, not a logic bug in stop()/awaitExit(). Raise the timeout passed at the call site from 15000ms to 30000ms to give more headroom on loaded runners, mirroring the same fix pattern already applied to this file for a prior Windows-specific race (6c76333ea9, a13c38c3fd). Co-Authored-By: Claude Sonnet 5 --- .../grails/cli/gradle/RunningApplicationProcessSpec.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy b/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy index 1931d2154c2..c65e7cf9012 100644 --- a/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy +++ b/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy @@ -208,8 +208,8 @@ class RunningApplicationProcessSpec extends Specification { expect: RunningApplicationProcess.isRunning(pidFile) - when: - def result = RunningApplicationProcess.stop(pidFile, 15000) + when: "a generous timeout budget gives headroom for reaper-notification lag on a contended CI runner" + def result = RunningApplicationProcess.stop(pidFile, 30000) then: result == RunningApplicationProcess.StopResult.STOPPED From 2d617df10c7d6bf09174d3d3d9dcae5fcc766003 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sun, 26 Jul 2026 23:28:43 -0500 Subject: [PATCH 2/3] Fix the actual reaper-thread flake in RunningApplicationProcessSpec Per review, the previous 15s->30s stop() timeout bump could not have been the fix: awaitExit()'s fallback checks !process.isAlive() which is OS-backed and unaffected by CI contention. The real suspect is the test's own tail assertion, process.waitFor(10, SECONDS), which blocks on the JVM's internal reaper thread and can lag under CI load independently of whether the process has actually exited - the same class of issue two earlier fixes on this file addressed. Replace it with a PollingConditions wait on the OS-backed isAlive() check, and tidy the when: label per review to describe the stimulus and note the 30_000 budget matches stop-app.groovy's own value. Co-Authored-By: Claude Sonnet 5 --- .../RunningApplicationProcessSpec.groovy | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy b/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy index c65e7cf9012..249477ea6bb 100644 --- a/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy +++ b/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy @@ -23,6 +23,7 @@ import java.util.concurrent.TimeUnit import spock.lang.Specification import spock.lang.TempDir +import spock.util.concurrent.PollingConditions /** * Tests for {@link RunningApplicationProcess}, the PID file based mechanism that lets @@ -208,14 +209,21 @@ class RunningApplicationProcessSpec extends Specification { expect: RunningApplicationProcess.isRunning(pidFile) - when: "a generous timeout budget gives headroom for reaper-notification lag on a contended CI runner" - def result = RunningApplicationProcess.stop(pidFile, 30000) + when: "the application is stopped" + // Same budget the shipped stop-app command passes (grails-profiles/base/commands/stop-app.groovy) + def result = RunningApplicationProcess.stop(pidFile, 30_000) then: result == RunningApplicationProcess.StopResult.STOPPED !pidFile.exists() - // stop() observes exit via the ProcessHandle; the Process object's own reaper can lag - // behind on Windows, so wait for it rather than sampling isAlive() immediately - process.waitFor(10, TimeUnit.SECONDS) + + and: "the JVM's own process bookkeeping catches up with the OS-level exit stop() already observed" + // Process.isAlive() queries OS process state directly, like stop()'s own awaitExit() fallback. + // Process.waitFor(timeout) instead blocks on the JVM's internal reaper thread, whose notification + // can lag under CI contention independently of whether the process has actually exited - polling + // the OS-backed check avoids that lag causing a spurious failure here. + new PollingConditions(timeout: 30, initialDelay: 0, delay: 0.1).eventually { + assert !process.isAlive() + } } } From 3811fcefc763c15993095c39c5b2eae11043bfa7 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 31 Jul 2026 19:23:59 -0500 Subject: [PATCH 3/3] Poll ProcessHandle.isAlive(), not Process.isAlive(), in the reaper-lag fix The prior commit's rationale was wrong: java.lang.Process#isAlive() is implemented as `!hasExited`, a flag set only when the same async ProcessHandleImpl reaper-thread completion that backs waitFor() and onExit() resolves (confirmed against the JDK 21 source for ProcessImpl/ ProcessHandleImpl). It is not a live OS query, so polling it with PollingConditions still depends on that reaper notification landing - it just widens the budget from 10s to 30s rather than removing the dependency. process.toHandle().isAlive() delegates to ProcessHandleImpl's native isAlive0(), which queries the OS process table directly on every call - the same mechanism RunningApplicationProcess.awaitExit()'s fallback relies on. Poll that instead so the assertion is genuinely immune to reaper lag, regardless of how generous the timeout is. Co-Authored-By: Claude Sonnet 5 --- .../cli/gradle/RunningApplicationProcessSpec.groovy | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy b/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy index 249477ea6bb..4074468f013 100644 --- a/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy +++ b/grails-shell-cli/src/test/groovy/org/grails/cli/gradle/RunningApplicationProcessSpec.groovy @@ -218,12 +218,14 @@ class RunningApplicationProcessSpec extends Specification { !pidFile.exists() and: "the JVM's own process bookkeeping catches up with the OS-level exit stop() already observed" - // Process.isAlive() queries OS process state directly, like stop()'s own awaitExit() fallback. - // Process.waitFor(timeout) instead blocks on the JVM's internal reaper thread, whose notification - // can lag under CI contention independently of whether the process has actually exited - polling - // the OS-backed check avoids that lag causing a spurious failure here. + // process.isAlive() (java.lang.Process) is backed by the same async reaper-thread completion + // as process.waitFor(timeout) - it is NOT a live OS query, so polling it would still be exposed + // to the same notification lag under CI contention. process.toHandle().isAlive() delegates to + // ProcessHandle's native isAlive0() check instead, which queries the OS process table directly + // on every call, exactly like stop()'s own awaitExit() fallback - so it is unaffected by reaper + // lag regardless of how long we poll for. new PollingConditions(timeout: 30, initialDelay: 0, delay: 0.1).eventually { - assert !process.isAlive() + assert !process.toHandle().isAlive() } } }