Preview multirun: authenticate the emulator console so shutdown is graceful - #21
Merged
Merged
Conversation
The emulator console starts unauthenticated and exposes only help/ping/auth/quit/avd, so 'adb emu kill' answered "KO: unknown command" and did nothing -- the job fell back to SIGTERM, and the emulator kept running (and the game kept playing) for up to two minutes before it froze. Root cause: the preview emulator READS $HOME/.emulator_console_auth_token but never creates it, and a fresh runner has no such file. Creating it before the first boot unlocks the full command set (kill, restart, ...) and turns 'adb emu kill' into a real graceful shutdown that saves the snapshot and exits in a couple of seconds. Both the emulator and adb read that same file, so the token value is arbitrary. Four related fixes: 1. Pre-create the console auth token; 'adb emu kill' becomes the primary shutdown path and the SIGTERM dance is now only a 90s safety net. 2. Take the exit screenshot LAST, after the diagnostics and the liveness probe, so it sits ~1s from the freeze instead of 15-20s before it. That gap is why runN-entry never resembled run(N-1)'s exit shot. 3. Replace the 'ls snapshots/default_boot' diagnostic, which only ever listed a 1-byte version.txt: the VM state lives inside the qcow2 overlays, so report 'qemu-img snapshot -l' on userdata-qemu.img.qcow2 plus the size of the GPU renderersave dir. The packaged qemu-img has a broken RUNPATH and needs LD_LIBRARY_PATH set explicitly. 4. Launch the emulator via setsid and scope the shutdown fallback to its own process group, instead of pkill -f "emulators/latest", which would match (and kill) any other preview emulator on the host -- or any process whose command line merely contains that string. Verified locally against the same preview build as CI (0.0.1-15885905) over four boot/snapshot cycles: the app process survives every restore, each shutdown completes in ~3s, and the entry-vs-exit drift drops from a doubled score to ~3% within the same wave.
jpcottin
added a commit
that referenced
this pull request
Jul 26, 2026
* Extract the preview-emulator setup into a composite action The two emulator-preview-* jobs each carried their own copy of the same setup: enable KVM, refresh cmdline-tools, install the ps16k image, create the AVD (with the 30-line hand-written-ini fallback), install emulators;latest, and install libpulse0. The copies had already started to drift -- the multi-run one had lost three explanatory comments and the "AVD root ini" / "preview emulator version" diagnostics -- and #21 widened the gap by rewriting shutdown logic in only one of them. Move all of it to .github/actions/preview-emulator, parameterised by api-level / target / abi / avd-name / device / ram-size / cores / disk-size so a second image can be added without another copy. ci.yml loses 153 lines. The action also owns the console auth token workaround from #21, which means the other preview job's "Stop emulator" step (adb emu kill) stops being a silent no-op too. That step now defers to an existing token instead of overwriting it, so once a preview build ships the proposed upstream fix that creates the file itself, the workaround becomes inert and can simply be deleted. Only the two non-blocking preview jobs are touched; build, test, smoke and android-cli-experiment are byte-identical. Verified by rendering the action's AVD step with its default inputs and running it against a sandbox HOME: the generated config.ini is byte-identical to what the inline version produced. (That check also caught dead code carried over from the inline copies -- a sed meant to strip heredoc indentation that never matched anything, since the YAML block scalar had already stripped it.) * Fix composite-action pipefail breaking the sdkmanager installs Composite action steps must declare `shell: bash`, which GitHub runs as `bash --noprofile --norc -e -o pipefail`. The inline workflow steps this action replaced had no `shell:` key, so they got the workflow default `bash -e` -- without pipefail. Under pipefail, `yes | sdkmanager ... > /dev/null` fails: yes takes SIGPIPE when sdkmanager exits and the pipeline reports 141. Both preview jobs died on the first install. Turn pipefail off for the three steps that pipe into sdkmanager/avdmanager. That restores the previous semantics exactly -- the pipeline reports the last command's status, so a genuine sdkmanager failure still fails the step; only the SIGPIPE from yes is ignored. The remaining pipeline in the action (echo | sudo tee) is safe: tee consumes its input fully, and a tee failure propagating is the behaviour we want.
jpcottin
added a commit
that referenced
this pull request
Jul 26, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The
Emulator Preview experiment multi-runjob worked aroundadb emu killbeing a no-op by SIGTERMing the launcher. That workaround was treating a symptom.The real cause: the emulator console starts unauthenticated, and in that state it exposes only
killisn't in that set, so the console answersKO: unknown command, try 'help'.— exactly what the job logs show. The preview emulator reads$HOME/.emulator_console_auth_tokenbut never creates it, and a fresh runner has no such file.Creating that file before the first boot unlocks the full command set (
kill,restart,screenrecord, …) and makesadb emu killa genuine graceful shutdown. Both the emulator andadbread the same file, so the token value is arbitrary.What changed
adb emu killis now the primary shutdown path; the emulator saves its snapshot and exits in ~3s. The SIGTERM path survives only as a 90s safety net.runN-entrynever resembledrun(N-1)'s exit shot — the game kept playing through the whole wait.ls snapshots/default_boot/only ever listed a 1-byteversion.txt; the VM state lives inside the qcow2 overlays. Now reportsqemu-img snapshot -lonuserdata-qemu.img.qcow2plus the size of the GPUrenderersave/dir. The packagedqemu-imghas a broken RUNPATH (it points at Bazel build dirs that aren't shipped), soLD_LIBRARY_PATHis set explicitly.setsidand the fallback kill targets its own process group.pkill -f "emulators/latest"would match any other preview emulator on the host — or any process whose command line merely contains that string.Verification
Replayed locally against the same preview build as CI (
0.0.1-15885905), same ps16k image, four boot/snapshot cycles:adb emu kill; the SIGTERM fallback never fires.qemu-img snapshot -lreports thedefault_bootsnapshot;renderersave/is ~950 MB.The no-token failure was reproduced from a throwaway
HOME(restricted command set,adb emu killno-op) and the fix confirmed against it.Notes
continue-on-error: true; nothing here gates a PR.