fix(debug-ios): make the CDP proxy pick a free port and survive reconnects

The port check bound localhost while the server binds every interface, and on macOS those do not conflict, so a taken port passed the check and the listen crashed the CLI. The check now tests all four addresses and the proxy moves on if the bind still fails.

The connection lock was released on the connection event, which never fires for a client that drops during the handshake, so every later client timed out until the CLI exited. A late close from a replaced client could also destroy the socket its successor had just received. Connections now run through an in-process queue, and each session only cleans up after itself; a replaced client is closed with a reason.

Also clears the port service's timers, whose number type check never matched in Node, which produced false port warnings.

Author: rigor789

lib/common/services/net-service.ts

@@ -48,35 +48,32 @@ export class Net implements INet {
 	}
 
 	public async isPortAvailable(port: number): Promise<boolean> {
-		return new Promise<boolean>((resolve, reject) => {
-			let isResolved = false;
-			const server = net.createServer();
+		// A listener on one address does not stop a bind on another (e.g. the
+		// wildcard next to the loopback), so each of them has to be free.
+		for (const host of ["127.0.0.1", "::1", "0.0.0.0", "::"]) {
+			if (!(await this.canListen(port, host))) {
+				return false;
+			}
+		}
 
-			server.on("error", (err: Error) => {
-				if (!isResolved) {
-					isResolved = true;
-					resolve(false);
-				}
-			});
+		return true;
+	}
 
-			server.once("close", () => {
-				if (!isResolved) {
-					// "close" will be emitted right after "error"
-					isResolved = true;
-					resolve(true);
-				}
-			});
+	private canListen(port: number, host: string): Promise<boolean> {
+		return new Promise<boolean>((resolve) => {
+			const server = net.createServer();
 
-			server.on("listening", (err: Error) => {
-				if (err && !isResolved) {
-					isResolved = true;
-					resolve(true);
-				}
+			server.once("error", (err: NodeJS.ErrnoException) => {
+				// The host has no such address (e.g. IPv6 is disabled), so nothing
+				// can be listening on it either.
+				resolve(err.code === "EADDRNOTAVAIL" || err.code === "EAFNOSUPPORT");
+			});
 
-				server.close();
+			server.once("listening", () => {
+				server.close(() => resolve(true));
 			});
 
-			server.listen(port, "localhost");
+			server.listen(port, host);
 		});
 	}
 

lib/common/test/unit-tests/services/net-service.ts

@@ -1,5 +1,6 @@
 import { Net } from "../../../services/net-service";
 import { assert } from "chai";
+import * as net from "net";
 import { Yok } from "../../../yok";
 import { ErrorsStub, CommonLoggerStub } from "../stubs";
 import { EOL } from "os";
@@ -25,6 +26,39 @@ describe("net", () => {
 		return testInjector;
 	};
 
+	describe("isPortAvailable", () => {
+		const hosts = [undefined, "127.0.0.1", "::1", "0.0.0.0", "::"];
+
+		for (const host of hosts) {
+			it(`returns false for a port taken on ${host || "the default address"}`, async () => {
+				const service = createTestInjector("darwin").resolve<INet>(Net);
+				const server = net.createServer();
+				await new Promise<void>((resolve) =>
+					host
+						? server.listen(0, host, () => resolve())
+						: server.listen(0, () => resolve()),
+				);
+				const { port } = <net.AddressInfo>server.address();
+
+				try {
+					assert.isFalse(await service.isPortAvailable(port));
+				} finally {
+					await new Promise((resolve) => server.close(resolve));
+				}
+			});
+		}
+
+		it("returns true for a free port", async () => {
+			const service = createTestInjector("darwin").resolve<INet>(Net);
+			const server = net.createServer();
+			await new Promise<void>((resolve) => server.listen(0, () => resolve()));
+			const { port } = <net.AddressInfo>server.address();
+			await new Promise((resolve) => server.close(resolve));
+
+			assert.isTrue(await service.isPortAvailable(port));
+		});
+	});
+
 	describe("waitForPortToListen", () => {
 		let execCalledCount = 0;
 		beforeEach(() => {

lib/device-sockets/ios/app-debug-socket-proxy-factory.ts

@@ -8,6 +8,13 @@ import { IDictionary, IErrors, INet } from "../../common/declarations";
 import { injector } from "../../common/yok";
 import { ITempService } from "../../definitions/temp-service";
 
+const REPLACED_CLOSE_REASON = "Replaced by a newer debugger client";
+
+interface IDebugProxySession {
+	appDebugSocket: net.Socket;
+	webSocket?: ws;
+}
+
 export class AppDebugSocketProxyFactory
 	extends EventEmitter
 	implements IAppDebugSocketProxyFactory
@@ -18,7 +25,6 @@ export class AppDebugSocketProxyFactory
 	constructor(
 		private $logger: ILogger,
 		private $errors: IErrors,
-		private $lockService: ILockService,
 		private $options: IOptions,
 		private $tempService: ITempService,
 		private $net: INet,
@@ -132,7 +138,6 @@ export class AppDebugSocketProxyFactory
 		projectName: string,
 		projectDir: string,
 	): Promise<ws.Server> {
-		let clientConnectionLockRelease: () => void;
 		const cacheKey = `${device.deviceInfo.identifier}-${appId}`;
 		const existingServer = this.deviceWebServers[cacheKey];
 		if (existingServer) {
@@ -141,76 +146,83 @@ export class AppDebugSocketProxyFactory
 			);
 		}
 
-		// NOTE: We will try to provide command line options to select ports, at least on the localhost.
-		const localPort = await this.$net.getAvailablePortInRange(41000);
-
 		this.$logger.info(
 			"\nSetting up debugger proxy...\nPress Ctrl + C to terminate, or disconnect.\n",
 		);
 
 		// NB: When the inspector frontend connects we might not have connected to the inspector backend yet.
 		// That's why we use the verifyClient callback of the websocket server to stall the upgrade request until we connect.
-		// We store the socket that connects us to the device in the upgrade request object itself and later on retrieve it
-		// in the connection callback.
-
-		let currentAppSocket: net.Socket = null;
-		let currentWebSocket: ws = null;
-		const server = new ws.Server(<any>{
-			port: localPort,
-			verifyClient: async (
+		// We store the session in the upgrade request object itself and later on retrieve it in the connection callback.
+
+		// The runtime serves a single frontend, so the newest client takes over.
+		// Handlers of a replaced session must leave the device socket alone, as
+		// by then it belongs to the session that replaced it.
+		let currentSession: IDebugProxySession = null;
+		let pendingConnect: Promise<void> = Promise.resolve();
+
+		const connectToApp = async (): Promise<IDebugProxySession> => {
+			const previousSession = currentSession;
+			currentSession = null;
+			if (previousSession) {
+				previousSession.webSocket?.close(1000, REPLACED_CLOSE_REASON);
+				await device.destroyDebugSocket(appId);
+			}
+
+			const appDebugSocket = await device.getDebugSocket(
+				appId,
+				projectName,
+				projectDir,
+			);
+			appDebugSocket.on("error", (err: Error) => {
+				this.$logger.trace("Error on debugger deviceSocket", err);
+			});
+			this.$logger.info("Backend socket created.");
+
+			currentSession = { appDebugSocket };
+			return currentSession;
+		};
+
+		const server = await this.listenOnAvailablePort(41000, {
+			verifyClient: (
 				info: any,
 				callback: (res: boolean, code?: number, message?: string) => void,
 			) => {
-				let acceptHandshake = true;
-				clientConnectionLockRelease = null;
-
-				try {
-					clientConnectionLockRelease = await this.$lockService.lock(
-						`debug-connection-${device.deviceInfo.identifier}-${appId}.lock`,
-					);
-
-					this.$logger.info("Frontend client connected.");
-					let appDebugSocket;
-					if (currentAppSocket) {
-						currentAppSocket.removeAllListeners();
-						currentAppSocket = null;
-						if (currentWebSocket) {
-							currentWebSocket.removeAllListeners();
-							currentWebSocket.close();
-							currentWebSocket = null;
-						}
-						await device.destroyDebugSocket(appId);
+				this.$logger.info("Frontend client connected.");
+
+				// Not released on "connection": that never fires for a client that
+				// drops during the handshake.
+				pendingConnect = pendingConnect.then(async () => {
+					try {
+						info.req["__session"] = await connectToApp();
+						callback(true);
+					} catch (err) {
+						err.deviceIdentifier = device.deviceInfo.identifier;
+						this.$logger.trace(err);
+						this.emit(CONNECTION_ERROR_EVENT_NAME, err);
+						this.$logger.warn(
+							`Cannot connect to device socket. The error message is '${err.message}'.`,
+						);
+						callback(false);
 					}
-					appDebugSocket = await device.getDebugSocket(
-						appId,
-						projectName,
-						projectDir,
-					);
-					currentAppSocket = appDebugSocket;
-					this.$logger.info("Backend socket created.");
-					info.req["__deviceSocket"] = appDebugSocket;
-				} catch (err) {
-					if (clientConnectionLockRelease) {
-						clientConnectionLockRelease();
-					}
-					err.deviceIdentifier = device.deviceInfo.identifier;
-					this.$logger.trace(err);
-					this.emit(CONNECTION_ERROR_EVENT_NAME, err);
-					acceptHandshake = false;
-					this.$logger.warn(
-						`Cannot connect to device socket. The error message is '${err.message}'.`,
-					);
-				}
-
-				callback(acceptHandshake);
+				});
 			},
 		});
 		this.deviceWebServers[cacheKey] = server;
 		server.on("connection", (webSocket, req) => {
-			currentWebSocket = webSocket;
 			const encoding = "utf16le";
 
-			const appDebugSocket: net.Socket = (<any>req)["__deviceSocket"];
+			webSocket.on("error", (err) => {
+				this.$logger.trace("Error on debugger websocket", err);
+			});
+
+			const session: IDebugProxySession = (<any>req)["__session"];
+			if (session !== currentSession || session.appDebugSocket.destroyed) {
+				webSocket.close(1000, REPLACED_CLOSE_REASON);
+				return;
+			}
+
+			session.webSocket = webSocket;
+			const appDebugSocket = session.appDebugSocket;
 			const packets = new MessageUnpackStream();
 			appDebugSocket.pipe(packets);
 
@@ -230,14 +242,6 @@ export class AppDebugSocketProxyFactory
 				}
 			});
 
-			webSocket.on("error", (err) => {
-				this.$logger.trace("Error on debugger websocket", err);
-			});
-
-			appDebugSocket.on("error", (err) => {
-				this.$logger.trace("Error on debugger deviceSocket", err);
-			});
-
 			webSocket.on("message", (message) => {
 				const msg = message.toString();
 				if (process.env.DEBUG_DEVTOOLS_SOCKETS) {
@@ -253,28 +257,65 @@ export class AppDebugSocketProxyFactory
 			});
 
 			appDebugSocket.on("close", () => {
-				currentAppSocket = null;
 				this.$logger.trace("Backend socket closed!");
+				if (currentSession === session) {
+					currentSession = null;
+				}
 				webSocket.close();
 			});
 
 			webSocket.on("close", async () => {
-				currentWebSocket = null;
 				this.$logger.trace("Frontend socket closed!");
 				appDebugSocket.unpipe(packets);
 				packets.destroy();
+				if (currentSession !== session) {
+					return;
+				}
+
+				currentSession = null;
 				await device.destroyDebugSocket(appId);
 				if (!this.$options.watch) {
 					process.exit(0);
 				}
 			});
-
-			clientConnectionLockRelease();
 		});
 
 		return server;
 	}
 
+	private async listenOnAvailablePort(
+		startPort: number,
+		options: ws.ServerOptions,
+	): Promise<ws.Server> {
+		// The port can still get taken after the check, by another CLI for example.
+		let port = await this.$net.getAvailablePortInRange(startPort);
+		while (true) {
+			const result = await new Promise<ws.Server | NodeJS.ErrnoException>(
+				(resolve) => {
+					const server = new ws.Server({ ...options, port });
+					server.once("listening", () => {
+						server.removeAllListeners("error");
+						server.on("error", (err: Error) => {
+							this.$logger.trace("Error on debugger web socket server", err);
+						});
+						resolve(server);
+					});
+					server.once("error", resolve);
+				},
+			);
+
+			if (result instanceof ws.Server) {
+				return result;
+			}
+
+			if (result.code !== "EADDRINUSE") {
+				throw result;
+			}
+
+			port = await this.$net.getAvailablePortInRange(port + 1);
+		}
+	}
+
 	public removeAllProxies() {
 		let deviceId;
 		for (deviceId in this.deviceWebServers) {

lib/services/ios-debugger-port-service.ts

@@ -143,8 +143,8 @@ export class IOSDebuggerPortService implements IIOSDebuggerPortService {
 	private clearTimeout(data: IIOSDebuggerPortData): void {
 		const storedData =
 			this.mapDebuggerPortData[`${data.deviceId}${data.appId}`];
-		if (storedData && typeof storedData.timer === "number") {
-			clearTimeout(storedData.timer);
+		if (storedData && storedData.timer) {
+			clearTimeout(<any>storedData.timer);
 		}
 	}
 }