fix(firebase): withTimeout actually bounds a non-cancellable operation (2nd full-suite hang)#1316
Conversation
…ation Second full-suite hang polluter surfaced by #1305's fail-closed CI: RemoteFeatureFlagsTests.testFetchIfNeeded_doesNotCrash timed out at 120.000s. Chain: fetchIfNeeded → fetchAndActivate → FirebaseManager.fetchAndActivateRemoteConfig → withTimeout(10s) { remoteConfig.fetchAndActivate() }. withTimeout was supposed to bound the fetch, but it raced the operation against a sleep inside a withThrowingTaskGroup — and a task group RE-AWAITS its children at scope exit. When the timeout won and cancelAll() fired, Firebase's fetchAndActivate() ignored cancellation, so exiting the group blocked on the still-running child anyway. The 10s bound was a no-op for exactly the non-cancellable call it existed to bound: the caller hung (120s in CI; on a dead network the app's flag fetch would hang in production too). The existing guard test used Task.sleep, which HONORS cancellation, so it passed and masked the bug. - Rewrite withTimeout as a continuation-based first-wins race (via a file-scope ResumeOnceBox<T: Sendable> that resumes exactly once, lock-guarded). On timeout the operation task is ORPHANED, not awaited — matching the method's own documented "orphaned fetch completes harmlessly in the background while the caller proceeds" contract. Non-cancellable hangs can no longer block the caller. - Add testWithTimeout_boundsANonCancellableHangingOperation, which drives an op that never completes AND ignores cancellation (withCheckedContinuation never resumed) — the real Firebase case the Task.sleep test cannot reproduce. It hangs 120s on the old impl, returns in 0.22s on the new one. Verified: RemoteFeatureFlagsTests 21/21 green; testFetchIfNeeded_doesNotCrash 120s-hang -> 0.57s. Companion to the DeviceLogCollector hang fix (#1314) — same "test reaches an unbounded live external dependency" class, different dependency. **Scope:** FirebaseManager.withTimeout mechanism only + its non-cancellable guard test. Success-path and fast-return behavior unchanged; no flag/default changes. **Not done:** the broader non-deterministic flaky pool (handoff doc) — this removes the second high-severity hang source, not the whole pool. **Deferred:** propagating caller-cancellation into the orphaned tasks (unstructured by design here so a non-cancellable child cannot re-block the caller; the bounded 10s flag fetch does not need caller-initiated cancellation).
🏗️ CodeAtlas Ledger Analysis✅ All Checks Passed♿ Accessibility (via AccessLint)
✅ No accessibility issues detected 🧪 Test Coverage (via QAAtlas)
🏛️ Architecture Analysis
🔍 Reachability Analysis
✅ No dead code detected 📊 0 files analyzed | 📦 Download Full Report Powered by CodeAtlas Ledger |
🧪 Unit Test Results📊 View Full Interactive Report ✅ ALL TESTS PASSED7868 tests | 7733 passed | 0 failed | 133 skipped | ⏱️ 17m 53s | 📊 98.3% | 📈 45.5% coverage Tests by Class — 902 classes (click to expand)
📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 45.5% Total coverage incl. UI/lifecycle: 44.5% (17 files excluded from testable denominator — see
🔗 Interactive HTML Report | CI Run Details 📦 Downloadable Artifacts
|
Root cause
The second full-suite hang polluter surfaced by
#1305's fail-closed CI (companion to #1314):RemoteFeatureFlagsTests.testFetchIfNeeded_doesNotCrashtimed out at 120.000s.fetchIfNeeded → fetchAndActivate → FirebaseManager.fetchAndActivateRemoteConfig → withTimeout(10s) { remoteConfig.fetchAndActivate() }.withTimeoutis supposed to bound the fetch, but it raced the operation against a sleep inside awithThrowingTaskGroup— and a task group re-awaits its children at scope exit. When the timeout won andcancelAll()fired, Firebase'sfetchAndActivate()ignores cancellation, so exiting the group blocked on the still-running child anyway. The 10s bound was a no-op for exactly the non-cancellable call it exists to bound: the caller hung (120s in CI; on a dead network the app's flag fetch would hang in production too — a real production bug, not just a test issue).The existing guard test used
Task.sleep, which honors cancellation, so it passed and masked the bug.Fix
withTimeoutas a continuation-based first-wins race (file-scopeResumeOnceBox<T: Sendable>, lock-guarded resume-exactly-once). On timeout the operation task is orphaned, not awaited — matching the method's own documented "orphaned fetch completes harmlessly in the background while the caller proceeds" contract. A non-cancellable hang can no longer block the caller.testWithTimeout_boundsANonCancellableHangingOperation: drives an operation that never completes and ignores cancellation (withCheckedContinuationnever resumed) — the real Firebase case theTask.sleeptest can't reproduce. Hangs 120s on the old impl; returns in 0.22s on the new one.Verification
RemoteFeatureFlagsTests21/21 green;testFetchIfNeeded_doesNotCrash120s-hang → 0.57s; existingtestWithTimeout_boundsAHangingOperationstill passes.Companion to #1314 — same "test reaches an unbounded live external dependency" class (there: live OSLogStore; here: Firebase), which is why the handoff doc's reset-registry approach never converged on these. Both are needed for #1309 (PP-4831, a clean PR blocked purely by this pollution) to go green.