Skip to content

fix: fix debug iOS connection issues #4279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/common/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
61 changes: 0 additions & 61 deletions lib/common/declarations.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/common/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ export async function connectEventuallyUntilTimeout(factory: () => Promise<net.S
socket.on("error", tryConnectAfterTimeout);
} catch (e) {
lastKnownError = e;
tryConnectAfterTimeout(e);
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/common/mobile/ios/device/ios-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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]);
Expand Down
25 changes: 14 additions & 11 deletions lib/common/mobile/ios/ios-device-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions lib/common/mobile/ios/simulator/ios-simulator-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
88 changes: 88 additions & 0 deletions lib/common/services/lock-service.ts
Original file line number Diff line number Diff line change
@@ -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);
63 changes: 0 additions & 63 deletions lib/common/services/lockfile.ts

This file was deleted.

20 changes: 5 additions & 15 deletions lib/common/services/user-settings-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion lib/common/test/unit-tests/mobile/ios-simulator-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions lib/common/test/unit-tests/stubs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Loading