Skip to content

Commit 268b302

Browse files
vzaidmanmeta-codesync[bot]
authored andcommitted
Clear pages polling interval on device recreate (#58928)
Summary: Pull Request resolved: #58928 `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 fbshipit-source-id: 3224453d2450dd2f1ab74283b7109d611d5a8e14
1 parent 618b844 commit 268b302

2 files changed

Lines changed: 50 additions & 0 deletions

File tree

‎packages/dev-middleware/src/__tests__/InspectorProxyDeviceHandoff-test.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ const PAGE_DEFAULTS = {
3434
vm: 'bar-vm',
3535
};
3636

37+
// Must match PAGES_POLLING_INTERVAL in `Device.js`.
38+
const PAGES_POLLING_INTERVAL_MS = 1000;
39+
3740
describe('inspector-proxy device socket handoff', () => {
3841
const serverRef = withServerForEachTest({
3942
logger: undefined,
@@ -254,6 +257,50 @@ describe('inspector-proxy device socket handoff', () => {
254257
}
255258
});
256259

260+
test('device ID collision clears the previous pages polling interval', async () => {
261+
const setIntervalSpy = jest.spyOn(globalThis, 'setInterval');
262+
const clearIntervalSpy = jest.spyOn(globalThis, 'clearInterval');
263+
let device1, device2;
264+
try {
265+
({device: device1} = await connectDevice(
266+
'/inspector/device?device=device&name=foo&app=bar',
267+
[
268+
{
269+
...PAGE_DEFAULTS,
270+
vm: 'bar-vm',
271+
},
272+
],
273+
));
274+
275+
const pollingIntervals = setIntervalSpy.mock.calls.flatMap(
276+
(call, index) =>
277+
call[1] === PAGES_POLLING_INTERVAL_MS
278+
? [setIntervalSpy.mock.results[index].value]
279+
: [],
280+
);
281+
expect(pollingIntervals).toHaveLength(1);
282+
283+
({device: device2} = await connectDevice(
284+
'/inspector/device?device=device&name=foo&app=bar',
285+
[
286+
{
287+
...PAGE_DEFAULTS,
288+
vm: 'bar-vm-updated',
289+
},
290+
],
291+
));
292+
293+
for (const intervalId of pollingIntervals) {
294+
expect(clearIntervalSpy).toBeCalledWith(intervalId);
295+
}
296+
} finally {
297+
device1?.close();
298+
device2?.close();
299+
setIntervalSpy.mockRestore();
300+
clearIntervalSpy.mockRestore();
301+
}
302+
});
303+
257304
test.each([
258305
['app', 'name'],
259306
['name', 'app'],

‎packages/dev-middleware/src/inspector-proxy/Device.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,9 @@ export default class Device {
301301
);
302302
}
303303

304+
// Stop polling the previous connection before its state is replaced.
305+
clearInterval(this.#pagesPollingIntervalId);
306+
304307
this.#dangerouslyConstruct(deviceOptions);
305308

306309
// Restore all debugger connections, not just the first one

0 commit comments

Comments
 (0)