Merge pull request #4396 from NativeScript/tachev/unify-debug-ios

fix: ensure a single web client connected for debugging

Author: Dimitar Tachev

lib/common/definitions/mobile.d.ts

@@ -107,9 +107,9 @@ declare module Mobile {
 
 	interface IiOSDevice extends IDevice {
 		getDebugSocket(appId: string, projectName: string): Promise<any>;
-		destroyDebugSocket(appId: string): void;
+		destroyDebugSocket(appId: string): Promise<void>;
 		openDeviceLogStream(options?: IiOSLogStreamOptions): Promise<void>;
-		destroyAllSockets(): void;
+		destroyAllSockets(): Promise<void>;
 	}
 
 	interface IAndroidDevice extends IDevice {

lib/common/mobile/ios/device/ios-application-manager.ts

@@ -70,7 +70,7 @@ export class IOSApplicationManager extends ApplicationManagerBase {
 	public async stopApplication(appData: Mobile.IApplicationData): Promise<void> {
 		const { appId } = appData;
 
-		this.device.destroyDebugSocket(appId);
+		await this.device.destroyDebugSocket(appId);
 
 		const action = () => this.$iosDeviceOperations.stop([{ deviceId: this.device.deviceInfo.identifier, ddi: this.$options.ddi, appId }]);
 

lib/common/mobile/ios/ios-device-base.ts

@@ -24,8 +24,8 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice {
 			this.cachedSockets[appId] = await this.getDebugSocketCore(appId, projectName);
 
 			if (this.cachedSockets[appId]) {
-				this.cachedSockets[appId].on("close", () => {
-					this.destroyDebugSocket(appId);
+				this.cachedSockets[appId].on("close", async () => {
+					await this.destroyDebugSocket(appId);
 				});
 
 				this.$processService.attachToProcessExitSignals(this, () => this.destroyDebugSocket(appId));
@@ -51,22 +51,25 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice {
 		return port;
 	}
 
-	public destroyAllSockets() {
+	public async destroyAllSockets(): Promise<void> {
 		for (const appId in this.cachedSockets) {
-			this.destroySocketSafe(this.cachedSockets[appId]);
+			await this.destroySocketSafe(this.cachedSockets[appId]);
 		}
 
 		this.cachedSockets = {};
 	}
 
-	public destroyDebugSocket(appId: string) {
-		this.destroySocketSafe(this.cachedSockets[appId]);
+	public async destroyDebugSocket(appId: string): Promise<void> {
+		await this.destroySocketSafe(this.cachedSockets[appId]);
 		this.cachedSockets[appId] = null;
 	}
 
-	private destroySocketSafe(socket: net.Socket) {
+	private async destroySocketSafe(socket: net.Socket): Promise<void> {
 		if (socket && !socket.destroyed) {
-			socket.destroy();
+			return new Promise<void>((resolve, reject) => {
+				socket.on("close", resolve);
+				socket.destroy();
+			});
 		}
 	}
 

lib/common/mobile/ios/simulator/ios-simulator-application-manager.ts

@@ -69,7 +69,7 @@ export class IOSSimulatorApplicationManager extends ApplicationManagerBase {
 	public async stopApplication(appData: Mobile.IApplicationData): Promise<void> {
 		const { appId } = appData;
 
-		this.device.destroyDebugSocket(appId);
+		await this.device.destroyDebugSocket(appId);
 		await this.detachNativeDebugger(appId);
 
 		await this.iosSim.stopApplication(this.device.deviceInfo.identifier, appData.appId, appData.projectName);

lib/common/services/lock-service.ts

@@ -48,7 +48,7 @@ export class LockService implements ILockService {
 		}
 	}
 
-	private lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<string> {
+	public lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<string> {
 		const { filePath, fileOpts } = this.getLockFileSettings(lockFilePath, lockOpts);
 		this.currentlyLockedFiles.push(filePath);
 
@@ -62,7 +62,7 @@ export class LockService implements ILockService {
 		});
 	}
 
-	private unlock(lockFilePath?: string): void {
+	public unlock(lockFilePath?: string): void {
 		const { filePath } = this.getLockFileSettings(lockFilePath);
 		_.remove(this.currentlyLockedFiles, e => e === lockFilePath);
 		lockfile.unlockSync(filePath);

lib/common/test/unit-tests/stubs.ts

@@ -4,6 +4,13 @@ import * as util from "util";
 import { EventEmitter } from "events";
 
 export class LockServiceStub implements ILockService {
+	public async lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<string> {
+		return lockFilePath;
+	}
+
+	public unlock(lockFilePath?: string): void {
+	}
+
 	public async executeActionWithLock<T>(action: () => Promise<T>, lockFilePath?: string, lockOpts?: ILockOptions): Promise<T> {
 		const result = await action();
 		return result;

lib/definitions/lock-service.d.ts

@@ -15,5 +15,19 @@ declare global {
          */
         executeActionWithLock<T>(action: () => Promise<T>, lockFilePath?: string, lockOpts?: ILockOptions): Promise<T>
         // TODO: expose as decorator
+
+        /**
+         * Wait until the `unlock` method is called for the specified file
+         * @param {string} lockFilePath Path to lock file that has to be created. Defaults to `<profile dir>/lockfile.lock`
+         * @param {ILockOptions} lockOpts Options used for creating the lock file.
+         * @returns {Promise<T>}
+         */
+        lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<string>
+
+        /**
+         * Resolve the lock methods for the specified file
+         * @param {string} lockFilePath Path to lock file that has to be removed. Defaults to `<profile dir>/lockfile.lock`
+         */
+        unlock(lockFilePath?: string): void
     }
 }

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

@@ -11,6 +11,7 @@ export class AppDebugSocketProxyFactory extends EventEmitter implements IAppDebu
 
 	constructor(private $logger: ILogger,
 		private $errors: IErrors,
+		private $lockService: ILockService,
 		private $options: IOptions,
 		private $net: INet) {
 		super();
@@ -54,9 +55,9 @@ export class AppDebugSocketProxyFactory extends EventEmitter implements IAppDebu
 				}
 			});
 
-			frontendSocket.on("close", () => {
+			frontendSocket.on("close", async () => {
 				this.$logger.info("Frontend socket closed");
-				device.destroyDebugSocket(appId);
+				await device.destroyDebugSocket(appId);
 			});
 
 			appDebugSocket.on("close", () => {
@@ -91,6 +92,7 @@ export class AppDebugSocketProxyFactory extends EventEmitter implements IAppDebu
 	}
 
 	private async addWebSocketProxy(device: Mobile.IiOSDevice, appId: string, projectName: string): Promise<ws.Server> {
+		const clientConnectionLockFile = `debug-connection-${device.deviceInfo.identifier}-${appId}.lock`;
 		const cacheKey = `${device.deviceInfo.identifier}-${appId}`;
 		const existingServer = this.deviceWebServers[cacheKey];
 		if (existingServer) {
@@ -107,22 +109,37 @@ export class AppDebugSocketProxyFactory extends EventEmitter implements IAppDebu
 		// 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,
 			host: "localhost",
 			verifyClient: async (info: any, callback: (res: boolean, code?: number, message?: string) => void) => {
+				await this.$lockService.lock(clientConnectionLockFile);
 				let acceptHandshake = true;
 				this.$logger.info("Frontend client connected.");
 				let appDebugSocket;
 				try {
+					if (currentAppSocket) {
+						currentAppSocket.removeAllListeners();
+						currentAppSocket = null;
+						if (currentWebSocket) {
+							currentWebSocket.removeAllListeners();
+							currentWebSocket.close();
+							currentWebSocket = null;
+						}
+						await device.destroyDebugSocket(appId);
+					}
 					appDebugSocket = await device.getDebugSocket(appId, projectName);
+					currentAppSocket = appDebugSocket;
 					this.$logger.info("Backend socket created.");
 					info.req["__deviceSocket"] = appDebugSocket;
 				} catch (err) {
 					err.deviceIdentifier = device.deviceInfo.identifier;
 					this.$logger.trace(err);
 					this.emit(CONNECTION_ERROR_EVENT_NAME, err);
 					acceptHandshake = false;
+					this.$lockService.unlock(clientConnectionLockFile);
 					this.$logger.warn(`Cannot connect to device socket. The error message is '${err.message}'.`);
 				}
 
@@ -131,6 +148,7 @@ export class AppDebugSocketProxyFactory extends EventEmitter implements IAppDebu
 		});
 		this.deviceWebServers[cacheKey] = server;
 		server.on("connection", (webSocket, req) => {
+			currentWebSocket = webSocket;
 			const encoding = "utf16le";
 
 			const appDebugSocket: net.Socket = (<any>req)["__deviceSocket"];
@@ -163,20 +181,23 @@ export class AppDebugSocketProxyFactory extends EventEmitter implements IAppDebu
 			});
 
 			appDebugSocket.on("close", () => {
+				currentAppSocket = null;
 				this.$logger.info("Backend socket closed!");
 				webSocket.close();
 			});
 
-			webSocket.on("close", () => {
+			webSocket.on("close", async () => {
+				currentWebSocket = null;
 				this.$logger.info('Frontend socket closed!');
 				appDebugSocket.unpipe(packets);
 				packets.destroy();
-				device.destroyDebugSocket(appId);
+				await device.destroyDebugSocket(appId);
 				if (!this.$options.watch) {
 					process.exit(0);
 				}
 			});
 
+			this.$lockService.unlock(clientConnectionLockFile);
 		});
 
 		return server;

lib/services/livesync/ios-device-livesync-service.ts

@@ -150,16 +150,16 @@ export class IOSDeviceLiveSyncService extends DeviceLiveSyncServiceBase implemen
 			});
 		} catch (error) {
 			this.$logger.trace("Error while sending message:", error);
-			this.destroySocket();
+			await this.destroySocket();
 		}
 	}
 
-	private destroySocket(): void {
+	private async destroySocket(): Promise<void> {
 		if (this.socket) {
 			// we do not support LiveSync on multiple apps on the same device
 			// in order to do that, we should cache the socket per app
 			// and destroy just the current app socket when possible
-			this.device.destroyAllSockets();
+			await this.device.destroyAllSockets();
 			this.socket = null;
 		}
 	}