diff --git a/docs/api-37-emulator-crash.md b/docs/api-37-emulator-crash.md index cacc409..0cc6ff0 100644 --- a/docs/api-37-emulator-crash.md +++ b/docs/api-37-emulator-crash.md @@ -101,10 +101,14 @@ host always has, so the assert fires whenever that path is taken. Two facts pin down what "always" means: -- **The capability is negotiated regardless of renderer.** The abort fires under ANGLE too - (r03/r05/r06), just far less often. `ANDROID_EMU_read_color_buffer_dma` lives in - `emulator/lib64/libgfxstream_backend.so`, which every `-gpu` mode goes through — it is the - only file in the whole SDK that contains the string. +- **The capability is negotiated regardless of renderer.** The evidence is the aborts + themselves: the assertion that fires is `!hasReadColorBufferDma`, and it fires under ANGLE + (r03/r05/r06) as well as under the host translator — just far less often. That is a direct + observation of the guest having negotiated DMA readback under both, and it stands alone. + (Supporting only, and weaker than it first looks: `ANDROID_EMU_read_color_buffer_dma` appears + in exactly one file in the SDK, `emulator/lib64/libgfxstream_backend.so`, which every `-gpu` + mode goes through. A string search establishes where the extension is implemented, not that + it is negotiated on every path.) - **It is not gated by any feature flag the emulator exposes.** See the ruled-out list below. So the renderer does not decide whether the guest *believes* DMA readback exists. It decides how @@ -285,8 +289,11 @@ Package com.android.systemui new state: disabled-user window Service window: found ``` -**Zero in 180 s, against 10–11 per 150 s.** That is the confirmation that region sampling is the -sole trigger, and it is worth recording even by someone who never wants the workaround. +**Zero in 180 s, against 10–11 per 150 s.** That is the strongest evidence that region sampling +is the dominant trigger, and it is worth recording even by someone who never wants the workaround. +It does not establish it as the *only* trigger: the paragraph below has an abort surviving the +disable, and nothing measured here says whether that residue is a second caller of the readback +path or a disable that did not fully take. Do not read that as "the crashes stop", though, because the harness path does not reproduce a clean zero. Its own post-disable check on the run recorded below printed diff --git a/docs/local-emulator.md b/docs/local-emulator.md index e2eac49..57b2e0e 100644 --- a/docs/local-emulator.md +++ b/docs/local-emulator.md @@ -204,6 +204,16 @@ spelling of `37.0`. Setting `GPU_MODE` forces one renderer on every level, which you want when measuring a mode; leaving it unset lets `gpu_for_api` pick, which is what you want when running the suite — 33–36 need `host` and 37 must not have it. +**A bare `run-e2e.sh` exits 1, and that is the design.** API 37 is in the default list +deliberately — leaving it out is what left the level unlooked-at for as long as it was — and +it is permanently two failures short of green: `Media3EngineTest` cannot drive the emulator's +`c2.goldfish.h264.decoder` on those images, which +[`api-37-emulator-crash.md`](api-37-emulator-crash.md) pins on the image and not on this app +(API 35 under the same renderer is green). The summary row names the two expected failures so +that a third is visibly new, and the script repeats the point on the way out. Anything that +treats a non-zero exit as breakage — a wrapper, a hook, a habit — should name the levels it +wants: `run-e2e.sh 33 34 35 36` is the sweep that can be green. + `swangle_indirect` is the fallback worth knowing about. It is entirely software, so it does not depend on reaching the session's GPU — useful over plain SSH, where `-gpu host` has not been tested and may not find a device. It is also the closest local analogue to diff --git a/tools/local-emulator/run-e2e.sh b/tools/local-emulator/run-e2e.sh index a512d93..811e809 100755 --- a/tools/local-emulator/run-e2e.sh +++ b/tools/local-emulator/run-e2e.sh @@ -12,6 +12,15 @@ # BOOT_TIMEOUT=300 seconds to wait for sys.boot_completed # KEEP_AVD=1 do not delete an AVD this script created # +# EXIT CODE: 0 only if every level was green; 1 if any level failed, wedged or could not be +# set up; 2 if it refused to start at all. **A bare `run-e2e.sh` therefore exits 1 by design.** +# API 37 is in the default list on purpose -- leaving it out is what left the level unlooked-at +# for as long as it was -- and it is permanently two failures short of green, on the emulator's +# own c2.goldfish.h264.decoder rather than on anything this app does. The summary names the two, +# so a third is visibly new, and the last line printed says the same thing. Anything that reads a +# non-zero exit as breakage should name the levels it wants: `run-e2e.sh 33 34 35 36` is the +# sweep that can be green. docs/api-37-emulator-crash.md has the measurements. +# # WHY THIS EXISTS, AND WHAT IT DELIBERATELY DOES NOT DO # # It is a *launcher*, not a second test harness. The diagnostics -- the FAILED-vs-WEDGED @@ -415,7 +424,10 @@ delete_created_avds() { local avd [ "${KEEP_AVD:-0}" = "1" ] && return 0 for avd in ${CREATED_AVDS[@]+"${CREATED_AVDS[@]}"}; do - avdmanager delete avd -n "$avd" > /dev/null 2>&1 + # A SIGKILLed emulator does not get to remove its own lock files, and avdmanager can + # refuse over them. Staying silent there would leak the very thing this exists to clean. + avdmanager delete avd -n "$avd" > /dev/null 2>&1 \ + || echo " WARNING: could not delete AVD $avd -- 'avdmanager delete avd -n $avd' by hand" done CREATED_AVDS=() } @@ -440,6 +452,10 @@ cleanup() { # 6 s, not 30: Ctrl-C has already reached the emulator through the foreground process group, # so this is only waiting for it to finish writing, and `kill -9` follows regardless. The # EXIT trap is disarmed before exiting so the status below is the one that survives. +# +# bash runs a trap only between commands, so this starts when whatever was in the foreground +# returns -- which for Ctrl-C is immediately, because the same interrupt reached that command +# too. `kill -INT` aimed at this script alone waits for the foreground command to finish. on_signal() { echo echo "interrupted (SIG$1) -- stopping the emulator and removing the AVDs this run created" @@ -492,6 +508,16 @@ PY SUMMARY=() overall=0 +# Whether the red exit is the expected one depends on which level produced it, and only the +# loop knows that -- so it is recorded where `overall` is set rather than guessed from the +# summary afterwards. A note at the end claiming a genuine API 34 failure was "by design" +# would be the same defect it is there to prevent, one layer up. +NON37_RED=0 +mark_red() { + overall=1 + case "$1" in 37 | 37.*) ;; *) NON37_RED=1 ;; esac +} + for api in "${APIS[@]}"; do avd="$(avd_for_api "$api")" gpu="$(gpu_for_api "$api")" @@ -503,7 +529,7 @@ for api in "${APIS[@]}"; do if ! ensure_avd "$api" "$avd"; then SUMMARY+=("API $api: AVD SETUP FAILED") - overall=1 + mark_red "$api" continue fi @@ -511,7 +537,7 @@ for api in "${APIS[@]}"; do host_forensics "$started" guest_forensics "$api" SUMMARY+=("API $api: BOOT FAILED") - overall=1 + mark_red "$api" stop_emulator continue fi @@ -545,7 +571,7 @@ for api in "${APIS[@]}"; do esac if [ "$rc" -ne 0 ]; then line="$line [gradle exit $rc]" - overall=1 + mark_red "$api" host_forensics "$started" fi SUMMARY+=("$line") @@ -558,4 +584,16 @@ echo echo "===================== LOCAL E2E SUMMARY ======================" printf '%s\n' ${SUMMARY[@]+"${SUMMARY[@]}"} echo "==============================================================" + +# An unexplained red exit trains people to stop reading exit codes, and this one is expected +# whenever API 37 is in the sweep -- which the default list makes the common case. Said here +# rather than only in the docs, because this is where it is actually read. Only when 37.x is +# the ONLY thing that went red: a note calling a real failure elsewhere "by design" would be +# worse than no note at all. +if [ "$overall" -ne 0 ] && [ "$NON37_RED" -eq 0 ]; then + echo "note: the only level that went red is API 37, which exits non-zero by design -- it is" + echo " permanently 2 failures short of green. Confirm its row above shows exactly those" + echo " two and nothing else; docs/api-37-emulator-crash.md says why they are the image." +fi + exit "$overall"