From 25027392bc9e6fba1b36a750683329d077c05283 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Wed, 7 Oct 2026 09:21:40 -0700 Subject: [PATCH] Clear pages polling interval on device recreate Summary: `Device#dangerouslyRecreateDevice` overwrote `#pagesPollingIntervalId` without clearing the previous interval. The old device socket's `close` handler could not clear it either, because its `socket === #deviceSocket` guard fails once reconstruct points `#deviceSocket` at the new socket. Every device ID collision therefore leaked a ref'd 1s `getPages` polling timer: the new device received duplicate polls, and the event loop was kept alive (Jest suites covering device handoff never exited on their own). Clear the previous polling interval before reconstructing. Clearing an already-cleared interval is a no-op, so the path where the old socket already closed is unaffected. With the leak fixed, test suites that exercise device handoff exit on their own, so their results can be relied on without force-exiting Jest. Changelog: [General][Fixed] - Fix the inspector proxy leaking a pages polling timer, and sending duplicate `getPages` requests, each time a device reconnects with the same device ID Differential Revision: D123883324 --- .../InspectorProxyDeviceHandoff-test.js | 47 +++++++++++++++++++ .../src/inspector-proxy/Device.js | 3 ++ 2 files changed, 50 insertions(+) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyDeviceHandoff-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyDeviceHandoff-test.js index 14fbb898e995..e3ad2ea29b56 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyDeviceHandoff-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyDeviceHandoff-test.js @@ -34,6 +34,9 @@ const PAGE_DEFAULTS = { vm: 'bar-vm', }; +// Must match PAGES_POLLING_INTERVAL in `Device.js`. +const PAGES_POLLING_INTERVAL_MS = 1000; + describe('inspector-proxy device socket handoff', () => { const serverRef = withServerForEachTest({ logger: undefined, @@ -254,6 +257,50 @@ describe('inspector-proxy device socket handoff', () => { } }); + test('device ID collision clears the previous pages polling interval', async () => { + const setIntervalSpy = jest.spyOn(globalThis, 'setInterval'); + const clearIntervalSpy = jest.spyOn(globalThis, 'clearInterval'); + let device1, device2; + try { + ({device: device1} = await connectDevice( + '/inspector/device?device=device&name=foo&app=bar', + [ + { + ...PAGE_DEFAULTS, + vm: 'bar-vm', + }, + ], + )); + + const pollingIntervals = setIntervalSpy.mock.calls.flatMap( + (call, index) => + call[1] === PAGES_POLLING_INTERVAL_MS + ? [setIntervalSpy.mock.results[index].value] + : [], + ); + expect(pollingIntervals).toHaveLength(1); + + ({device: device2} = await connectDevice( + '/inspector/device?device=device&name=foo&app=bar', + [ + { + ...PAGE_DEFAULTS, + vm: 'bar-vm-updated', + }, + ], + )); + + for (const intervalId of pollingIntervals) { + expect(clearIntervalSpy).toBeCalledWith(intervalId); + } + } finally { + device1?.close(); + device2?.close(); + setIntervalSpy.mockRestore(); + clearIntervalSpy.mockRestore(); + } + }); + test.each([ ['app', 'name'], ['name', 'app'], diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 5325a3996254..5ef0007e6b39 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -301,6 +301,9 @@ export default class Device { ); } + // Stop polling the previous connection before its state is replaced. + clearInterval(this.#pagesPollingIntervalId); + this.#dangerouslyConstruct(deviceOptions); // Restore all debugger connections, not just the first one