From 072c9330d4c5255a15ceda2dc5d6cfbaff2d9085 Mon Sep 17 00:00:00 2001
From: JP Cottin <jpcottin@gmail.com>
Date: Sat, 25 Jul 2026 19:26:58 -0700
Subject: [PATCH] Fix the race when reading the emulator's process group

setsid(2) runs asynchronously in the child, so for a brief window after
backgrounding the launcher its pid still reports the *shell's* process
group. Reading the pgid exactly once loses that race intermittently: the
guard then sees EMU_PGID == SELF_PGID, correctly refuses to group-kill our
own step, and silently drops to the single-pid fallback -- so the process
group scoping added in #21 was not reliably active.

Observed on the runner in 1 of 4 boot cycles (#22's run); 0 of 4 in #21's,
which is why it was missed. Consequence was mild -- 'adb emu kill' is the
primary shutdown path and the fallback still terminates the launcher -- but
the scoping is the whole point of that change.

Poll until the pgid settles instead, giving up only if the emulator dies or
after 10s. The first iteration is identical to the previous single read, so
this can only ever do better. Also log the resolved pgid, so a future run
shows whether isolation actually happened rather than staying silent.

The failure cannot be reproduced on an unloaded dev machine (0 losses in 40
trials); forcing the timing -- a child that lingers in our process group for
300ms before setsid -- the old single read loses 10/10 and the polling
version loses 0/10.
---
 .github/workflows/ci.yml | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index a880981..c083f37 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -585,11 +585,24 @@ jobs:
             # whose command line happens to contain "emulators/latest".
             setsid "$SDK/emulators/latest/emulator" @test -no-window -gpu auto -noaudio -no-boot-anim -camera-back none -memory 4096 -verbose -show-kernel -debug-metrics -metrics-collection > "${{ github.workspace }}/emulator_run$N.txt" 2>&1 &
             EMU_PID=$!
-            EMU_PGID="$(ps -o pgid= -p "$EMU_PID" 2>/dev/null | tr -d ' ')"
-            # Never group-kill our own process group (would take out this step).
-            if [ -z "$EMU_PGID" ] || [ "$EMU_PGID" = "$SELF_PGID" ]; then
+            # setsid(2) runs asynchronously in the child, so for a brief moment
+            # the pid still reports OUR process group. Reading it once loses
+            # that race intermittently (observed 1 run in 4), silently dropping
+            # to the single-pid fallback -- so poll until it settles.
+            # Never group-kill our own process group: that would take out this
+            # step, hence the SELF_PGID comparison rather than a plain -n test.
+            EMU_PGID=""
+            for _ in $(seq 1 20); do
+              P="$(ps -o pgid= -p "$EMU_PID" 2>/dev/null | tr -d ' ')"
+              if [ -n "$P" ] && [ "$P" != "$SELF_PGID" ]; then EMU_PGID="$P"; break; fi
+              # If the emulator died there is nothing left to isolate.
+              kill -0 "$EMU_PID" 2>/dev/null || break
+              sleep 0.5
+            done
+            if [ -n "$EMU_PGID" ]; then
+              echo "RUN $N emulator pid=$EMU_PID isolated in process group $EMU_PGID (self=$SELF_PGID)"
+            else
               echo "WARNING: could not isolate a process group for the emulator; falling back to single-pid shutdown"
-              EMU_PGID=""
             fi
             tail -F "${{ github.workspace }}/emulator_run$N.txt" &
             TAIL_PID=$!