Skip to content

fix(android): reject pending callbacks on device close#40722

Merged
pavelfeldman merged 2 commits intomicrosoft:mainfrom
SebTardif:fix/android-callback-leak
May 8, 2026
Merged

fix(android): reject pending callbacks on device close#40722
pavelfeldman merged 2 commits intomicrosoft:mainfrom
SebTardif:fix/android-callback-leak

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

Summary

  • Reject all pending _callbacks when AndroidDevice._close() is called, preventing in-flight _send() calls from hanging forever
  • Add a synchronous _isClosed guard after the await in _send() to prevent a race condition where _close() clears the callback map during the microtask yield

Problem

AndroidDevice._close() tears down the driver, backend, and browser connections, but never rejects the pending entries in _callbacks. Any in-flight _send() call that is awaiting a response when the device closes will hang indefinitely because its promise is never resolved or rejected.

All four browser protocol sessions (CR, FF, WK, Bidi) already handle this correctly:

// crConnection.ts:187-197
dispose() {
  this._closed = true;
  for (const callback of this._callbacks.values()) {
    callback.error.setMessage('Internal server error, session closed.');
    callback.reject(callback.error);
  }
  this._callbacks.clear();
}

The Android device uses the same callback map pattern (_callbacks: Map<number, { fulfill, reject }>) but was missing the cleanup loop.

Additionally, _send() has a race condition: it awaits this._driver() before checking _isClosed and adding to the callback map. A concurrent _close() can clear the map during that yield, leaving the new callback orphaned.

Related issues

  • #21510: Android script gets stuck on context.newPage() / launchBrowser()
  • #38424: webview.page() hangs forever on hidden/detached WebViews
  • #35104: Android server stops unexpectedly before starting test
  • #21049: launchBrowser() stuck on Oppo/Xiaomi devices
  • #31474: Cancel inflight promises on page/context close

These issues all involve Android operations hanging or becoming unresponsive. While this fix does not address the root cause of each one (often device-specific firmware issues), it ensures that when the device does close, all pending operations fail cleanly instead of hanging the test process.

Comparable fix in Appium: appium/appium#6562 (hanging after clicking button that closes Android WebView, caused by pending callbacks not cleaned up).

Fix

  1. In _close(): iterate _callbacks, reject each with Error('Device closed'), then clear the map. Placed before driver/backend teardown so callbacks are rejected while the transport is still valid.
  2. In _send(): add this._isClosed check after await this._driver() to close the race window.

Changes

  • packages/playwright-core/src/server/android/android.ts: 4 lines added

SebTardif added 2 commits May 7, 2026 20:46
AndroidDevice._close() did not reject pending _callbacks entries.
Any in-flight _send() call would hang forever if the device was
closed before the response arrived.

Add callback rejection loop and clear the map, matching the pattern
used by CRSession.dispose(), FFSession.dispose(),
WKSession.dispose(), and BidiSession.dispose().
Address race condition: _send() has an await (this._driver()) between
the implicit closed check and _callbacks.set(). A concurrent _close()
can clear the map during that yield, leaving the new callback
unresolvable. Add a synchronous _isClosed check after the await.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

Test results for "MCP"

5 failed
❌ [firefox] › mcp/annotate.spec.ts:137 › should abort MCP annotation when last screenshot is removed @mcp-windows-latest-firefox
❌ [firefox] › mcp/tracing.spec.ts:54 › check that trace is saved with browser_start_tracing (no output dir) @mcp-windows-latest-firefox
❌ [msedge] › mcp/cli-storage.spec.ts:61 › state-load restores storage state from file @mcp-windows-latest-msedge
❌ [msedge] › mcp/cli-storage.spec.ts:100 › state-save and state-load roundtrip @mcp-windows-latest-msedge
❌ [msedge] › mcp/test-run.spec.ts:126 › test_run should stop when aborted @mcp-windows-latest-msedge

7026 passed, 1068 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

Test results for "tests 1"

4 flaky ⚠️ [installation tests] › playwright-test-package-managers.spec.ts:38 › npm: @playwright/test + playwright-core should work `@package-installations-windows-latest`
⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/video.spec.ts:275 › screencast › should capture navigation `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@chromium-ubuntu-22.04-node22`

41692 passed, 850 skipped


Merge workflow run.

@pavelfeldman pavelfeldman merged commit 31b3163 into microsoft:main May 8, 2026
43 of 45 checks passed
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.

2 participants