Skip to content

Recover Flutter Chrome tests stalled on a lost Browser.close reply - #1832

Merged
leehack merged 5 commits into
mainfrom
fix/chrome-shutdown-hang
Nov 9, 2026
Merged

leehack merged 5 commits into
mainfrom
fix/chrome-shutdown-hang

Conversation

@leehack

@leehack leehack commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Cause (refs #1702): Flutter 3.47.1 flutter test --platform chrome closes Chrome by sending DevTools Browser.close and awaiting the reply with no timeout (flutter_tools/lib/src/web/chrome.dart:571-572); webkit_inspection_protocol never fails pending commands when the socket closes. If Chrome exits before the reply arrives, the CLI waits forever after printing results. The Model Store moved to dart test -p chrome in #1773; Web Chat Contract was the last CI path still exposed, up to its 40-minute job limit.
  • Fix: Web Chat Contract now runs its Chrome tests under tool/ci/supervise_chrome_test.py with flutter test --file-reporter json:..., a 10-minute deadline and a diagnostics artifact. New --reporter-file / --post-done-seconds (default 30) supervisor options: when the CLI is still running 30 s after the JSON reporter's done event and no browser process remains, the supervisor stops the owned processes, logs a ::warning::, records browser_close_stall in result.json, and exits with package:test's own verdict (0 only for success: true with at least one visible test; 1 for success: false). Every other stall (browser alive, no done, success: null, zero tests) still fails at the deadline (124). A real CLI exit always wins over the reporter, and a pre-existing reporter file refuses to start.
  • Why not the printed summary: the verdict comes from package:test's structured done event, which test_core emits before runner.close() closes Chrome. The CLI's own exit code after a forced stop is unusable: Flutter exits 0 on SIGTERM even with a failing test (fault run below).

Production-readiness scope

  • User-facing scope: None. CI tooling only.
  • Supported platforms/paths: Linux Web Chat Contract job; the supervisor stays Linux-only (/proc, pidfd).
  • Unsupported or intentionally unavailable paths: Without --reporter-file, supervisor behavior is unchanged (Model Store lane).
  • Out of scope / follow-ups: The upstream fix (bound the Browser.close wait in flutter_tools, or fail pending commands on close in webkit_inspection_protocol) has no upstream issue yet. The loading-stage stall (run 41709296800, Chrome alive, no results) remains unattributed; on this path it is now bounded at 10 minutes with process diagnostics instead of 40.

Completeness checklist

  • Declared scope is fully implemented, or explicitly reduced above.
  • Unsupported platform/option combinations fail loudly: unknown stalls still fail at the deadline; a stale reporter file exits 125 before starting the command.
  • Docs updated: doc/ci_selection.md "Flutter Chrome test shutdown". Changelog N/A: nothing users can observe.
  • Regression coverage: supervisor fixtures for the stall (pass and fail verdicts), browser-still-alive, unsettled/zero-test results, real exit precedence and stale file; a workflow pin test that fails against base ci.yml.
  • Validation coverage registry: N/A, no public capability change.
  • Security/privacy: new result.json/event fields are booleans and counts; no paths, argv or environment recorded.
  • Follow-ups: listed above.

PR type guidance

  • Bugfix PR: root cause above. The upstream race needs real Chrome plus a DevTools fault proxy, so the durable tests are process fixtures; the fault-injection reproduction is below. Platform: Linux CI.

Test Plan

  • dart run tool/prepare_workspace.dart: tree clean.
  • dart format --output=none --set-exit-if-changed .: no Dart changes.
  • dart analyze: N/A, no Dart changes.
  • dart test -p vm test/unit/tooling/: 1590 pass, 6 fail (windows_registry_qualification_test.dart, needs pwsh; unrelated, same as on main).
  • dart test -p chrome: N/A, no library or web code change.
  • coverage-lib: N/A, no lib/ change.
  • Other: python3 -m unittest discover -s test/ci -p 'test_*.py' passes on macOS (Linux-only cases skipped); test_supervise_chrome_test.py 28/28 passes on Linux (Debian trixie arm64 container), 3 consecutive runs.

Fault injection: before / after

Debian trixie arm64 container, Chromium 156.0.8078.79, Flutter 3.47.1, flutter create widget test, flutter test --platform chrome --file-reporter json:... under the supervisor. A CHROME_EXECUTABLE shim relays the DevTools port (stripping permessage-deflate so commands are visible); drop discards Chrome's bytes after Browser.close, pass only relays.

Supervisor Mode / test Runs Exit Wall Notes
main drop, passing test 1 124 (deadline 90 s) 90 s All tests passed!, Chrome exit 0, only dart:flutter_to + dart:frontend_s left in futex_do_wait: the hosted job 125206388946 signature. Reporter file already held {"success":true,"type":"done"}.
this PR drop, passing test 5 5/5 exit 0 38.8-39.7 s browser_close_stall 30.0 s after reporter_done; no surviving process
this PR drop, failing test 3 3/3 exit 1 39.3-39.5 s CLI exit after SIGTERM was 0; the reporter verdict kept the failure
this PR pass, passing test 11 11/11 exit 0 9.0-9.9 s Browser.close reply seen every run; no stall recovery, no forced cleanup

Hosted history (ci.yml, 400 runs, 2026-10-30 to 2026-11-08)

Job / step Runs Hung Median Max
Web Chat Contract / Run chat app tests 331 0 139 s 370 s
Model Store Chrome step (supervised, dart test) 156 0 23.5 s 74 s
Model Store Analyze and test (pre-#1773 flutter test) — 2 (41424032145 shutdown, 41709296800 loading) — job limit

The race is rare (0/331 on Web Chat), so reruns cannot show a frequency change; the fault table is the before/after evidence, and the hosted CI runs confirm the supervised step adds no stall on the normal path.

Hosted supervised step on this head

Attempt Chrome step Supervisor exit CLI exit Forced cleanup browser_close_stall Tests done to CLI exit
1 70 s 0 0 no no 133 0.20 s
2 68 s 0 0 no no 133 0.25 s
3 68 s 0 0 no no 133 n/a (artifact name reused)
4 70 s 0 0 no no 133 0.16 s

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
examples-tests Web Chat Contract chat app VM + Chrome tests hosted Linux CI, model-free PASS 4 attempts of run 42535897645 below
high-risk-exact-head-independent-qa independent blocking-only review of exact head/current base ec2ffba46 / b78008cf3 PASS Full audit at 6fb2d1fcd (no blocking findings); exact-head re-audit at ec2ffba46: conflict resolution keeps only both lines, later merges' patch-ids equal main's #1835/#1825, PR diff patch-id identical to the audited resolution, Linux container 29/29, Dart tooling +44, Web Chat Contract run 42546893494 passed with no recovery triggered
other high-risk rows release metadata, structured output, LiteRT-LM N/A N/A Only CI supervisor, one workflow step and their tests/docs change

Review Notes

  • Independent review status: accepted at exact head ec2ffba464941de4879bf6159ff806382441e9b0 against main b78008cf3. The maintainer approved deriving pass/fail from package:test's structured done event; #1702 stays open to remove the recovery once flutter/flutter#196191 is fixed.
  • Merge with main: 6fb2d1fcd conflicted with main 5300a6c03 (#1828) in tool/ci/supervise_chrome_test.py; resolved by a merge commit keeping reserve_standard_descriptors() first, then the stale-reporter check. Merge head ec2cf3400d43c84d37be3599f776b24169c3984a (tree aa3fed8ac59a, matching the auditor's resolution). Local on the new head: test_supervise_chrome_test.py passes (Linux cases skipped on macOS), format clean, analyze clean, chrome_test_supervisor_test.dart + ci_workflow_selection_test.dart 44 pass.
  • Second merge: current main 9423d6737 (#1835) merged so the audit base is an ancestor of the head. New head 6617cdd72aecf4fe13b44a21834d1d7213a33221; the diff against ec2cf3400 touches only tool/native/sync_native_release_pins.py and tool/native/test_sync_native_release_pins.py. Supervisor Python tests pass. CI on this head: 33 pass, 8 skipped, 0 fail.
  • Third merge: main b78008cf3 (#1825); new head ec2ffba464941de4879bf6159ff806382441e9b0, bringing in only Skip delete-pending model cache entries on Windows instead of failing removal #1825's files. CI on this head: 33 pass, 8 skipped, 0 fail.
  • Superseded evidence: CI and hosted attempts on 6fb2d1fcd, ec2cf3400 and 6617cdd72 no longer count; current evidence is on ec2ffba46.
  • Non-blocking follow-ups (tracked in #1837): (1) exit-code precedence (recovered_exit vs leftover-process/EOF to 125) is not pinned by tests; (2) the workflow test does not pin the 30 s --post-done-seconds default; (3) a leftover reporter file makes the supervisor exit 125 silently, with no message or diagnostics directory.
  • Earlier CI status / head SHA (superseded): all checks passed at 6fb2d1fcdf42a4fcbbdfa96073a48fdd73a61ef9 (33 pass, 8 skipped; CI run 42535897645). The first head failed one new fixture on Ubuntu (uutils sleep refuses a renamed binary); the fake browser is now a script.

High-risk regression review

  • Classification: high-risk (regressionPolicy surface: .github/workflows/ci.yml)
  • Implementation task: Claude session (this PR)
  • Independent blocking QA task: fresh independent agent sessions (full audit at 6fb2d1fcd, exact-head re-audit at ec2ffba46): PASS, no blocking findings
  • Exact head / current base: ec2ffba464941de4879bf6159ff806382441e9b0 / b78008cf31fbf6c967e1cc5bf2c557764c2072d3 (earlier heads 6fb2d1fcd, ec2cf3400, 6617cdd72)
  • Readiness evaluator: tool/testing/high_risk_readiness.dart exit 2 (unverifiedPrerequisites / externalPrerequisitesUnavailable), repository-local evidence consistent
  • Production-branch deletion, bypass, or miswire proof: WorkflowTest.test_web_chat_chrome_tests_take_only_their_own_reporter_result fails against base ci.yml; Linux supervisor mutants: dropping the browser-alive check, passing any done event, or allowing a stale reporter file each fail a fixture. Removing the post-done wait (now - done_at >= post_done) is not reliably killed: it failed only 2 of 6 runs, by a race in test_reporter_result_never_replaces_a_real_cli_exit; no test pins that wait deterministically.
  • Affected-family real-model/artifact evidence: N/A, no model or artifact path.
  • Explicit unavailable-family or other N/A evidence: N/A
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

flutter test --platform chrome awaits Chrome's DevTools Browser.close reply
with no timeout and hangs after reporting results when Chrome exits first.
Web Chat Contract now runs its Chrome tests under the supervisor, which
takes the JSON file reporter's result once the CLI outlives its browser by
30 seconds after the done event, and fails any other stall at a 10-minute
deadline.

Refs #1702
Ubuntu runners ship uutils coreutils, whose sleep refuses to run under
another name.
@leehack
leehack marked this pull request as ready for review November 9, 2026 02:08
@leehack
leehack merged commit ec4bf4f into main Nov 9, 2026
41 checks passed
@leehack
leehack deleted the fix/chrome-shutdown-hang branch November 9, 2026 02:08
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Chat app preview removed for leehack/llamadart-chat-pr-1832.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant