Merge pull request #4279 from NativeScript/tachev/fix-ios-debug-issues

fix: fix debug iOS connection issues

Author: Dimitar Tachev

lib/common/bootstrap.ts

@@ -122,4 +122,4 @@ $injector.require("qr", "./services/qr");
 $injector.require("printPluginsService", "./services/plugins/print-plugins-service");
 $injector.require("npmPluginsService", "./services/plugins/npm-plugins-service");
 
-$injector.require("lockfile", "./services/lockfile");
+$injector.require(["lockfile", "lockService"], "./services/lock-service");

lib/common/declarations.d.ts

@@ -1958,67 +1958,6 @@ interface IiOSNotificationService {
 	postNotification(deviceIdentifier: string, notification: string, commandType?: string): Promise<number>;
 }
 
-/**
- * Copied from @types/lockfile
- * Describes the options that can be passed to lockfile methods.
- */
-interface ILockFileOptions {
-	/**
-	 * A number of milliseconds to wait for locks to expire before giving up.
-	 * Only used by lockFile.lock. Poll for opts.wait ms.
-	 * If the lock is not cleared by the time the wait expires, then it returns with the original error.
-	 */
-	wait?: number;
-
-	/**
-	 * When using opts.wait, this is the period in ms in which it polls to check if the lock has expired. Defaults to 100.
-	 */
-	pollPeriod?: number;
-
-	/**
-	 * A number of milliseconds before locks are considered to have expired.
-	 */
-	stale?: number;
-
-	/**
-	 * Used by lock and lockSync. Retry n number of times before giving up.
-	 */
-	retries?: number;
-
-	/**
-	 * Used by lock. Wait n milliseconds before retrying.
-	 */
-	retryWait?: number;
-}
-
-/**
- * Describes methods that can be used to use file locking.
- */
-interface ILockFile {
-	/**
-	 * Acquire a file lock on the specified path.
-	 * @param {string} lockFilePath Path to lockfile that has to be created. Defaults to `<profile dir>/lockfile.lock`
-	 * @param {ILockFileOptions} lockFileOpts Options used for creating the lockfile.
-	 * @returns {Promise<void>}
-	 */
-	lock(lockFilePath?: string, lockFileOpts?: ILockFileOptions): Promise<void>;
-
-	/**
-	 * Close and unlink the lockfile.
-	 * @param {string} lockFilePath Path to lockfile that has to be created. Defaults to `<profile dir>/lockfile.lock`
-	 * @returns {void}
-	 */
-	unlock(lockFilePath?: string): void;
-
-	/**
-	 * Check if the lockfile is locked and not stale.
-	 * @param {string} lockFilePath Path to lockfile that has to be created. Defaults to `<profile dir>/lockfile.lock`
-	 * @param {ILockFileOptions} lockFileOpts Options used for creating the lockfile.
-	 * @returns {boolean} true in case file is locked, false otherwise
-	 */
-	check(lockFilePath?: string, lockFileOpts?: ILockFileOptions): boolean;
-}
-
 declare module "stringify-package" {
 	function stringifyPackage(data: any, indent: any, newline: string): string
 	export = stringifyPackage

lib/common/helpers.ts

@@ -495,6 +495,7 @@ export async function connectEventuallyUntilTimeout(factory: () => Promise<net.S
 				socket.on("error", tryConnectAfterTimeout);
 			} catch (e) {
 				lastKnownError = e;
+				tryConnectAfterTimeout(e);
 			}
 		}
 

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

@@ -17,6 +17,7 @@ export class IOSDevice extends IOSDeviceBase {
 		protected $errors: IErrors,
 		private $injector: IInjector,
 		protected $iOSDebuggerPortService: IIOSDebuggerPortService,
+		protected $lockService: ILockService,
 		private $iOSSocketRequestExecutor: IiOSSocketRequestExecutor,
 		protected $processService: IProcessService,
 		private $deviceLogProvider: Mobile.IDeviceLogProvider,
@@ -63,6 +64,7 @@ export class IOSDevice extends IOSDeviceBase {
 		await this.$iOSSocketRequestExecutor.executeAttachRequest(this, constants.AWAIT_NOTIFICATION_TIMEOUT_SECONDS, appId);
 		const port = await this.getDebuggerPort(appId);
 		const deviceId = this.deviceInfo.identifier;
+
 		const socket = await helpers.connectEventuallyUntilTimeout(
 			async () => {
 				const deviceResponse = _.first((await this.$iosDeviceOperations.connectToPort([{ deviceId: deviceId, port: port }]))[deviceId]);

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

@@ -5,6 +5,7 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice {
 	protected abstract $errors: IErrors;
 	protected abstract $iOSDebuggerPortService: IIOSDebuggerPortService;
 	protected abstract $processService: IProcessService;
+	protected abstract $lockService: ILockService;
 	abstract deviceInfo: Mobile.IDeviceInfo;
 	abstract applicationManager: Mobile.IDeviceApplicationManager;
 	abstract fileSystem: Mobile.IDeviceFileSystem;
@@ -24,21 +25,23 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice {
 	}
 
 	public async getSocket(appId: string): Promise<net.Socket> {
-		if (this.cachedSockets[appId]) {
-			return this.cachedSockets[appId];
-		}
+		return this.$lockService.executeActionWithLock(async () => {
+			if (this.cachedSockets[appId]) {
+				return this.cachedSockets[appId];
+			}
 
-		this.cachedSockets[appId] = await this.getSocketCore(appId);
+			this.cachedSockets[appId] = await this.getSocketCore(appId);
 
-		if (this.cachedSockets[appId]) {
-			this.cachedSockets[appId].on("close", () => {
-				this.destroySocket(appId);
-			});
+			if (this.cachedSockets[appId]) {
+				this.cachedSockets[appId].on("close", () => {
+					this.destroySocket(appId);
+				});
 
-			this.$processService.attachToProcessExitSignals(this, () => this.destroySocket(appId));
-		}
+				this.$processService.attachToProcessExitSignals(this, () => this.destroySocket(appId));
+			}
 
-		return this.cachedSockets[appId];
+			return this.cachedSockets[appId];
+		}, "ios-debug-socket.lock");
 	}
 
 	public destroyLiveSyncSocket(appId: string) {

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

@@ -14,6 +14,7 @@ export class IOSSimulator extends IOSDeviceBase implements Mobile.IiOSDevice {
 	constructor(private simulator: Mobile.IiSimDevice,
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		protected $errors: IErrors,
+		protected $lockService: ILockService,
 		private $injector: IInjector,
 		protected $iOSDebuggerPortService: IIOSDebuggerPortService,
 		private $iOSSimResolver: Mobile.IiOSSimResolver,

lib/common/services/lock-service.ts

@@ -0,0 +1,88 @@
+import * as lockfile from "lockfile";
+import * as path from "path";
+import { cache } from "../decorators";
+
+export class LockService implements ILockService {
+	private currentlyLockedFiles: string[] = [];
+
+	@cache()
+	private get defaultLockFilePath(): string {
+		return this.getAbsoluteLockFilePath("lockfile.lock");
+	}
+
+	private getAbsoluteLockFilePath(relativeLockFilePath: string) {
+		return path.join(this.$settingsService.getProfileDir(), relativeLockFilePath);
+	}
+
+	private get defaultLockParams(): ILockOptions {
+		// We'll retry 100 times and time between retries is 100ms, i.e. full wait in case we are unable to get lock will be 10 seconds.
+		// In case lock is older than the `stale` value, consider it stale and try to get a new lock.
+		const lockParams: ILockOptions = {
+			retryWait: 100,
+			retries: 100,
+			stale: 30 * 1000,
+		};
+
+		return lockParams;
+	}
+
+	constructor(private $fs: IFileSystem,
+		private $settingsService: ISettingsService,
+		private $processService: IProcessService) {
+		this.$processService.attachToProcessExitSignals(this, () => {
+			const locksToRemove = _.clone(this.currentlyLockedFiles);
+			_.each(locksToRemove, lock => {
+				this.unlock(lock);
+			});
+		});
+	}
+
+	public async executeActionWithLock<T>(action: () => Promise<T>, lockFilePath?: string, lockOpts?: ILockOptions): Promise<T> {
+		const resolvedLockFilePath = await this.lock(lockFilePath, lockOpts);
+
+		try {
+			const result = await action();
+			return result;
+		} finally {
+			this.unlock(resolvedLockFilePath);
+		}
+	}
+
+	private lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<string> {
+		const { filePath, fileOpts } = this.getLockFileSettings(lockFilePath, lockOpts);
+		this.currentlyLockedFiles.push(filePath);
+
+		// Prevent ENOENT error when the dir, where lock should be created, does not exist.
+		this.$fs.ensureDirectoryExists(path.dirname(filePath));
+
+		return new Promise<string>((resolve, reject) => {
+			lockfile.lock(filePath, fileOpts, (err: Error) => {
+				err ? reject(new Error(`Timeout while waiting for lock "${filePath}"`)) : resolve(filePath);
+			});
+		});
+	}
+
+	private unlock(lockFilePath?: string): void {
+		const { filePath } = this.getLockFileSettings(lockFilePath);
+		_.remove(this.currentlyLockedFiles, e => e === lockFilePath);
+		lockfile.unlockSync(filePath);
+	}
+
+	private getLockFileSettings(filePath?: string, fileOpts?: ILockOptions): { filePath: string, fileOpts: ILockOptions } {
+		if (filePath && !path.isAbsolute(filePath)) {
+			filePath = this.getAbsoluteLockFilePath(filePath);
+		}
+
+		filePath = filePath || this.defaultLockFilePath;
+		fileOpts = fileOpts || this.defaultLockParams;
+
+		return {
+			filePath,
+			fileOpts
+		};
+	}
+}
+
+$injector.register("lockService", LockService);
+// backwards compatibility
+$injector.register("lockfile", LockService);

lib/common/services/lockfile.ts

@@ -5,12 +5,12 @@ export class UserSettingsServiceBase implements IUserSettingsService {
 	private userSettingsFilePath: string = null;
 	protected userSettingsData: any = null;
 	private get lockFilePath(): string {
-		return `${this.userSettingsFilePath}.lock`;
+		return `user-settings.lock`;
 	}
 
 	constructor(userSettingsFilePath: string,
 		protected $fs: IFileSystem,
-		protected $lockfile: ILockFile,
+		protected $lockService: ILockService,
 		private $logger: ILogger) {
 		this.userSettingsFilePath = userSettingsFilePath;
 	}
@@ -21,7 +21,7 @@ export class UserSettingsServiceBase implements IUserSettingsService {
 			return this.userSettingsData ? this.userSettingsData[settingName] : null;
 		};
 
-		return this.executeActionWithLock<T>(action);
+		return this.$lockService.executeActionWithLock<T>(action, this.lockFilePath);
 	}
 
 	public async saveSetting<T>(key: string, value: T): Promise<void> {
@@ -39,17 +39,7 @@ export class UserSettingsServiceBase implements IUserSettingsService {
 			await this.saveSettings();
 		};
 
-		return this.executeActionWithLock<void>(action);
-	}
-
-	private async executeActionWithLock<T>(action: () => Promise<T>): Promise<T> {
-		try {
-			await this.$lockfile.lock(this.lockFilePath);
-			const result = await action();
-			return result;
-		} finally {
-			this.$lockfile.unlock(this.lockFilePath);
-		}
+		return this.$lockService.executeActionWithLock<void>(action, this.lockFilePath);
 	}
 
 	public saveSettings(data?: any): Promise<void> {
@@ -66,7 +56,7 @@ export class UserSettingsServiceBase implements IUserSettingsService {
 			this.$fs.writeJson(this.userSettingsFilePath, this.userSettingsData);
 		};
 
-		return this.executeActionWithLock<void>(action);
+		return this.$lockService.executeActionWithLock<void>(action, this.lockFilePath);
 	}
 
 	// TODO: Remove Promise, reason: writeFile - blocked as other implementation of the interface has async operation.

lib/common/services/user-settings-service.ts

@@ -4,7 +4,7 @@ import { Yok } from "../../../yok";
 import { assert } from "chai";
 import { DeviceDiscoveryEventNames, CONNECTED_STATUS } from "../../../constants";
 import { DevicePlatformsConstants } from "../../../mobile/device-platforms-constants";
-import { ErrorsStub, CommonLoggerStub, HooksServiceStub } from "../stubs";
+import { ErrorsStub, CommonLoggerStub, HooksServiceStub, LockServiceStub } from "../stubs";
 import { FileSystemStub } from "../../../../../test/stubs";
 
 let currentlyRunningSimulators: Mobile.IiSimDevice[];
@@ -16,6 +16,7 @@ function createTestInjector(): IInjector {
 	injector.register("injector", injector);
 	injector.register("errors", ErrorsStub);
 	injector.register("iOSDebuggerPortService", {});
+	injector.register("lockService", LockServiceStub);
 	injector.register("iOSSimResolver", {
 		iOSSim: {
 			getRunningSimulators: async () => currentlyRunningSimulators

lib/common/test/unit-tests/mobile/ios-simulator-discovery.ts

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