Skip to content

Fix stale inspector connections on Android - #59012

Open
vonovak wants to merge 2 commits into
react:mainfrom
vonovak:stale-inspector-connection
Open

vonovak wants to merge 2 commits into
react:mainfrom
vonovak:stale-inspector-connection

Conversation

@vonovak

@vonovak vonovak commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Stacked on #58971. Review only the last commit.

Summary:

On Android, after an app switches to another Metro server, the debugger stays attached through the old one. The inspector connection lives across reloads, and DevServerHelper.openInspectorConnection did nothing when a connection already existed, so it never moved to the new host. Open and close also ran in separate AsyncTasks, with the same ordering race that #58971 fixes for the packager connection.

Changes:

  • Open and close the inspector connection on the same single-threaded executor as the packager connection. An open for a new host replaces the old connection.
  • inspectorPackagerConnection is volatile, because disableDebugger() reads it from other threads.
  • DevServerHelper no longer uses AsyncTask, so the import, the related comments and @SuppressLint("StaticFieldLeak") are gone. Lint reports no StaticFieldLeak issue without it.
  • Tests use a small factory field to replace the native CxxInspectorPackagerConnection, which can't run under Robolectric.

Possible follow-up: Change Bundle Location could also close the inspector connection before reloading, as it does for the packager connection, to cover a reload whose bundle fails to load.

Changelog:

[ANDROID] [FIXED] - Fix the debugger staying connected through the previous Metro server after switching to another one

Test Plan:

New tests in DevServerHelperTest pass, and so do the existing related tests.

On an emulator running RNTester with Metro on ports 8082 and 8083, I switched from 8082 to 8083 with Change Bundle Location. Before this change, Metro 8082 still listed the app in /json/list minutes later and 8083 listed nothing. With this change, 8083 lists the app and 8082 drops it within seconds.

🤖 Generated with Claude Code

vonovak and others added 2 commits October 9, 2026 16:46
DevServerHelper opened and closed the packager connection in separate
AsyncTasks on THREAD_POOL_EXECUTOR. A close could run before a pending
open, and repeated opens could create two clients. Changing the bundle
location from the dev menu also kept the old connection. In both cases,
the old dev server could still reload the app.

- Run openPackagerConnection and closePackagerConnection as tasks on one
  single-threaded executor. Tasks run in call order, off the UI thread,
  and only that thread uses packagerClient. The executor names its
  thread ReactPackagerConnection and stops it after 30 seconds without
  work.
- Remember the host of the open client. If an open finds a client for a
  different host, close it and connect to the new host.
- Check clientId on the caller's thread. A failed init() still crashes
  the app, as before; packagerClient stays null, so a later open can
  retry.
- Change Bundle Location: close the packager connection before the
  reload, as setBundleSource does. Trim the input, then reject a host
  that OkHttp cannot parse in the connection URL and show a toast.
  Before, an invalid host was saved without a check.
- ReconnectingWebSocket: change close state under the socket's monitor,
  so no message handler starts after closeQuietly() returns. Close a
  socket whose handshake finishes after closeQuietly() instead of
  reporting it as connected.
- Add DevServerHelperTest, ReconnectingWebSocketTest and
  DevSupportHttpClientTest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DevServerHelper opened and closed the inspector connection in separate
AsyncTasks, and the connection lives across reloads. After the dev
server host changed, the next open saw an existing connection and did
nothing, so the debugger stayed attached through the old Metro server.

- Run openInspectorConnection and closeInspectorConnection on the same
  single-threaded executor as the packager connection.
- Remember the host of the open inspector connection. If an open finds a
  connection for a different host, close it and connect to the new host.
- Make inspectorPackagerConnection volatile, because disableDebugger()
  reads it from other threads.
- DevServerHelper no longer uses AsyncTask: remove the import, the
  related comments, and @SuppressLint("StaticFieldLeak"). Lint reports
  no StaticFieldLeak issue without it.
- Add inspector tests to DevServerHelperTest, with a test factory that
  avoids the native connection class.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 12, 2026
@vonovak
vonovak marked this pull request as ready for review October 12, 2026 22:02
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 12, 2026

This branch has not been deployed

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant