From 19fa2425f2b36c1a8cac2478babac5c67cb760c6 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 10 Apr 2019 12:10:24 +0300 Subject: [PATCH 01/16] fix: stop handling SIGINT signal When Ctrl+C is used in terminal, CLI receives SIGINT signal and tries to clear its resources. In Node.js, when you handle SIGINT signal, you'll have to clean all resources on your own. This leads to different problems, for example in case you press Ctrl+C during doctor execution, CLI will only kill the doctor process, but will continue working. Instead of handling the SIGINT signal, let the OS handle it on its own. This works quite well except one thing - any package that is required may add handler for SIGINT signal and this will break the idea. So repace the `process.on` function with our own one which does not allow adding handler for SIGINT signal. --- lib/common/services/process-service.ts | 2 +- lib/common/test/unit-tests/process-service.ts | 2 +- lib/nativescript-cli.ts | 15 ++++++++++++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/common/services/process-service.ts b/lib/common/services/process-service.ts index 25eed30732..ff7fb4a540 100644 --- a/lib/common/services/process-service.ts +++ b/lib/common/services/process-service.ts @@ -1,5 +1,5 @@ export class ProcessService implements IProcessService { - private static PROCESS_EXIT_SIGNALS = ["exit", "SIGINT", "SIGTERM"]; + private static PROCESS_EXIT_SIGNALS = ["exit", "SIGTERM"]; private _listeners: IListener[]; public get listenersCount(): number { diff --git a/lib/common/test/unit-tests/process-service.ts b/lib/common/test/unit-tests/process-service.ts index 8daf0434d4..18d382c8e9 100644 --- a/lib/common/test/unit-tests/process-service.ts +++ b/lib/common/test/unit-tests/process-service.ts @@ -2,7 +2,7 @@ import { Yok } from "../../yok"; import { ProcessService } from "../../services/process-service"; import { assert } from "chai"; -const processExitSignals = ["exit", "SIGINT", "SIGTERM"]; +const processExitSignals = ["exit", "SIGTERM"]; const emptyFunction = () => { /* no implementation required */ }; function createTestInjector(): IInjector { const testInjector = new Yok(); diff --git a/lib/nativescript-cli.ts b/lib/nativescript-cli.ts index 941367fad7..431413ca28 100644 --- a/lib/nativescript-cli.ts +++ b/lib/nativescript-cli.ts @@ -1,12 +1,23 @@ require("./bootstrap"); + import { EOL } from "os"; import * as shelljs from "shelljs"; shelljs.config.silent = true; shelljs.config.fatal = true; import { installUncaughtExceptionListener } from "./common/errors"; +import { settlePromises } from "./common/helpers"; installUncaughtExceptionListener(process.exit.bind(process, ErrorCodes.UNCAUGHT)); -import { settlePromises } from "./common/helpers"; +const logger: ILogger = $injector.resolve("logger"); +const originalProcessOn = process.on; + +process.on = (event: string, listener: any): any => { + if (event === "SIGINT") { + logger.trace(new Error(`Trying to handle SIGINT event. CLI overrides this behavior and does not allow handling SIGINT as this causes issues with Ctrl + C in terminal`).stack); + } else { + return originalProcessOn.apply(process, [event, listener]); + } +}; /* tslint:disable:no-floating-promises */ (async () => { @@ -14,8 +25,6 @@ import { settlePromises } from "./common/helpers"; const err: IErrors = $injector.resolve("$errors"); err.printCallStack = config.DEBUG; - const logger: ILogger = $injector.resolve("logger"); - const extensibilityService: IExtensibilityService = $injector.resolve("extensibilityService"); try { await settlePromises(extensibilityService.loadExtensions()); From b820bc589db6b2a5c897a223d12d689d29f59887 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 10 Apr 2019 12:26:16 +0300 Subject: [PATCH 02/16] fix: kill leaked analytics process With recent changes, CLI's processService does not handle SIGINT signal, so it actually never sends Finish message to analytics process. CLI dies gracefully, but the analytics process is not killed. To fix this add `process.exit` in the analytics process when it receives disconnect event. Also remove all logic for Finish message as CLI will never send it. --- lib/common/declarations.d.ts | 5 --- .../analytics/analytics-broker-process.ts | 37 +++---------------- lib/services/analytics/analytics-service.ts | 9 ----- 3 files changed, 5 insertions(+), 46 deletions(-) diff --git a/lib/common/declarations.d.ts b/lib/common/declarations.d.ts index 006b24015e..ab90c6f884 100644 --- a/lib/common/declarations.d.ts +++ b/lib/common/declarations.d.ts @@ -207,11 +207,6 @@ declare const enum TrackingTypes { * Defines that the broker process should get and track the data from preview app to Google Analytics */ PreviewAppData = "PreviewAppData", - - /** - * Defines that all information has been sent and no more data will be tracked in current session. - */ - Finish = "finish" } /** diff --git a/lib/services/analytics/analytics-broker-process.ts b/lib/services/analytics/analytics-broker-process.ts index a687fd87df..b522bab446 100644 --- a/lib/services/analytics/analytics-broker-process.ts +++ b/lib/services/analytics/analytics-broker-process.ts @@ -20,38 +20,17 @@ const analyticsBroker = $injector.resolve(AnalyticsBroker, { p let trackingQueue: Promise = Promise.resolve(); -let sentFinishMsg = false; -let receivedFinishMsg = false; - const sendDataForTracking = async (data: ITrackingInformation) => { trackingQueue = trackingQueue.then(() => analyticsBroker.sendDataForTracking(data)); await trackingQueue; }; const finishTracking = async (data?: ITrackingInformation) => { - analyticsLoggingService.logData({ message: `analytics-broker-process finish tracking started, sentFinishMsg: ${sentFinishMsg}, receivedFinishMsg: ${receivedFinishMsg}` }); - - if (!sentFinishMsg) { - sentFinishMsg = true; - - data = data || { type: TrackingTypes.Finish }; - const action = async () => { - await trackingQueue; - analyticsLoggingService.logData({ message: `analytics-broker-process tracking finished` }); - process.disconnect(); - }; - - if (receivedFinishMsg) { - await action(); - } else { - // In case we've got here without receiving "finish" message from parent (receivedFinishMsg is false) - // there might be various reasons, but most probably the parent is dead. - // However, there's no guarantee that we've received all messages. So wait some time before sending finish message to children. - setTimeout(async () => { - await action(); - }, 1000); - } - } + analyticsLoggingService.logData({ message: `analytics-broker-process finish tracking started` }); + await trackingQueue; + analyticsLoggingService.logData({ message: `analytics-broker-process tracking finished` }); + process.disconnect(); + process.exit(); }; const trackPreviewAppData = async (data: any) => { @@ -84,12 +63,6 @@ const trackPreviewAppData = async (data: any) => { process.on("message", async (data: ITrackingInformation) => { analyticsLoggingService.logData({ message: `analytics-broker-process received message of type: ${JSON.stringify(data)}` }); - if (data.type === TrackingTypes.Finish) { - receivedFinishMsg = true; - await finishTracking(data); - return; - } - if (data.type === TrackingTypes.PreviewAppData) { await trackPreviewAppData(data); return; diff --git a/lib/services/analytics/analytics-service.ts b/lib/services/analytics/analytics-service.ts index 3871e75752..cc98102b6a 100644 --- a/lib/services/analytics/analytics-service.ts +++ b/lib/services/analytics/analytics-service.ts @@ -13,7 +13,6 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { constructor(private $logger: ILogger, private $options: IOptions, - private $processService: IProcessService, private $staticConfig: Config.IStaticConfig, private $prompter: IPrompter, private $userSettingsService: UserSettings.IUserSettingsService, @@ -228,15 +227,7 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { if (!isSettled) { isSettled = true; - - this.$processService.attachToProcessExitSignals(this, () => { - broker.send({ - type: TrackingTypes.Finish - }); - }); - this.brokerProcess = broker; - resolve(broker); } } From 4d848abf78575c8c47a9bb8b3169b304d79db836 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 10 Apr 2019 23:33:13 +0300 Subject: [PATCH 03/16] feat: introduce cleanup child process In case CLI uses non-Nodejs resources, there's nowhere to clean them as CLI does not handle the SIGINT (Ctrl+C) signal. For example, when we debug on Android, we setup port forward with `adb` command. Once CLI process is about to stop, the adb forward should be removed, but currently there's nowhere to do it. So, to handle this, introduce a new cleanup child process. It is detached and unrefed. As we have IPC communication with it, when CLI exits gracefully (i.e. when we are not in a long living command which usually exits with Ctrl+C), we should manually disconnect the process. Whenever some external resources must be cleaned, the actual cleanup commands that should be executed are sent to the new process. It caches them and once the CLI finishes its work, the cleanup process will receive `disconnect` event. At this point the cleanup process will execute all cleanup actions and after that it will exit. The idea of the process is very similar to the process used for analytics tracking, so several interfaces were reused. They are just renamed to have a better meaning for the new used cases. Introduce `--cleanupLogFile` option, which allows the user to specify path to file, where information about all actions will be logged. Also expose a method in CLI's public API that allows setting this file when using CLI as a library. --- lib/bootstrap.ts | 1 + lib/common/services/commands-service.ts | 4 +- lib/declarations.d.ts | 1 + lib/definitions/cleanup-service.d.ts | 20 ++++ lib/definitions/file-log-service.d.ts | 19 ++++ .../cleanup-process-definitions.d.ts | 28 +++++ lib/detached-processes/cleanup-process.ts | 68 ++++++++++++ .../detached-process-enums.d.ts | 28 +++++ lib/detached-processes/file-log-service.ts | 15 +++ lib/options.ts | 1 + .../analytics/analytics-broker-process.ts | 8 +- lib/services/analytics/analytics-broker.ts | 4 +- .../analytics/analytics-constants.d.ts | 24 ----- .../analytics/analytics-logging-service.ts | 15 --- lib/services/analytics/analytics-service.ts | 2 +- lib/services/analytics/analytics.d.ts | 20 ---- .../analytics/google-analytics-provider.ts | 8 +- lib/services/cleanup-service.ts | 101 ++++++++++++++++++ test/nativescript-cli-lib.ts | 3 + test/platform-commands.ts | 3 + test/platform-service.ts | 3 + test/plugins-service.ts | 5 + test/services/analytics/analytics-service.ts | 8 +- 23 files changed, 314 insertions(+), 75 deletions(-) create mode 100644 lib/definitions/cleanup-service.d.ts create mode 100644 lib/definitions/file-log-service.d.ts create mode 100644 lib/detached-processes/cleanup-process-definitions.d.ts create mode 100644 lib/detached-processes/cleanup-process.ts create mode 100644 lib/detached-processes/detached-process-enums.d.ts create mode 100644 lib/detached-processes/file-log-service.ts delete mode 100644 lib/services/analytics/analytics-constants.d.ts delete mode 100644 lib/services/analytics/analytics-logging-service.ts create mode 100644 lib/services/cleanup-service.ts diff --git a/lib/bootstrap.ts b/lib/bootstrap.ts index cc08b41de5..e359fa3f35 100644 --- a/lib/bootstrap.ts +++ b/lib/bootstrap.ts @@ -192,3 +192,4 @@ $injector.require("qrCodeTerminalService", "./services/qr-code-terminal-service" $injector.require("testInitializationService", "./services/test-initialization-service"); $injector.require("networkConnectivityValidator", "./helpers/network-connectivity-validator"); +$injector.requirePublic("cleanupService", "./services/cleanup-service"); diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index d441a2959a..1f9ccfde2e 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -28,7 +28,8 @@ export class CommandsService implements ICommandsService { private $helpService: IHelpService, private $extensibilityService: IExtensibilityService, private $optionsTracker: IOptionsTracker, - private $projectDataService: IProjectDataService) { + private $projectDataService: IProjectDataService, + private $cleanupService: ICleanupService) { let projectData = null; try { projectData = this.$projectDataService.getProjectData(); @@ -37,6 +38,7 @@ export class CommandsService implements ICommandsService { } this.$options.setupOptions(projectData); + this.$cleanupService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); } public allCommands(opts: { includeDevCommands: boolean }): string[] { diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index 49ab41fe42..e048c14ba4 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -570,6 +570,7 @@ interface IOptions extends IRelease, IDeviceIdentifier, IJustLaunch, IAvd, IAvai analyticsLogFile: string; performance: Object; setupOptions(projectData: IProjectData): void; + cleanupLogFile: string; } interface IEnvOptions { diff --git a/lib/definitions/cleanup-service.d.ts b/lib/definitions/cleanup-service.d.ts new file mode 100644 index 0000000000..8817ea0495 --- /dev/null +++ b/lib/definitions/cleanup-service.d.ts @@ -0,0 +1,20 @@ +/** + * Descibes the cleanup service which allows scheduling cleanup actions + * The actions will be executed once CLI process exits. + */ +interface ICleanupService extends IShouldDispose, IDisposable { + /** + * Add new action to be executed when CLI process exits. + * @param {ICleanupAction} action The action that should be executed, including command and args. + * @returns {Promise} + */ + addCleanupAction(action: ICleanupAction): Promise; + + /** + * Sets the file in which the cleanup process will write its logs. + * This method must be called before starting the cleanup process, i.e. when CLI is initialized. + * @param {string} filePath Path to file where the logs will be written. The logs are appended to the passed file. + * @returns {void} + */ + setCleanupLogFile(filePath: string): void +} diff --git a/lib/definitions/file-log-service.d.ts b/lib/definitions/file-log-service.d.ts new file mode 100644 index 0000000000..1692db207c --- /dev/null +++ b/lib/definitions/file-log-service.d.ts @@ -0,0 +1,19 @@ +/** + * Describes message that needs to be logged in the analytics logging file. + */ +interface IFileLogMessage { + message: string; + type?: FileLogMessageType; +} + +/** + * Describes methods to get local logs from analytics tracking. + */ +interface IFileLogService { + /** + * Logs specified message to the previously specified file. + * @param {IFileLogMessage} fileLogMessage The message that has to be written to the logs file. + * @returns {void} + */ + logData(fileLogMessage: IFileLogMessage): void; +} diff --git a/lib/detached-processes/cleanup-process-definitions.d.ts b/lib/detached-processes/cleanup-process-definitions.d.ts new file mode 100644 index 0000000000..49968583b3 --- /dev/null +++ b/lib/detached-processes/cleanup-process-definitions.d.ts @@ -0,0 +1,28 @@ +interface ICleanupAction { + /** + * Executable to be started. + */ + command: string; + + /** + * Arguments that will be passed to the child process + */ + args: string[]; + /** + * Timeout to execute the action. + */ + timeout?: number; +} + +interface ICleanupProcessMessage { + /** + * Type of the action + */ + actionType: CleanupProcessMessageType; + + /** + * Describes the action that must be executed + */ + action: ICleanupAction; +} + diff --git a/lib/detached-processes/cleanup-process.ts b/lib/detached-processes/cleanup-process.ts new file mode 100644 index 0000000000..1490e21539 --- /dev/null +++ b/lib/detached-processes/cleanup-process.ts @@ -0,0 +1,68 @@ +// NOTE: This file is used to clean up resources used by CLI, when the CLI is killed. +// The instances here are not shared with the ones in main CLI process. +import * as fs from "fs"; +import { FileLogService } from "./file-log-service"; + +const pathToBootstrap = process.argv[2]; +if (!pathToBootstrap || !fs.existsSync(pathToBootstrap)) { + throw new Error("Invalid path to bootstrap."); +} + +const logFile = process.argv[3]; +// After requiring the bootstrap we can use $injector +require(pathToBootstrap); + +const fileLogService = $injector.resolve(FileLogService, { logFile }); +fileLogService.logData({ message: "Initializing Cleanup process." }); + +const actionsToExecute: ICleanupAction[] = []; + +const executeCleanup = async () => { + const $childProcess = $injector.resolve("childProcess"); + for (const action of actionsToExecute) { + try { + fileLogService.logData({ message: `Start executing action: ${JSON.stringify(action)}` }); + + // TODO: Add timeout for each action here + await $childProcess.trySpawnFromCloseEvent(action.command, action.args); + fileLogService.logData({ message: `Successfully executed action: ${JSON.stringify(action)}` }); + } catch (err) { + fileLogService.logData({ message: `Unable to execute action: ${JSON.stringify(action)}`, type: FileLogMessageType.Error }); + } + } + + fileLogService.logData({ message: `cleanup-process finished` }); + process.exit(); +}; + +const addCleanupAction = (newAction: ICleanupAction): void => { + if (!_.some(actionsToExecute, currentAction => _.isEqual(currentAction, newAction))) { + fileLogService.logData({ message: `cleanup-process added action for execution: ${JSON.stringify(newAction)}` }); + actionsToExecute.push(newAction); + } else { + fileLogService.logData({ message: `cleanup-process will not add action for execution as it has been added already: ${JSON.stringify(newAction)}` }); + } +}; + +process.on("message", async (cleanupProcessMessage: ICleanupProcessMessage) => { + fileLogService.logData({ message: `cleanup-process received message of type: ${JSON.stringify(cleanupProcessMessage)}` }); + + switch (cleanupProcessMessage.actionType) { + case CleanupProcessMessageType.AddCleanAction: + addCleanupAction(cleanupProcessMessage.action); + break; + default: + fileLogService.logData({ message: `Unable to handle message of type ${cleanupProcessMessage.actionType}. Full message is ${JSON.stringify(cleanupProcessMessage)}`, type: FileLogMessageType.Error }); + break; + } + +}); + +process.on("disconnect", async () => { + fileLogService.logData({ message: "cleanup-process received process.disconnect event" }); + await executeCleanup(); +}); + +fileLogService.logData({ message: `cleanup-process will send ${DetachedProcessMessages.ProcessReadyToReceive} message` }); + +process.send(DetachedProcessMessages.ProcessReadyToReceive); diff --git a/lib/detached-processes/detached-process-enums.d.ts b/lib/detached-processes/detached-process-enums.d.ts new file mode 100644 index 0000000000..4e5974bfd5 --- /dev/null +++ b/lib/detached-processes/detached-process-enums.d.ts @@ -0,0 +1,28 @@ +/** + * Defines messages used in communication between CLI's process and analytics subprocesses. + */ +declare const enum DetachedProcessMessages { + /** + * The detached process is initialized and is ready to receive information for tracking. + */ + ProcessReadyToReceive = "ProcessReadyToReceive" +} + +/** + * Defines the type of the messages that should be written in the local analyitcs log file (in case such is specified). + */ +declare const enum FileLogMessageType { + /** + * Information message. This is the default value in case type is not specified. + */ + Info = "Info", + + /** + * Error message - used to indicate that some action did not succeed. + */ + Error = "Error" +} + +declare const enum CleanupProcessMessageType { + AddCleanAction = "AddCleanAction", +} diff --git a/lib/detached-processes/file-log-service.ts b/lib/detached-processes/file-log-service.ts new file mode 100644 index 0000000000..ce1308ca36 --- /dev/null +++ b/lib/detached-processes/file-log-service.ts @@ -0,0 +1,15 @@ +import { EOL } from "os"; +import { getFixedLengthDateString } from "../common/helpers"; + +export class FileLogService implements IFileLogService { + constructor(private $fs: IFileSystem, + private logFile: string) { } + + public logData(fileLoggingMessage: IFileLogMessage): void { + if (this.logFile && fileLoggingMessage && fileLoggingMessage.message) { + fileLoggingMessage.type = fileLoggingMessage.type || FileLogMessageType.Info; + const formattedDate = getFixedLengthDateString(); + this.$fs.appendFile(this.logFile, `[${formattedDate}] [${fileLoggingMessage.type}] ${fileLoggingMessage.message}${EOL}`); + } + } +} diff --git a/lib/options.ts b/lib/options.ts index f06be1e70b..b75042a669 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -147,6 +147,7 @@ export class Options { default: { type: OptionType.Boolean, hasSensitiveValue: false }, count: { type: OptionType.Number, hasSensitiveValue: false }, analyticsLogFile: { type: OptionType.String, hasSensitiveValue: true }, + cleanupLogFile: { type: OptionType.String, hasSensitiveValue: true }, hooks: { type: OptionType.Boolean, default: true, hasSensitiveValue: false }, link: { type: OptionType.Boolean, default: false, hasSensitiveValue: false }, aab: { type: OptionType.Boolean, hasSensitiveValue: false }, diff --git a/lib/services/analytics/analytics-broker-process.ts b/lib/services/analytics/analytics-broker-process.ts index b522bab446..9fee55b3a5 100644 --- a/lib/services/analytics/analytics-broker-process.ts +++ b/lib/services/analytics/analytics-broker-process.ts @@ -2,7 +2,7 @@ // The instances here are not shared with the ones in main CLI process. import * as fs from "fs"; import { AnalyticsBroker } from "./analytics-broker"; -import { AnalyticsLoggingService } from "./analytics-logging-service"; +import { FileLogService } from "../../detached-processes/file-log-service"; const pathToBootstrap = process.argv[2]; if (!pathToBootstrap || !fs.existsSync(pathToBootstrap)) { @@ -13,7 +13,7 @@ const logFile = process.argv[3]; // After requiring the bootstrap we can use $injector require(pathToBootstrap); -const analyticsLoggingService = $injector.resolve(AnalyticsLoggingService, { logFile }); +const analyticsLoggingService = $injector.resolve(FileLogService, { logFile }); analyticsLoggingService.logData({ message: "Initializing AnalyticsBroker." }); const analyticsBroker = $injector.resolve(AnalyticsBroker, { pathToBootstrap, analyticsLoggingService }); @@ -76,6 +76,6 @@ process.on("disconnect", async () => { await finishTracking(); }); -analyticsLoggingService.logData({ message: `analytics-broker-process will send ${AnalyticsMessages.BrokerReadyToReceive} message` }); +analyticsLoggingService.logData({ message: `analytics-broker-process will send ${DetachedProcessMessages.ProcessReadyToReceive} message` }); -process.send(AnalyticsMessages.BrokerReadyToReceive); +process.send(DetachedProcessMessages.ProcessReadyToReceive); diff --git a/lib/services/analytics/analytics-broker.ts b/lib/services/analytics/analytics-broker.ts index 296ba02ff0..918d41050d 100644 --- a/lib/services/analytics/analytics-broker.ts +++ b/lib/services/analytics/analytics-broker.ts @@ -10,14 +10,14 @@ export class AnalyticsBroker implements IAnalyticsBroker { constructor(private $analyticsSettingsService: IAnalyticsSettingsService, private $injector: IInjector, - private analyticsLoggingService: IAnalyticsLoggingService) { } + private analyticsLoggingService: IFileLogService) { } public async sendDataForTracking(trackInfo: ITrackingInformation): Promise { try { const googleProvider = await this.getGoogleAnalyticsProvider(); await googleProvider.trackHit(trackInfo); } catch (err) { - this.analyticsLoggingService.logData({ message: `AnalyticsBroker unable to execute action in sendDataForTracking: ${err}`, type: AnalyticsLoggingMessageType.Error }); + this.analyticsLoggingService.logData({ message: `AnalyticsBroker unable to execute action in sendDataForTracking: ${err}`, type: FileLogMessageType.Error }); } } } diff --git a/lib/services/analytics/analytics-constants.d.ts b/lib/services/analytics/analytics-constants.d.ts deleted file mode 100644 index f2d71ce96a..0000000000 --- a/lib/services/analytics/analytics-constants.d.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Defines messages used in communication between CLI's process and analytics subprocesses. - */ -declare const enum AnalyticsMessages { - /** - * Analytics Broker is initialized and is ready to receive information for tracking. - */ - BrokerReadyToReceive = "BrokerReadyToReceive" -} - -/** - * Defines the type of the messages that should be written in the local analyitcs log file (in case such is specified). - */ -declare const enum AnalyticsLoggingMessageType { - /** - * Information message. This is the default value in case type is not specified. - */ - Info = "Info", - - /** - * Error message - used to indicate that some action while trying to track information did not succeeded. - */ - Error = "Error" -} diff --git a/lib/services/analytics/analytics-logging-service.ts b/lib/services/analytics/analytics-logging-service.ts deleted file mode 100644 index 008382f0f8..0000000000 --- a/lib/services/analytics/analytics-logging-service.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { EOL } from "os"; -import { getFixedLengthDateString } from "../../common/helpers"; - -export class AnalyticsLoggingService implements IAnalyticsLoggingService { - constructor(private $fs: IFileSystem, - private logFile: string) { } - - public logData(analyticsLoggingMessage: IAnalyticsLoggingMessage): void { - if (this.logFile && analyticsLoggingMessage && analyticsLoggingMessage.message) { - analyticsLoggingMessage.type = analyticsLoggingMessage.type || AnalyticsLoggingMessageType.Info; - const formattedDate = getFixedLengthDateString(); - this.$fs.appendFile(this.logFile, `[${formattedDate}] [${analyticsLoggingMessage.type}] ${analyticsLoggingMessage.message}${EOL}`); - } - } -} diff --git a/lib/services/analytics/analytics-service.ts b/lib/services/analytics/analytics-service.ts index cc98102b6a..f160359193 100644 --- a/lib/services/analytics/analytics-service.ts +++ b/lib/services/analytics/analytics-service.ts @@ -222,7 +222,7 @@ export class AnalyticsService implements IAnalyticsService, IDisposable { }); broker.on("message", (data: any) => { - if (data === AnalyticsMessages.BrokerReadyToReceive) { + if (data === DetachedProcessMessages.ProcessReadyToReceive) { clearTimeout(timeoutId); if (!isSettled) { diff --git a/lib/services/analytics/analytics.d.ts b/lib/services/analytics/analytics.d.ts index 6517ea016f..90d8837383 100644 --- a/lib/services/analytics/analytics.d.ts +++ b/lib/services/analytics/analytics.d.ts @@ -50,23 +50,3 @@ interface IGoogleAnalyticsProvider { */ trackHit(data: IGoogleAnalyticsData): Promise; } - -/** - * Describes message that needs to be logged in the analytics logging file. - */ -interface IAnalyticsLoggingMessage { - message: string; - type?: AnalyticsLoggingMessageType -} - -/** - * Describes methods to get local logs from analytics tracking. - */ -interface IAnalyticsLoggingService { - /** - * Logs specified message to the file specified with `--analyticsLogFile`. - * @param {IAnalyticsLoggingMessage} analyticsLoggingMessage The message that has to be written to the logs file. - * @returns {void} - */ - logData(analyticsLoggingMessage: IAnalyticsLoggingMessage): void; -} diff --git a/lib/services/analytics/google-analytics-provider.ts b/lib/services/analytics/google-analytics-provider.ts index a9a3463655..9173318793 100644 --- a/lib/services/analytics/google-analytics-provider.ts +++ b/lib/services/analytics/google-analytics-provider.ts @@ -12,7 +12,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { private $logger: ILogger, private $proxyService: IProxyService, private $config: IConfiguration, - private analyticsLoggingService: IAnalyticsLoggingService) { + private analyticsLoggingService: IFileLogService) { } public async trackHit(trackInfo: IGoogleAnalyticsData): Promise { @@ -21,7 +21,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { try { await this.track(this.$config.GA_TRACKING_ID, trackInfo, sessionId); } catch (e) { - this.analyticsLoggingService.logData({ type: AnalyticsLoggingMessageType.Error, message: `Unable to track information ${JSON.stringify(trackInfo)}. Error is: ${e}` }); + this.analyticsLoggingService.logData({ type: FileLogMessageType.Error, message: `Unable to track information ${JSON.stringify(trackInfo)}. Error is: ${e}` }); this.$logger.trace("Analytics exception: ", e); } } @@ -95,7 +95,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { this.analyticsLoggingService.logData({ message: `Unable to track event with category: '${trackInfo.category}', action: '${trackInfo.action}', label: '${trackInfo.label}', ` + `value: '${trackInfo.value}' attached page: ${this.currentPage}. Error is: ${err}.`, - type: AnalyticsLoggingMessageType.Error + type: FileLogMessageType.Error }); reject(err); @@ -121,7 +121,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider { if (err) { this.analyticsLoggingService.logData({ message: `Unable to track pageview with path '${trackInfo.path}' and title: '${trackInfo.title}' Error is: ${err}.`, - type: AnalyticsLoggingMessageType.Error + type: FileLogMessageType.Error }); reject(err); diff --git a/lib/services/cleanup-service.ts b/lib/services/cleanup-service.ts new file mode 100644 index 0000000000..e44eb518eb --- /dev/null +++ b/lib/services/cleanup-service.ts @@ -0,0 +1,101 @@ +import { ChildProcess } from "child_process"; +import * as path from "path"; +import { cache, exported } from "../common/decorators"; + +export class CleanupService implements ICleanupService { + private static CLEANUP_PROCESS_START_TIMEOUT = 10 * 1000; + private pathToCleanupLogFile: string; + private cleanupProcess: ChildProcess; + + constructor($options: IOptions, + private $staticConfig: Config.IStaticConfig, + private $childProcess: IChildProcess) { + this.pathToCleanupLogFile = $options.cleanupLogFile; + } + + public shouldDispose = false; + + public async addCleanupAction(action: ICleanupAction): Promise { + const cleanupProcess = await this.getCleanupProcess(); + cleanupProcess.send({ actionType: CleanupProcessMessageType.AddCleanAction, action }); + } + + @exported("cleanupService") + public setCleanupLogFile(filePath: string): void { + this.pathToCleanupLogFile = filePath; + } + + public dispose(): void { + if (this.cleanupProcess && this.shouldDispose) { + this.cleanupProcess.disconnect(); + } + } + + public setShouldDispose(shouldDispose: boolean): void { + this.shouldDispose = shouldDispose; + } + + // TODO: Consider extracting this method to a separate service + // as it has the same logic as the one used in analytics-service + @cache() + private getCleanupProcess(): Promise { + return new Promise((resolve, reject) => { + const cleanupProcessArgs = this.getCleanupProcessArgs(); + + const cleanupProcess = this.$childProcess.spawn(process.execPath, + cleanupProcessArgs, + { + stdio: ["ignore", "ignore", "ignore", "ipc"], + detached: true + } + ); + + cleanupProcess.unref(); + + let isSettled = false; + + const timeoutId = setTimeout(() => { + if (!isSettled) { + reject(new Error("Unable to start Cleanup process.")); + } + }, CleanupService.CLEANUP_PROCESS_START_TIMEOUT); + + cleanupProcess.on("error", (err: Error) => { + clearTimeout(timeoutId); + + if (!isSettled) { + isSettled = true; + // In case we throw error here, CLI will break its execution. + reject(err); + } + }); + + cleanupProcess.on("message", (data: any) => { + if (data === DetachedProcessMessages.ProcessReadyToReceive) { + clearTimeout(timeoutId); + + if (!isSettled) { + isSettled = true; + this.cleanupProcess = cleanupProcess; + resolve(cleanupProcess); + } + } + }); + }); + } + + private getCleanupProcessArgs(): string[] { + const cleanupProcessArgs = [ + path.join(__dirname, "..", "detached-processes", "cleanup-process.js"), + this.$staticConfig.PATH_TO_BOOTSTRAP, + ]; + + if (this.pathToCleanupLogFile) { + cleanupProcessArgs.push(path.resolve(this.pathToCleanupLogFile)); + } + + return cleanupProcessArgs; + } +} + +$injector.register("cleanupService", CleanupService); diff --git a/test/nativescript-cli-lib.ts b/test/nativescript-cli-lib.ts index 9238cfcc61..f32f35f779 100644 --- a/test/nativescript-cli-lib.ts +++ b/test/nativescript-cli-lib.ts @@ -59,6 +59,9 @@ describe("nativescript-cli-lib", () => { sysInfo: [ "getSupportedNodeVersionRange", "getSystemWarnings" + ], + cleanupService: [ + "setCleanupLogFile" ] }; diff --git a/test/platform-commands.ts b/test/platform-commands.ts index fa19182278..5bba06ea46 100644 --- a/test/platform-commands.ts +++ b/test/platform-commands.ts @@ -179,6 +179,9 @@ function createTestInjector() { testInjector.register("doctorService", { checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined }); + testInjector.register("cleanupService", { + setShouldDispose: (shouldDispose: boolean): void => undefined + }); return testInjector; } diff --git a/test/platform-service.ts b/test/platform-service.ts index 8503a71a06..1f61e9fc77 100644 --- a/test/platform-service.ts +++ b/test/platform-service.ts @@ -121,6 +121,9 @@ function createTestInjector() { testInjector.register("doctorService", { checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined }); + testInjector.register("cleanupService", { + setShouldDispose: (shouldDispose: boolean): void => undefined + }); return testInjector; } diff --git a/test/plugins-service.ts b/test/plugins-service.ts index f2530b2e73..bb2b011cd1 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -150,6 +150,11 @@ function createTestInjector() { }, extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => undefined }); + + testInjector.register("cleanupService", { + setShouldDispose: (shouldDispose: boolean): void => undefined + }); + return testInjector; } diff --git a/test/services/analytics/analytics-service.ts b/test/services/analytics/analytics-service.ts index ed626f64dc..6dc149ffbd 100644 --- a/test/services/analytics/analytics-service.ts +++ b/test/services/analytics/analytics-service.ts @@ -194,7 +194,7 @@ describe("analyticsService", () => { spawnedProcess.connected = false; spawnedProcess.send = (): void => undefined; - setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + setTimeout(() => spawnedProcess.emit("message", DetachedProcessMessages.ProcessReadyToReceive), 1); return spawnedProcess; }; @@ -213,7 +213,7 @@ describe("analyticsService", () => { throw new Error("Failed to sent data."); }; - setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + setTimeout(() => spawnedProcess.emit("message", DetachedProcessMessages.ProcessReadyToReceive), 1); return spawnedProcess; }; @@ -233,7 +233,7 @@ describe("analyticsService", () => { action(); }; - setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + setTimeout(() => spawnedProcess.emit("message", DetachedProcessMessages.ProcessReadyToReceive), 1); return spawnedProcess; }; @@ -264,7 +264,7 @@ describe("analyticsService", () => { action(); }; - setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + setTimeout(() => spawnedProcess.emit("message", DetachedProcessMessages.ProcessReadyToReceive), 1); return spawnedProcess; }; From 6028eaece6cdc78cbc41b19b887605f0e6a415ce Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Thu, 11 Apr 2019 19:34:11 +0300 Subject: [PATCH 04/16] feat: improve cleanup service Currently it is possible just to add new actions to be executed in the cleanup. However, in some cases, we want to add action and remove it after that. Also, we need to delete some lock files when CLI is not using them anymore. So, introduce new API in the cleanup service that allow all of these actions. --- lib/definitions/cleanup-service.d.ts | 25 ++++++++- .../cleanup-process-definitions.d.ts | 8 +++ lib/detached-processes/cleanup-process.ts | 54 +++++++++++++++++-- .../detached-process-enums.d.ts | 19 +++++++ lib/services/cleanup-service.ts | 15 ++++++ 5 files changed, 117 insertions(+), 4 deletions(-) diff --git a/lib/definitions/cleanup-service.d.ts b/lib/definitions/cleanup-service.d.ts index 8817ea0495..cae0837bd9 100644 --- a/lib/definitions/cleanup-service.d.ts +++ b/lib/definitions/cleanup-service.d.ts @@ -10,11 +10,34 @@ interface ICleanupService extends IShouldDispose, IDisposable { */ addCleanupAction(action: ICleanupAction): Promise; + /** + * Remove action to be executed when CLI process exits. + * NOTE: The action should be added in the action list by calling `addCleanupAction` first. + * @param {ICleanupAction} action The action that should be removed from cleanup execution, including command and args. + * @returns {Promise} + */ + removeCleanupAction(action: ICleanupAction): Promise + /** * Sets the file in which the cleanup process will write its logs. * This method must be called before starting the cleanup process, i.e. when CLI is initialized. * @param {string} filePath Path to file where the logs will be written. The logs are appended to the passed file. * @returns {void} */ - setCleanupLogFile(filePath: string): void + setCleanupLogFile(filePath: string): void; + + /** + * Adds file/dir to be deleted once CLI process exits. + * @param {string} filePath Path to file/directory to be deleted. + * @returns {Promise} + */ + addCleanupDeleteAction(filePath: string): Promise; + + /** + * Removes file/dir from the list of files to be deleted once CLI process exits. + * NOTE: The file should be first added with `addCleanupDeleteAction` + * @param {string} filePath Path to file/directory to be removed from the list of files to be deleted. + * @returns {Promise} + */ + removeCleanupDeleteAction(filePath: string): Promise; } diff --git a/lib/detached-processes/cleanup-process-definitions.d.ts b/lib/detached-processes/cleanup-process-definitions.d.ts index 49968583b3..578d0d01a4 100644 --- a/lib/detached-processes/cleanup-process-definitions.d.ts +++ b/lib/detached-processes/cleanup-process-definitions.d.ts @@ -19,10 +19,18 @@ interface ICleanupProcessMessage { * Type of the action */ actionType: CleanupProcessMessageType; +} +interface ICleanupActionMessage extends ICleanupProcessMessage { /** * Describes the action that must be executed */ action: ICleanupAction; } +interface ICleanupDeleteActionMessage extends ICleanupProcessMessage { + /** + * Path to file/directory to be deleted. + */ + filePath: string; +} diff --git a/lib/detached-processes/cleanup-process.ts b/lib/detached-processes/cleanup-process.ts index 1490e21539..5d2b65f3c8 100644 --- a/lib/detached-processes/cleanup-process.ts +++ b/lib/detached-processes/cleanup-process.ts @@ -1,6 +1,8 @@ // NOTE: This file is used to clean up resources used by CLI, when the CLI is killed. // The instances here are not shared with the ones in main CLI process. import * as fs from "fs"; +import * as path from "path"; +import * as shelljs from "shelljs"; import { FileLogService } from "./file-log-service"; const pathToBootstrap = process.argv[2]; @@ -16,6 +18,7 @@ const fileLogService = $injector.resolve(FileLogService, { logF fileLogService.logData({ message: "Initializing Cleanup process." }); const actionsToExecute: ICleanupAction[] = []; +const filesToDelete: string[] = []; const executeCleanup = async () => { const $childProcess = $injector.resolve("childProcess"); @@ -31,16 +34,52 @@ const executeCleanup = async () => { } } + if (filesToDelete.length) { + fileLogService.logData({ message: `Deleting files ${filesToDelete.join(" ")}` }); + shelljs.rm("-Rf", filesToDelete); + } + fileLogService.logData({ message: `cleanup-process finished` }); process.exit(); }; const addCleanupAction = (newAction: ICleanupAction): void => { - if (!_.some(actionsToExecute, currentAction => _.isEqual(currentAction, newAction))) { + if (_.some(actionsToExecute, currentAction => _.isEqual(currentAction, newAction))) { + fileLogService.logData({ message: `cleanup-process will not add action for execution as it has been added already: ${JSON.stringify(newAction)}` }); + } else { fileLogService.logData({ message: `cleanup-process added action for execution: ${JSON.stringify(newAction)}` }); actionsToExecute.push(newAction); + } +}; + +const removeCleanupAction = (actionToRemove: ICleanupAction): void => { + if (_.some(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove))) { + _.remove(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove)) + fileLogService.logData({ message: `cleanup-process removed action for execution: ${JSON.stringify(actionToRemove)}` }); } else { - fileLogService.logData({ message: `cleanup-process will not add action for execution as it has been added already: ${JSON.stringify(newAction)}` }); + fileLogService.logData({ message: `cleanup-process cannot remove action for execution as it has note been added before: ${JSON.stringify(actionToRemove)}` }); + } +}; + +const addDeleteAction = (filePath: string): void => { + const fullPath = path.resolve(filePath); + + if (_.some(filesToDelete, f => f === fullPath)) { + fileLogService.logData({ message: `cleanup-process will not add ${fullPath} for deletion as it has been added already` }); + } else { + filesToDelete.push(fullPath); + fileLogService.logData({ message: `cleanup-process added ${fullPath} for deletion` }); + } +}; + +const removeDeleteAction = (filePath: string): void => { + const fullPath = path.resolve(filePath); + + if (_.some(filesToDelete, f => f === fullPath)) { + _.remove(filesToDelete, f => f === fullPath); + fileLogService.logData({ message: `cleanup-process removed ${fullPath} from the list of files for deletion.` }); + } else { + fileLogService.logData({ message: `cleanup-process cannot remove ${fullPath} for deletion as no such entry is found in the files marked for deletion` }); } }; @@ -49,7 +88,16 @@ process.on("message", async (cleanupProcessMessage: ICleanupProcessMessage) => { switch (cleanupProcessMessage.actionType) { case CleanupProcessMessageType.AddCleanAction: - addCleanupAction(cleanupProcessMessage.action); + addCleanupAction((cleanupProcessMessage).action); + break; + case CleanupProcessMessageType.RemoveCleanAction: + removeCleanupAction((cleanupProcessMessage).action); + break; + case CleanupProcessMessageType.AddDeleteAction: + addDeleteAction((cleanupProcessMessage).filePath); + break; + case CleanupProcessMessageType.RemoveDeleteAction: + removeDeleteAction((cleanupProcessMessage).filePath); break; default: fileLogService.logData({ message: `Unable to handle message of type ${cleanupProcessMessage.actionType}. Full message is ${JSON.stringify(cleanupProcessMessage)}`, type: FileLogMessageType.Error }); diff --git a/lib/detached-processes/detached-process-enums.d.ts b/lib/detached-processes/detached-process-enums.d.ts index 4e5974bfd5..2ee9c41cb9 100644 --- a/lib/detached-processes/detached-process-enums.d.ts +++ b/lib/detached-processes/detached-process-enums.d.ts @@ -24,5 +24,24 @@ declare const enum FileLogMessageType { } declare const enum CleanupProcessMessageType { + /** + * This type of action defines that cleanup procedure should execute specific action. + */ AddCleanAction = "AddCleanAction", + + /** + * This type of action defines that cleanup procedure should not execute previously defined cleanup action. + */ + RemoveCleanAction = "RemoveCleanAction", + + /** + * This type of action defines that cleanup procedure should delete specified files. + */ + AddDeleteAction = "AddDeleteAction", + + /** + * This type of action defines the cleanup procedure should not delete previously specified file. + */ + RemoveDeleteAction = "RemoveDeleteAction", + } diff --git a/lib/services/cleanup-service.ts b/lib/services/cleanup-service.ts index e44eb518eb..2be9be3d08 100644 --- a/lib/services/cleanup-service.ts +++ b/lib/services/cleanup-service.ts @@ -20,6 +20,21 @@ export class CleanupService implements ICleanupService { cleanupProcess.send({ actionType: CleanupProcessMessageType.AddCleanAction, action }); } + public async removeCleanupAction(action: ICleanupAction): Promise { + const cleanupProcess = await this.getCleanupProcess(); + cleanupProcess.send({ actionType: CleanupProcessMessageType.RemoveCleanAction, action }); + } + + public async addCleanupDeleteAction(filePath: string): Promise { + const cleanupProcess = await this.getCleanupProcess(); + cleanupProcess.send({ actionType: CleanupProcessMessageType.AddDeleteAction, filePath }); + } + + public async removeCleanupDeleteAction(filePath: string): Promise { + const cleanupProcess = await this.getCleanupProcess(); + cleanupProcess.send({ actionType: CleanupProcessMessageType.RemoveDeleteAction, filePath }); + } + @exported("cleanupService") public setCleanupLogFile(filePath: string): void { this.pathToCleanupLogFile = filePath; From 0ce13f5bf48ab4054721621e2ac981a32a4d3173 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Thu, 11 Apr 2019 19:44:16 +0300 Subject: [PATCH 05/16] fix: replace processService with cleanupService Replace processService with cleanupService where appropriate. The cleanupService allows execution of code after CLI has died. `processService` was used to handle some events, but it caused more issues than benefits. So remove it from the codebase. --- lib/common/bootstrap.ts | 1 - lib/common/declarations.d.ts | 5 - lib/common/http-client.ts | 6 -- lib/common/mobile/android/logcat-helper.ts | 5 - .../ios/device/ios-device-operations.ts | 7 +- lib/common/mobile/ios/device/ios-device.ts | 1 - lib/common/mobile/ios/ios-device-base.ts | 3 - .../ios-simulator-application-manager.ts | 8 -- .../ios/simulator/ios-simulator-device.ts | 3 +- .../simulator/ios-simulator-log-provider.ts | 3 - .../mobile-core/android-process-service.ts | 26 ++---- lib/common/services/lock-service.ts | 22 ++--- lib/common/services/process-service.ts | 43 --------- .../test/unit-tests/analytics-service.ts | 4 - .../mobile/android/logcat-helper.ts | 5 - .../test/unit-tests/mobile/devices-service.ts | 3 - .../mobile/ios-simulator-discovery.ts | 3 +- lib/common/test/unit-tests/process-service.ts | 93 ------------------- lib/common/test/unit-tests/stubs.ts | 2 +- lib/common/timers.ts | 13 +-- lib/definitions/lock-service.d.ts | 4 +- lib/detached-processes/cleanup-process.ts | 2 +- lib/services/android-device-debug-service.ts | 10 +- lib/services/hmr-status-service.ts | 9 -- lib/services/ios-device-debug-service.ts | 2 - ...android-device-livesync-sockets-service.ts | 18 +++- .../livesync/android-livesync-tool.ts | 24 ----- .../livesync/ios-device-livesync-service.ts | 3 - lib/services/livesync/livesync-service.ts | 10 -- test/ios-project-service.ts | 1 - test/services/analytics/analytics-service.ts | 3 - test/services/android-device-debug-service.ts | 8 +- test/services/ios-debugger-port-service.ts | 3 - test/services/ios-device-debug-service.ts | 7 +- test/services/livesync-service.ts | 3 - .../livesync/android-livesync-tool.ts | 3 +- test/stubs.ts | 8 -- 37 files changed, 53 insertions(+), 321 deletions(-) delete mode 100644 lib/common/services/process-service.ts delete mode 100644 lib/common/test/unit-tests/process-service.ts diff --git a/lib/common/bootstrap.ts b/lib/common/bootstrap.ts index 78370b281a..8bb6a9d31e 100644 --- a/lib/common/bootstrap.ts +++ b/lib/common/bootstrap.ts @@ -85,7 +85,6 @@ $injector.require("iOSEmulatorServices", "./mobile/ios/simulator/ios-emulator-se $injector.require("wp8EmulatorServices", "./mobile/wp8/wp8-emulator-services"); $injector.require("autoCompletionService", "./services/auto-completion-service"); -$injector.require("processService", "./services/process-service"); $injector.requirePublic("settingsService", "./services/settings-service"); $injector.require("opener", "./opener"); $injector.require("microTemplateService", "./services/micro-templating-service"); diff --git a/lib/common/declarations.d.ts b/lib/common/declarations.d.ts index ab90c6f884..e65cae28e4 100644 --- a/lib/common/declarations.d.ts +++ b/lib/common/declarations.d.ts @@ -1466,11 +1466,6 @@ interface INet { waitForPortToListen(waitForPortListenData: IWaitForPortListenData): Promise; } -interface IProcessService { - listenersCount: number; - attachToProcessExitSignals(context: any, callback: () => void): void; -} - interface IDependencyInformation { name: string; version?: string; diff --git a/lib/common/http-client.ts b/lib/common/http-client.ts index 9d4db22847..66e6ca647e 100644 --- a/lib/common/http-client.ts +++ b/lib/common/http-client.ts @@ -19,15 +19,9 @@ export class HttpClient implements Server.IHttpClient { private cleanupData: ICleanupRequestData[]; constructor(private $logger: ILogger, - private $processService: IProcessService, private $proxyService: IProxyService, private $staticConfig: Config.IStaticConfig) { this.cleanupData = []; - this.$processService.attachToProcessExitSignals(this, () => { - this.cleanupData.forEach(d => { - this.cleanupAfterRequest(d); - }); - }); } public async httpRequest(options: any, proxySettings?: IProxySettings): Promise { diff --git a/lib/common/mobile/android/logcat-helper.ts b/lib/common/mobile/android/logcat-helper.ts index d662733afe..9c7fb9b05c 100644 --- a/lib/common/mobile/android/logcat-helper.ts +++ b/lib/common/mobile/android/logcat-helper.ts @@ -16,7 +16,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper { private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $logger: ILogger, private $injector: IInjector, - private $processService: IProcessService, private $devicesService: Mobile.IDevicesService) { this.mapDevicesLoggingData = Object.create(null); } @@ -53,8 +52,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper { const lineText = line.toString(); this.$deviceLogProvider.logData(lineText, this.$devicePlatformsConstants.Android, deviceIdentifier); }); - - this.$processService.attachToProcessExitSignals(this, logcatStream.kill); } } @@ -72,8 +69,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper { logcatDumpStream.removeAllListeners(); lineStream.removeAllListeners(); }); - - this.$processService.attachToProcessExitSignals(this, logcatDumpStream.kill); } /** diff --git a/lib/common/mobile/ios/device/ios-device-operations.ts b/lib/common/mobile/ios/device/ios-device-operations.ts index f2d90eafce..1676b1ca09 100644 --- a/lib/common/mobile/ios/device/ios-device-operations.ts +++ b/lib/common/mobile/ios/device/ios-device-operations.ts @@ -9,16 +9,11 @@ export class IOSDeviceOperations extends EventEmitter implements IIOSDeviceOpera public shouldDispose: boolean; private deviceLib: IOSDeviceLib.IOSDeviceLib; - constructor(private $logger: ILogger, - private $processService: IProcessService) { + constructor(private $logger: ILogger) { super(); this.isInitialized = false; this.shouldDispose = true; - this.$processService.attachToProcessExitSignals(this, () => { - this.setShouldDispose(true); - this.dispose(); - }); } public async install(ipaPath: string, deviceIdentifiers: string[], errorHandler: DeviceOperationErrorHandler): Promise { diff --git a/lib/common/mobile/ios/device/ios-device.ts b/lib/common/mobile/ios/device/ios-device.ts index f51ba972a5..e10a589114 100644 --- a/lib/common/mobile/ios/device/ios-device.ts +++ b/lib/common/mobile/ios/device/ios-device.ts @@ -20,7 +20,6 @@ export class IOSDevice extends IOSDeviceBase { protected $deviceLogProvider: Mobile.IDeviceLogProvider, protected $lockService: ILockService, private $iOSSocketRequestExecutor: IiOSSocketRequestExecutor, - protected $processService: IProcessService, private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $iOSDeviceProductNameMapper: Mobile.IiOSDeviceProductNameMapper, private $iosDeviceOperations: IIOSDeviceOperations, diff --git a/lib/common/mobile/ios/ios-device-base.ts b/lib/common/mobile/ios/ios-device-base.ts index d9e6d11ae6..62be589cfc 100644 --- a/lib/common/mobile/ios/ios-device-base.ts +++ b/lib/common/mobile/ios/ios-device-base.ts @@ -6,7 +6,6 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice { protected abstract $errors: IErrors; protected abstract $deviceLogProvider: Mobile.IDeviceLogProvider; protected abstract $iOSDebuggerPortService: IIOSDebuggerPortService; - protected abstract $processService: IProcessService; protected abstract $lockService: ILockService; abstract deviceInfo: Mobile.IDeviceInfo; abstract applicationManager: Mobile.IDeviceApplicationManager; @@ -33,8 +32,6 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice { this.cachedSockets[appId].on("close", async () => { await this.destroyDebugSocket(appId); }); - - this.$processService.attachToProcessExitSignals(this, () => this.destroyDebugSocket(appId)); } return this.cachedSockets[appId]; diff --git a/lib/common/mobile/ios/simulator/ios-simulator-application-manager.ts b/lib/common/mobile/ios/simulator/ios-simulator-application-manager.ts index d0d70022df..6f1cb80593 100644 --- a/lib/common/mobile/ios/simulator/ios-simulator-application-manager.ts +++ b/lib/common/mobile/ios/simulator/ios-simulator-application-manager.ts @@ -15,18 +15,10 @@ export class IOSSimulatorApplicationManager extends ApplicationManagerBase { private device: Mobile.IiOSDevice, private $options: IOptions, private $fs: IFileSystem, - private $processService: IProcessService, private $deviceLogProvider: Mobile.IDeviceLogProvider, $logger: ILogger, $hooksService: IHooksService) { super($logger, $hooksService); - this.$processService.attachToProcessExitSignals(this, () => { - for (const appId in this._lldbProcesses) { - /* tslint:disable:no-floating-promises */ - this.detachNativeDebugger(appId); - /* tslint:enable:no-floating-promises */ - } - }); } public async getInstalledApplications(): Promise { diff --git a/lib/common/mobile/ios/simulator/ios-simulator-device.ts b/lib/common/mobile/ios/simulator/ios-simulator-device.ts index b05dde67b6..50a178d4f3 100644 --- a/lib/common/mobile/ios/simulator/ios-simulator-device.ts +++ b/lib/common/mobile/ios/simulator/ios-simulator-device.ts @@ -22,8 +22,7 @@ export class IOSSimulator extends IOSDeviceBase implements Mobile.IiOSDevice { private $iOSEmulatorServices: Mobile.IiOSSimulatorService, private $iOSNotification: IiOSNotification, private $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider, - private $logger: ILogger, - protected $processService: IProcessService) { + private $logger: ILogger) { super(); this.applicationManager = this.$injector.resolve(applicationManagerPath.IOSSimulatorApplicationManager, { iosSim: this.$iOSSimResolver.iOSSim, device: this }); this.fileSystem = this.$injector.resolve(fileSystemPath.IOSSimulatorFileSystem, { iosSim: this.$iOSSimResolver.iOSSim }); diff --git a/lib/common/mobile/ios/simulator/ios-simulator-log-provider.ts b/lib/common/mobile/ios/simulator/ios-simulator-log-provider.ts index 79464bd8bc..a01fcee0ed 100644 --- a/lib/common/mobile/ios/simulator/ios-simulator-log-provider.ts +++ b/lib/common/mobile/ios/simulator/ios-simulator-log-provider.ts @@ -8,7 +8,6 @@ export class IOSSimulatorLogProvider extends EventEmitter implements Mobile.IiOS constructor(private $iOSSimResolver: Mobile.IiOSSimResolver, private $logger: ILogger, - private $processService: IProcessService, private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $deviceLogProvider: Mobile.IDeviceLogProvider) { super(); @@ -47,8 +46,6 @@ export class IOSSimulatorLogProvider extends EventEmitter implements Mobile.IiOS deviceLogChildProcess.stderr.on("data", action.bind(this)); } - this.$processService.attachToProcessExitSignals(this, deviceLogChildProcess.kill); - this.simulatorsLoggingEnabled[deviceId] = true; this.simulatorsLogProcess[deviceId] = deviceLogChildProcess; } diff --git a/lib/common/mobile/mobile-core/android-process-service.ts b/lib/common/mobile/mobile-core/android-process-service.ts index b2fa281035..2abc66d872 100644 --- a/lib/common/mobile/mobile-core/android-process-service.ts +++ b/lib/common/mobile/mobile-core/android-process-service.ts @@ -1,8 +1,7 @@ import { EOL } from "os"; -import * as shelljs from "shelljs"; import { DeviceAndroidDebugBridge } from "../android/device-android-debug-bridge"; import { TARGET_FRAMEWORK_IDENTIFIERS } from "../../constants"; -import { exported, cache } from "../../decorators"; +import { exported } from "../../decorators"; export class AndroidProcessService implements Mobile.IAndroidProcessService { private _devicesAdbs: IDictionary; @@ -11,7 +10,8 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { constructor(private $errors: IErrors, private $injector: IInjector, private $net: INet, - private $processService: IProcessService) { + private $cleanupService: ICleanupService, + private $staticConfig: IStaticConfig) { this._devicesAdbs = {}; this._forwardedLocalPorts = {}; } @@ -22,8 +22,6 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { } public async mapAbstractToTcpPort(deviceIdentifier: string, appIdentifier: string, framework: string): Promise { - this.tryAttachToProcessExitSignals(); - const adb = await this.setupForPortForwarding({ deviceIdentifier, appIdentifier }); const processId = (await this.getProcessIds(adb, [appIdentifier]))[appIdentifier]; @@ -120,16 +118,16 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { if (!localPort) { localPort = await this.$net.getFreePort(); - await adb.executeCommand(["forward", `tcp:${localPort}`, portForwardInputData.abstractPort], {deviceIdentifier: portForwardInputData.deviceIdentifier}); + await adb.executeCommand(["forward", `tcp:${localPort}`, portForwardInputData.abstractPort], { deviceIdentifier: portForwardInputData.deviceIdentifier }); } this._forwardedLocalPorts[portForwardInputData.deviceIdentifier] = localPort; + // TODO: use correct path to adb + await this.$cleanupService.addCleanupAction({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", portForwardInputData.deviceIdentifier, "forward", "--remove", `tcp:${localPort}`] }); return localPort && +localPort; } private async setupForPortForwarding(portForwardInputData?: Mobile.IPortForwardDataBase): Promise { - this.tryAttachToProcessExitSignals(); - const adb = this.getAdb(portForwardInputData.deviceIdentifier); return adb; @@ -279,18 +277,6 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { return result; } - - @cache() - private tryAttachToProcessExitSignals(): void { - this.$processService.attachToProcessExitSignals(this, () => { - _.each(this._forwardedLocalPorts, (port: number, deviceIdentifier: string) => { - // We need to use shelljs here instead of $adb because listener functions of exit, SIGINT and SIGTERM must only perform synchronous operations. - // The Node.js process will exit immediately after calling the 'exit' event listeners causing any additional work still queued in the event loop to be abandoned. - // See the official documentation for more information and examples - https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_exit. - shelljs.exec(`adb -s ${deviceIdentifier} forward --remove tcp:${port}`); - }); - }); - } } $injector.register("androidProcessService", AndroidProcessService); diff --git a/lib/common/services/lock-service.ts b/lib/common/services/lock-service.ts index 530fe2e12e..5cb057ecf7 100644 --- a/lib/common/services/lock-service.ts +++ b/lib/common/services/lock-service.ts @@ -4,7 +4,6 @@ import { cache } from "../decorators"; import { getHash } from "../helpers"; export class LockService implements ILockService { - private currentlyLockedFiles: string[] = []; @cache() private get defaultLockFilePath(): string { @@ -28,14 +27,7 @@ export class LockService implements ILockService { constructor(private $fs: IFileSystem, private $settingsService: ISettingsService, - private $processService: IProcessService) { - this.$processService.attachToProcessExitSignals(this, () => { - const locksToRemove = _.clone(this.currentlyLockedFiles); - for (const lockToRemove of locksToRemove) { - lockfile.unlockSync(lockToRemove); - this.cleanLock(lockToRemove); - } - }); + private $cleanupService: ICleanupService) { } public async executeActionWithLock(action: () => Promise, lockFilePath?: string, lockOpts?: ILockOptions): Promise { @@ -51,29 +43,29 @@ export class LockService implements ILockService { public async lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<() => void> { const { filePath, fileOpts } = this.getLockFileSettings(lockFilePath, lockOpts); - this.currentlyLockedFiles.push(filePath); + await this.$cleanupService.addCleanupDeleteAction(filePath); this.$fs.writeFile(filePath, ""); try { const releaseFunc = await lockfile.lock(filePath, fileOpts); return async () => { await releaseFunc(); - this.cleanLock(filePath); + await this.cleanLock(filePath); }; } catch (err) { throw new Error(`Timeout while waiting for lock "${filePath}"`); } } - public unlock(lockFilePath?: string): void { + public async unlock(lockFilePath?: string): Promise { const { filePath } = this.getLockFileSettings(lockFilePath); lockfile.unlockSync(filePath); - this.cleanLock(filePath); + await this.cleanLock(filePath); } - private cleanLock(lockPath: string): void { - _.remove(this.currentlyLockedFiles, e => e === lockPath); + private async cleanLock(lockPath: string): Promise { this.$fs.deleteFile(lockPath); + await this.$cleanupService.removeCleanupDeleteAction(lockPath); } private getLockFileSettings(filePath?: string, fileOpts?: ILockOptions): { filePath: string, fileOpts: ILockOptions } { diff --git a/lib/common/services/process-service.ts b/lib/common/services/process-service.ts deleted file mode 100644 index ff7fb4a540..0000000000 --- a/lib/common/services/process-service.ts +++ /dev/null @@ -1,43 +0,0 @@ -export class ProcessService implements IProcessService { - private static PROCESS_EXIT_SIGNALS = ["exit", "SIGTERM"]; - private _listeners: IListener[]; - - public get listenersCount(): number { - return this._listeners.length; - } - - constructor() { - this._listeners = []; - } - - public attachToProcessExitSignals(context: any, callback: () => void): void { - const callbackToString = callback.toString(); - - if (this._listeners.length === 0) { - _.each(ProcessService.PROCESS_EXIT_SIGNALS, (signal: NodeJS.Signals) => { - process.on(signal, () => this.executeAllCallbacks.apply(this)); - }); - } - - if (!_.some(this._listeners, (listener: IListener) => context === listener.context && callbackToString === listener.callback.toString())) { - this._listeners.push({ context, callback }); - } - } - - private executeAllCallbacks(): void { - _.each(this._listeners, (listener: IListener) => { - try { - listener.callback.apply(listener.context); - } catch (err) { - // ignore the error and let other handlers to be called. - } - }); - } -} - -interface IListener { - context: any; - callback: () => void; -} - -$injector.register("processService", ProcessService); diff --git a/lib/common/test/unit-tests/analytics-service.ts b/lib/common/test/unit-tests/analytics-service.ts index 356c6d3e53..46c84f3717 100644 --- a/lib/common/test/unit-tests/analytics-service.ts +++ b/lib/common/test/unit-tests/analytics-service.ts @@ -89,10 +89,6 @@ function createTestInjector(testScenario: ITestScenario): IInjector { return testScenario.isInteractive; }; - testInjector.register("processService", { - attachToProcessExitSignals: (context: any, callback: () => void): void => (undefined) - }); - testInjector.register("childProcess", {}); testInjector.register("projectDataService", {}); testInjector.register("mobileHelper", {}); diff --git a/lib/common/test/unit-tests/mobile/android/logcat-helper.ts b/lib/common/test/unit-tests/mobile/android/logcat-helper.ts index 4e8127fde8..561a7b1114 100644 --- a/lib/common/test/unit-tests/mobile/android/logcat-helper.ts +++ b/lib/common/test/unit-tests/mobile/android/logcat-helper.ts @@ -60,11 +60,6 @@ function createTestInjector(): IInjector { } }); injector.register("devicePlatformsConstants", { Android: "Android" }); - injector.register("processService", { - attachToProcessExitSignals(context: any, callback: () => void): void { - //left blank intentionally because of lint - }, - }); injector.register("deviceLogProvider", { logData(line: string, platform: string, deviceIdentifier: string): void { //left blank intentionally because of lint diff --git a/lib/common/test/unit-tests/mobile/devices-service.ts b/lib/common/test/unit-tests/mobile/devices-service.ts index 37bd61f4bf..984156c311 100644 --- a/lib/common/test/unit-tests/mobile/devices-service.ts +++ b/lib/common/test/unit-tests/mobile/devices-service.ts @@ -160,9 +160,6 @@ function createTestInjector(): IInjector { testInjector.register("iOSEmulatorServices", IOSEmulatorServices); testInjector.register("messages", Messages); testInjector.register("prompter", {}); - testInjector.register("processService", { - attachToProcessExitSignals: (context: any, callback: () => Promise) => { /* no implementation required */ } - }); testInjector.register("mobileHelper", { platformNames: ["ios", "android"], diff --git a/lib/common/test/unit-tests/mobile/ios-simulator-discovery.ts b/lib/common/test/unit-tests/mobile/ios-simulator-discovery.ts index e962305cfe..245e884e15 100644 --- a/lib/common/test/unit-tests/mobile/ios-simulator-discovery.ts +++ b/lib/common/test/unit-tests/mobile/ios-simulator-discovery.ts @@ -5,7 +5,7 @@ import { assert } from "chai"; import { DeviceDiscoveryEventNames, CONNECTED_STATUS } from "../../../constants"; import { DevicePlatformsConstants } from "../../../mobile/device-platforms-constants"; import { ErrorsStub, CommonLoggerStub, HooksServiceStub, LockServiceStub } from "../stubs"; -import { FileSystemStub, ChildProcessStub, ProcessServiceStub } from "../../../../../test/stubs"; +import { FileSystemStub, ChildProcessStub } from "../../../../../test/stubs"; let currentlyRunningSimulators: Mobile.IiSimDevice[]; @@ -41,7 +41,6 @@ function createTestInjector(): IInjector { injector.register("deviceLogProvider", {}); injector.register("iOSEmulatorServices", {}); injector.register("iOSNotification", {}); - injector.register("processService", ProcessServiceStub); injector.register("options", {}); injector.register("hooksService", HooksServiceStub); injector.register("logger", CommonLoggerStub); diff --git a/lib/common/test/unit-tests/process-service.ts b/lib/common/test/unit-tests/process-service.ts deleted file mode 100644 index 18d382c8e9..0000000000 --- a/lib/common/test/unit-tests/process-service.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { Yok } from "../../yok"; -import { ProcessService } from "../../services/process-service"; -import { assert } from "chai"; - -const processExitSignals = ["exit", "SIGTERM"]; -const emptyFunction = () => { /* no implementation required */ }; -function createTestInjector(): IInjector { - const testInjector = new Yok(); - - testInjector.register("processService", ProcessService); - - return testInjector; -} - -describe("Process service", () => { - let testInjector: IInjector; - let $processService: IProcessService; - - beforeEach(() => { - testInjector = createTestInjector(); - $processService = testInjector.resolve("processService"); - }); - - it("should not add only one listener for the exit, SIGIN and SIGTERM events.", () => { - $processService.attachToProcessExitSignals({}, emptyFunction); - $processService.attachToProcessExitSignals({}, emptyFunction); - - _.each(processExitSignals, (signal: NodeJS.Signals) => { - // We need to search only for our listener because each exit signal have different listeners added to it. - const actualListeners = _.filter(process.listeners(signal), (listener: Function) => listener.toString().indexOf("executeAllCallbacks") >= 0); - assert.deepEqual(actualListeners.length, 1); - }); - }); - - it("should add listener with context only once if there already is callback with the same context.", () => { - const context = { test: "test" }; - const listener = () => 42; - - $processService.attachToProcessExitSignals(context, listener); - $processService.attachToProcessExitSignals(context, listener); - - assert.deepEqual($processService.listenersCount, 1); - }); - - it("should add two different listeners for one context.", () => { - const context = { test: "test" }; - const numberListener = () => 42; - const booleanListener = () => true; - - $processService.attachToProcessExitSignals(context, numberListener); - $processService.attachToProcessExitSignals(context, booleanListener); - - assert.deepEqual($processService.listenersCount, 2); - }); - - it("should add one listener with different context twice.", () => { - const listener = () => 42; - - $processService.attachToProcessExitSignals({}, listener); - $processService.attachToProcessExitSignals({}, listener); - - assert.deepEqual($processService.listenersCount, 2); - }); - - it("should execute all attached listeners.", () => { - let hasCalledFirstListener = false; - let hasCalledSecondListener = false; - let hasCalledThirdListener = false; - - const firstListener = () => { - hasCalledFirstListener = true; - }; - - const secondListener = () => { - hasCalledSecondListener = true; - }; - - const thirdListener = () => { - hasCalledThirdListener = true; - }; - - $processService.attachToProcessExitSignals({}, firstListener); - $processService.attachToProcessExitSignals({}, secondListener); - $processService.attachToProcessExitSignals({}, thirdListener); - - // Do not use exit or SIGINT because the tests after this one will not be executed. - global.process.emit("SIGTERM"); - - assert.isTrue(hasCalledFirstListener); - assert.isTrue(hasCalledSecondListener); - assert.isTrue(hasCalledThirdListener); - }); -}); diff --git a/lib/common/test/unit-tests/stubs.ts b/lib/common/test/unit-tests/stubs.ts index 284c2051e2..ba0609a2b8 100644 --- a/lib/common/test/unit-tests/stubs.ts +++ b/lib/common/test/unit-tests/stubs.ts @@ -8,7 +8,7 @@ export class LockServiceStub implements ILockService { return () => { }; } - public unlock(lockFilePath?: string): void { + public async unlock(lockFilePath?: string): Promise { } public async executeActionWithLock(action: () => Promise, lockFilePath?: string, lockOpts?: ILockOptions): Promise { diff --git a/lib/common/timers.ts b/lib/common/timers.ts index 2325eb7481..47d52a58ea 100644 --- a/lib/common/timers.ts +++ b/lib/common/timers.ts @@ -1,24 +1,13 @@ +// TODO: Delete this service, it does not bring any value export class Timers implements ITimers { - private pendingIntervalIds: NodeJS.Timer[] = []; - - constructor(private $processService: IProcessService) { - this.$processService.attachToProcessExitSignals(this, () => { - _.each(this.pendingIntervalIds, clearInterval); - }); - } - public setInterval(callback: (...args: any[]) => void, ms: number, ...args: any[]): NodeJS.Timer { const timer = setInterval(callback, ms, args); - this.pendingIntervalIds.push(timer); - return timer; } public clearInterval(intervalId: NodeJS.Timer): void { clearInterval(intervalId); - - _.remove(this.pendingIntervalIds, id => id === intervalId); } } diff --git a/lib/definitions/lock-service.d.ts b/lib/definitions/lock-service.d.ts index 2bc892da00..bf80586d58 100644 --- a/lib/definitions/lock-service.d.ts +++ b/lib/definitions/lock-service.d.ts @@ -34,6 +34,6 @@ declare global { * Resolves the specified file lock. Whenever possible you should use the `release` function instead. * @param {string} lockFilePath Path to lock file that has to be unlocked. Defaults to `/lockfile.lock` */ - unlock(lockFilePath?: string): void + unlock(lockFilePath?: string): Promise } -} \ No newline at end of file +} diff --git a/lib/detached-processes/cleanup-process.ts b/lib/detached-processes/cleanup-process.ts index 5d2b65f3c8..aa66a6da62 100644 --- a/lib/detached-processes/cleanup-process.ts +++ b/lib/detached-processes/cleanup-process.ts @@ -54,7 +54,7 @@ const addCleanupAction = (newAction: ICleanupAction): void => { const removeCleanupAction = (actionToRemove: ICleanupAction): void => { if (_.some(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove))) { - _.remove(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove)) + _.remove(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove)); fileLogService.logData({ message: `cleanup-process removed action for execution: ${JSON.stringify(actionToRemove)}` }); } else { fileLogService.logData({ message: `cleanup-process cannot remove action for execution as it has note been added before: ${JSON.stringify(actionToRemove)}` }); diff --git a/lib/services/android-device-debug-service.ts b/lib/services/android-device-debug-service.ts index b764e5bd11..017fcb8cd5 100644 --- a/lib/services/android-device-debug-service.ts +++ b/lib/services/android-device-debug-service.ts @@ -17,7 +17,9 @@ export class AndroidDeviceDebugService extends DebugServiceBase implements IDevi private $logger: ILogger, private $androidProcessService: Mobile.IAndroidProcessService, private $net: INet, - private $deviceLogProvider: Mobile.IDeviceLogProvider) { + private $cleanupService: ICleanupService, + private $deviceLogProvider: Mobile.IDeviceLogProvider, + private $staticConfig: IStaticConfig) { super(device, $devicesService); this.deviceIdentifier = device.deviceInfo.identifier; @@ -50,6 +52,7 @@ export class AndroidDeviceDebugService extends DebugServiceBase implements IDevi return this.device.adb.executeCommand(["forward", "--remove", `tcp:${port}`]); } + // TODO: Remove this method and reuse logic from androidProcessService private async getForwardedDebugPort(deviceId: string, packageName: string): Promise { let port = -1; const forwardsResult = await this.device.adb.executeCommand(["forward", "--list"]); @@ -68,11 +71,14 @@ export class AndroidDeviceDebugService extends DebugServiceBase implements IDevi await this.unixSocketForward(port, `${unixSocketName}`); } + await this.$cleanupService.addCleanupAction({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", deviceId, "forward", "--remove", `tcp:${port}`] }); + return port; } + // TODO: Remove this method and reuse logic from androidProcessService private async unixSocketForward(local: number, remote: string): Promise { - return this.device.adb.executeCommand(["forward", `tcp:${local}`, `localabstract:${remote}`]); + await this.device.adb.executeCommand(["forward", `tcp:${local}`, `localabstract:${remote}`]); } @performanceLog() diff --git a/lib/services/hmr-status-service.ts b/lib/services/hmr-status-service.ts index 60e65b9fbc..689ea180a5 100644 --- a/lib/services/hmr-status-service.ts +++ b/lib/services/hmr-status-service.ts @@ -10,10 +10,8 @@ export class HmrStatusService implements IHmrStatusService { private intervals: IDictionary = {}; constructor(private $logParserService: ILogParserService, - private $processService: IProcessService, private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $logger: ILogger) { - this.$processService.attachToProcessExitSignals(this, this.dispose); } public getHmrStatus(deviceId: string, operationHash: string): Promise { @@ -106,13 +104,6 @@ export class HmrStatusService implements IHmrStatusService { this.hashOperationStatuses[key].status = status; } - - private dispose() { - _.forEach(this.intervals, (value, key) => { - clearInterval(value); - this.intervals[key] = null; - }); - } } $injector.register("hmrStatusService", HmrStatusService); diff --git a/lib/services/ios-device-debug-service.ts b/lib/services/ios-device-debug-service.ts index 0479006dba..16b34c3207 100644 --- a/lib/services/ios-device-debug-service.ts +++ b/lib/services/ios-device-debug-service.ts @@ -17,12 +17,10 @@ export class IOSDeviceDebugService extends DebugServiceBase implements IDeviceDe private $logger: ILogger, private $errors: IErrors, private $packageInstallationManager: IPackageInstallationManager, - private $processService: IProcessService, private $appDebugSocketProxyFactory: IAppDebugSocketProxyFactory, private $projectDataService: IProjectDataService) { super(device, $devicesService); - this.$processService.attachToProcessExitSignals(this, this.debugStop); this.$appDebugSocketProxyFactory.on(CONNECTION_ERROR_EVENT_NAME, (e: Error) => this.emit(CONNECTION_ERROR_EVENT_NAME, e)); this.deviceIdentifier = this.device.deviceInfo.identifier; } diff --git a/lib/services/livesync/android-device-livesync-sockets-service.ts b/lib/services/livesync/android-device-livesync-sockets-service.ts index 3f5da8422a..d001b4b133 100644 --- a/lib/services/livesync/android-device-livesync-sockets-service.ts +++ b/lib/services/livesync/android-device-livesync-sockets-service.ts @@ -19,7 +19,7 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe $logger: ILogger, protected device: Mobile.IAndroidDevice, private $options: IOptions, - private $processService: IProcessService, + private $cleanupService: ICleanupService, private $fs: IFileSystem, private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, $filesHashService: IFilesHashService) { @@ -59,6 +59,18 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe } } + private async getCleanupAction(appIdentifier: string): Promise { + return { + command: await this.$staticConfig.getAdbFilePath(), args: [ + "-s", + this.device.deviceInfo.identifier, + "shell", + "rm", + "-rf", + appIdentifier + ] + }; + } private async doSync(liveSyncInfo: ILiveSyncResultInfo, projectData: IProjectData): Promise { const operationId = this.livesyncTool.generateOperationIdentifier(); @@ -74,12 +86,14 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe } }, AndroidDeviceSocketsLiveSyncService.STATUS_UPDATE_INTERVAL); + const cleanupAction = await this.getCleanupAction(liveSyncInfo.deviceAppData.appIdentifier); const actionOnEnd = async () => { clearInterval(syncInterval); await this.device.fileSystem.deleteFile(this.getPathToLiveSyncFileOnDevice(liveSyncInfo.deviceAppData.appIdentifier), liveSyncInfo.deviceAppData.appIdentifier); + await this.$cleanupService.removeCleanupAction(cleanupAction); }; - this.$processService.attachToProcessExitSignals(this, actionOnEnd); + await this.$cleanupService.addCleanupAction(cleanupAction); // We need to clear resources when the action fails // But we also need the real result of the action. await doSyncPromise.then(actionOnEnd.bind(this), actionOnEnd.bind(this)); diff --git a/lib/services/livesync/android-livesync-tool.ts b/lib/services/livesync/android-livesync-tool.ts index 4f77096869..d55d010aec 100644 --- a/lib/services/livesync/android-livesync-tool.ts +++ b/lib/services/livesync/android-livesync-tool.ts @@ -41,12 +41,10 @@ export class AndroidLivesyncTool implements IAndroidLivesyncTool { private $fs: IFileSystem, private $logger: ILogger, private $mobileHelper: Mobile.IMobileHelper, - private $processService: IProcessService, private $injector: IInjector) { this.operationPromises = Object.create(null); this.socketError = null; this.socketConnection = null; - this.$processService.attachToProcessExitSignals(this, this.dispose); } public async connect(configuration: IAndroidLivesyncToolConfiguration): Promise { @@ -445,28 +443,6 @@ export class AndroidLivesyncTool implements IAndroidLivesyncTool { return this.$mobileHelper.buildDevicePath(relativeFilePath); } - private dispose(): void { - if (this.pendingConnectionData) { - clearTimeout(this.pendingConnectionData.connectionTimer); - - if (this.pendingConnectionData.socketTimer) { - clearTimeout(this.pendingConnectionData.socketTimer); - } - if (this.pendingConnectionData.socket) { - this.pendingConnectionData.socket.removeAllListeners(); - } - - this.pendingConnectionData.rejectHandler("LiveSync aborted."); - } - this.end(); - - _.keys(this.operationPromises) - .forEach(operationId => { - const operationPromise = this.operationPromises[operationId]; - clearTimeout(operationPromise.timeoutId); - }); - } - private async writeToSocket(data: Buffer): Promise { this.verifyActiveConnection(); const result = await this.socketConnection.writeAsync(data); diff --git a/lib/services/livesync/ios-device-livesync-service.ts b/lib/services/livesync/ios-device-livesync-service.ts index 249ec41a1e..af027caffd 100644 --- a/lib/services/livesync/ios-device-livesync-service.ts +++ b/lib/services/livesync/ios-device-livesync-service.ts @@ -11,7 +11,6 @@ export class IOSDeviceLiveSyncService extends DeviceLiveSyncServiceBase implemen constructor( private $logger: ILogger, - private $processService: IProcessService, protected $platformsData: IPlatformsData, protected device: Mobile.IiOSDevice) { super($platformsData, device); @@ -108,8 +107,6 @@ export class IOSDeviceLiveSyncService extends DeviceLiveSyncServiceBase implemen } private attachEventHandlers(): void { - this.$processService.attachToProcessExitSignals(this, this.destroySocket); - this.socket.on("close", (hadError: boolean) => { this.$logger.trace(`Socket closed, hadError is ${hadError}.`); this.socket = null; diff --git a/lib/services/livesync/livesync-service.ts b/lib/services/livesync/livesync-service.ts index 8f68a793b6..4bb3837a4a 100644 --- a/lib/services/livesync/livesync-service.ts +++ b/lib/services/livesync/livesync-service.ts @@ -28,7 +28,6 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, private $nodeModulesDependenciesBuilder: INodeModulesDependenciesBuilder, private $logger: ILogger, - private $processService: IProcessService, private $hooksService: IHooksService, private $pluginsService: IPluginsService, private $debugService: IDebugService, @@ -810,15 +809,6 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi this.liveSyncProcessesInfo[liveSyncData.projectDir].watcherInfo = { watcher, patterns }; this.liveSyncProcessesInfo[liveSyncData.projectDir].timer = timeoutTimer; - - this.$processService.attachToProcessExitSignals(this, () => { - _.keys(this.liveSyncProcessesInfo).forEach(projectDir => { - // Do not await here, we are in process exit's handler. - /* tslint:disable:no-floating-promises */ - this.stopLiveSync(projectDir); - /* tslint:enable:no-floating-promises */ - }); - }); } } diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index 74468e6e6b..b4988f6e7a 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -119,7 +119,6 @@ function createTestInjector(projectPath: string, projectName: string, xCode?: IX getAllInstalledPlugins: (): string[] => [] }); testInjector.register("androidProcessService", {}); - testInjector.register("processService", {}); testInjector.register("sysInfo", { getXcodeVersion: async () => "" }); diff --git a/test/services/analytics/analytics-service.ts b/test/services/analytics/analytics-service.ts index 6dc149ffbd..2247e1b286 100644 --- a/test/services/analytics/analytics-service.ts +++ b/test/services/analytics/analytics-service.ts @@ -36,9 +36,6 @@ const createTestInjector = (opts?: { projectHelperErrorMsg?: string, projectDir? }); testInjector.register("osInfo", {}); testInjector.register("childProcess", {}); - testInjector.register("processService", { - attachToProcessExitSignals: (context: any, callback: () => void): void => undefined - }); testInjector.register("projectDataService", { getProjectData: (projectDir?: string): IProjectData => { return { diff --git a/test/services/android-device-debug-service.ts b/test/services/android-device-debug-service.ts index 0b3076062a..622e55658a 100644 --- a/test/services/android-device-debug-service.ts +++ b/test/services/android-device-debug-service.ts @@ -11,8 +11,10 @@ class AndroidDeviceDebugServiceInheritor extends AndroidDeviceDebugService { $logger: ILogger, $androidProcessService: Mobile.IAndroidProcessService, $net: INet, - $deviceLogProvider: Mobile.IDeviceLogProvider) { - super({ deviceInfo: { identifier: "123" } }, $devicesService, $errors, $logger, $androidProcessService, $net, $deviceLogProvider); + $deviceLogProvider: Mobile.IDeviceLogProvider, + $cleanupService: ICleanupService, + $staticConfig: IStaticConfig) { + super({ deviceInfo: { identifier: "123" } }, $devicesService, $errors, $logger, $androidProcessService, $net, $cleanupService, $deviceLogProvider, $staticConfig); } public getChromeDebugUrl(debugOptions: IDebugOptions, port: number): string { @@ -31,6 +33,8 @@ const createTestInjector = (): IInjector => { testInjector.register("net", {}); testInjector.register("projectDataService", {}); testInjector.register("deviceLogProvider", {}); + testInjector.register("cleanupService", {}); + testInjector.register("staticConfig", {}); return testInjector; }; diff --git a/test/services/ios-debugger-port-service.ts b/test/services/ios-debugger-port-service.ts index 6af66967e7..512ea74ebf 100644 --- a/test/services/ios-debugger-port-service.ts +++ b/test/services/ios-debugger-port-service.ts @@ -45,9 +45,6 @@ function createTestInjector() { iOSSim: () => ({}) }); injector.register("logger", LoggerStub); - injector.register("processService", { - attachToProcessExitSignals: () => ({}) - }); injector.register("projectDataService", { getProjectData: (projectDir: string) => ({ projectName: "test", diff --git a/test/services/ios-device-debug-service.ts b/test/services/ios-device-debug-service.ts index bfdffe1153..d97f37863d 100644 --- a/test/services/ios-device-debug-service.ts +++ b/test/services/ios-device-debug-service.ts @@ -12,11 +12,10 @@ class IOSDeviceDebugServiceInheritor extends IOSDeviceDebugService { $logger: ILogger, $errors: IErrors, $packageInstallationManager: IPackageInstallationManager, - $processService: IProcessService, $appDebugSocketProxyFactory: IAppDebugSocketProxyFactory, $projectDataService: IProjectDataService) { super({ deviceInfo: { identifier: "123" } }, $devicesService, $childProcess, $hostInfo, $logger, $errors, - $packageInstallationManager, $processService, $appDebugSocketProxyFactory, $projectDataService); + $packageInstallationManager, $appDebugSocketProxyFactory, $projectDataService); } public getChromeDebugUrl(debugOptions: IDebugOptions, port: number): string { @@ -37,10 +36,6 @@ const createTestInjector = (): IInjector => { testInjector.register("packageInstallationManager", {}); testInjector.register("iOSNotification", {}); testInjector.register("iOSSocketRequestExecutor", {}); - testInjector.register("processService", { - attachToProcessExitSignals: (context: any, callback: () => void): void => undefined - }); - testInjector.register("appDebugSocketProxyFactory", { on: (event: string | symbol, listener: Function): any => undefined }); diff --git a/test/services/livesync-service.ts b/test/services/livesync-service.ts index 65e4284b86..9a4008ce8c 100644 --- a/test/services/livesync-service.ts +++ b/test/services/livesync-service.ts @@ -17,7 +17,6 @@ const createTestInjector = (): IInjector => { testInjector.register("devicePlatformsConstants", {}); testInjector.register("nodeModulesDependenciesBuilder", {}); testInjector.register("logger", LoggerStub); - testInjector.register("processService", {}); testInjector.register("debugService", {}); testInjector.register("errors", {}); testInjector.register("debugDataService", {}); @@ -52,7 +51,6 @@ class LiveSyncServiceInheritor extends LiveSyncService { $devicePlatformsConstants: Mobile.IDevicePlatformsConstants, $nodeModulesDependenciesBuilder: INodeModulesDependenciesBuilder, $logger: ILogger, - $processService: IProcessService, $hooksService: IHooksService, $pluginsService: IPluginsService, $debugService: IDebugService, @@ -75,7 +73,6 @@ class LiveSyncServiceInheritor extends LiveSyncService { $devicePlatformsConstants, $nodeModulesDependenciesBuilder, $logger, - $processService, $hooksService, $pluginsService, $debugService, diff --git a/test/services/livesync/android-livesync-tool.ts b/test/services/livesync/android-livesync-tool.ts index bb5c0e62f1..49f14905bc 100644 --- a/test/services/livesync/android-livesync-tool.ts +++ b/test/services/livesync/android-livesync-tool.ts @@ -1,7 +1,7 @@ import { Yok } from "../../../lib/common/yok"; import { assert } from "chai"; import * as sinon from "sinon"; -import { LoggerStub, ProcessServiceStub } from "../../stubs"; +import { LoggerStub } from "../../stubs"; import { AndroidLivesyncTool } from "../../../lib/services/livesync/android-livesync-tool"; import { LiveSyncSocket } from "../../../lib/services/livesync/livesync-socket"; import { MobileHelper } from "../../../lib/common/mobile/mobile-helper"; @@ -67,7 +67,6 @@ const createTestInjector = (socket: INetSocket, fileStreams: IDictionary void): void { - return undefined; - } -} - export class FileSystemStub implements IFileSystem { deleteDirectorySafe(directory: string): void { return this.deleteDirectory(directory); From b97f5aa07657847b5eba54a2d8c9c70eae797077 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 12 Apr 2019 09:39:39 +0300 Subject: [PATCH 06/16] chore: remove `$timers` Remove previously introduced `$timers` - the idea was to use $timers as we wanted to kill remaining timers when SIGINT is sent to CLI. As CLI no longer handles SIGINT signal, Node.js kills the timers on its own, so there's no need of additional timers implementation. --- lib/common/bootstrap.ts | 1 - lib/common/definitions/timers.d.ts | 4 ---- lib/common/mobile/mobile-core/devices-service.ts | 11 +++++------ .../test/unit-tests/mobile/devices-service.ts | 2 -- lib/common/timers.ts | 14 -------------- test/ios-project-service.ts | 1 - 6 files changed, 5 insertions(+), 28 deletions(-) delete mode 100644 lib/common/definitions/timers.d.ts delete mode 100644 lib/common/timers.ts diff --git a/lib/common/bootstrap.ts b/lib/common/bootstrap.ts index 8bb6a9d31e..2aa2c255c8 100644 --- a/lib/common/bootstrap.ts +++ b/lib/common/bootstrap.ts @@ -6,7 +6,6 @@ $injector.require("errors", "./errors"); $injector.requirePublic("fs", "./file-system"); $injector.require("hostInfo", "./host-info"); $injector.require("osInfo", "./os-info"); -$injector.require("timers", "./timers"); $injector.require("dispatcher", "./dispatchers"); $injector.require("commandDispatcher", "./dispatchers"); diff --git a/lib/common/definitions/timers.d.ts b/lib/common/definitions/timers.d.ts deleted file mode 100644 index 2b57b8b06f..0000000000 --- a/lib/common/definitions/timers.d.ts +++ /dev/null @@ -1,4 +0,0 @@ -interface ITimers { - setInterval(callback: (...args: any[]) => void, ms: number, ...args: any[]): NodeJS.Timer; - clearInterval(intervalId: NodeJS.Timer): void; -} \ No newline at end of file diff --git a/lib/common/mobile/mobile-core/devices-service.ts b/lib/common/mobile/mobile-core/devices-service.ts index 11efa236ed..72ebf1b517 100644 --- a/lib/common/mobile/mobile-core/devices-service.ts +++ b/lib/common/mobile/mobile-core/devices-service.ts @@ -42,8 +42,7 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi private $androidEmulatorServices: Mobile.IEmulatorPlatformService, private $androidEmulatorDiscovery: Mobile.IDeviceDiscovery, private $emulatorHelper: Mobile.IEmulatorHelper, - private $prompter: IPrompter, - private $timers: ITimers) { + private $prompter: IPrompter) { super(); this.attachToKnownDeviceDiscoveryEvents(); this.attachToKnownEmulatorDiscoveryEvents(); @@ -293,7 +292,7 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi if (!this.deviceDetectionInterval) { let isDeviceDetectionIntervalInProgress = false; - this.deviceDetectionInterval = this.$timers.setInterval(async () => { + this.deviceDetectionInterval = setInterval(async () => { if (isDeviceDetectionIntervalInProgress) { return; } @@ -320,7 +319,7 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi @exported("devicesService") public startEmulatorDetectionInterval(opts: Mobile.IHasDetectionInterval = {}): void { let isEmulatorDetectionIntervalRunning = false; - this.emulatorDetectionInterval = this.$timers.setInterval(async () => { + this.emulatorDetectionInterval = setInterval(async () => { if (isEmulatorDetectionIntervalRunning) { return; } @@ -336,14 +335,14 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi @exported("devicesService") public stopDeviceDetectionInterval(): void { if (this.deviceDetectionInterval) { - this.$timers.clearInterval(this.deviceDetectionInterval); + clearInterval(this.deviceDetectionInterval); } } @exported("devicesService") public stopEmulatorDetectionInterval(): void { if (this.emulatorDetectionInterval) { - this.$timers.clearInterval(this.emulatorDetectionInterval); + clearInterval(this.emulatorDetectionInterval); } } diff --git a/lib/common/test/unit-tests/mobile/devices-service.ts b/lib/common/test/unit-tests/mobile/devices-service.ts index 984156c311..d61a2d0298 100644 --- a/lib/common/test/unit-tests/mobile/devices-service.ts +++ b/lib/common/test/unit-tests/mobile/devices-service.ts @@ -14,7 +14,6 @@ import { CommonLoggerStub, ErrorsStub } from "../stubs"; import { Messages } from "../../../messages/messages"; import * as constants from "../../../constants"; import { DevicePlatformsConstants } from "../../../mobile/device-platforms-constants"; -import { Timers } from "../../../timers"; const helpers = require("../../../helpers"); const originalIsInteractive = helpers.isInteractive; @@ -183,7 +182,6 @@ function createTestInjector(): IInjector { testInjector.register("androidProcessService", { /* no implementation required */ }); testInjector.register("androidEmulatorDiscovery", AndroidEmulatorDiscoveryStub); testInjector.register("emulatorHelper", {}); - testInjector.register("timers", Timers); return testInjector; } diff --git a/lib/common/timers.ts b/lib/common/timers.ts deleted file mode 100644 index 47d52a58ea..0000000000 --- a/lib/common/timers.ts +++ /dev/null @@ -1,14 +0,0 @@ -// TODO: Delete this service, it does not bring any value -export class Timers implements ITimers { - public setInterval(callback: (...args: any[]) => void, ms: number, ...args: any[]): NodeJS.Timer { - const timer = setInterval(callback, ms, args); - - return timer; - } - - public clearInterval(intervalId: NodeJS.Timer): void { - clearInterval(intervalId); - } -} - -$injector.register("timers", Timers); diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index b4988f6e7a..d01f8df6c0 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -159,7 +159,6 @@ function createTestInjector(projectPath: string, projectName: string, xCode?: IX removeExtensions: () => { /* */ }, addExtensionsFromPath: () => Promise.resolve() }); - testInjector.register("timers", {}); return testInjector; } From 916b17e005987021361dcbc8b2261caa286bbeda Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 12 Apr 2019 09:52:32 +0300 Subject: [PATCH 07/16] docs: update PublicAPI for cleanupService Add information in PublicAPI for cleanupService and its methods. --- PublicAPI.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/PublicAPI.md b/PublicAPI.md index a2f251389e..066640eb0d 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -70,6 +70,8 @@ const tns = require("nativescript"); * [deviceLog](#devicelog) * [previewQrCodeService](#previewqrcodeservice) * [getPlaygroundAppQrCode](#getplaygroundappqrcode) +* [cleanupService](#cleanupservice) + * [setCleanupLogFile](#setcleanuplogfile) ## Module projectService @@ -1487,6 +1489,31 @@ tns.previewQrCodeService.getPlaygroundAppQrCode() }); ``` +## cleanupService +The `cleanupService` is used to handle actions that should be executed after CLI's process had exited. This is an internal service, that runs detached childProcess in which it executes CLI's cleanup actions once CLI is dead. As the process is detached, logs from it are not shown anywhere, so the service exposes a way to add log file in which the child process will write its logs. + +### setCleanupLogFile +Defines the log file location where the child cleanup process will write its logs. + +> NOTE: You must call this method immediately after requiring NativeScript CLI. In case you call it after the cleanup process had started, it will not use the passed log file. + +* Definition +```TypeScript +/** + * Sets the file in which the cleanup process will write its logs. + * This method must be called before starting the cleanup process, i.e. when CLI is initialized. + * @param {string} filePath Path to file where the logs will be written. The logs are appended to the passed file. + * @returns {void} + */ +setCleanupLogFile(filePath: string): void; +``` + +* Usage +```JavaScript +const tns = require("nativescript"); +tns.cleanupService.setCleanupLogFile("/Users/username/cleanup-logs.txt"); +``` + ## How to add a new method to Public API CLI is designed as command line tool and when it is used as a library, it does not give you access to all of the methods. This is mainly implementation detail. Most of the CLI's code is created to work in command line, not as a library, so before adding method to public API, most probably it will require some modification. For example the `$options` injected module contains information about all `--` options passed on the terminal. When the CLI is used as a library, the options are not populated. Before adding method to public API, make sure its implementation does not rely on `$options`. From 6e366d858da37d22804c926a1bb7ce483cb335cf Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 12 Apr 2019 09:58:46 +0300 Subject: [PATCH 08/16] chore: remove already implemented TODO --- lib/common/mobile/mobile-core/android-process-service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/common/mobile/mobile-core/android-process-service.ts b/lib/common/mobile/mobile-core/android-process-service.ts index 2abc66d872..abf19d115e 100644 --- a/lib/common/mobile/mobile-core/android-process-service.ts +++ b/lib/common/mobile/mobile-core/android-process-service.ts @@ -122,7 +122,6 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { } this._forwardedLocalPorts[portForwardInputData.deviceIdentifier] = localPort; - // TODO: use correct path to adb await this.$cleanupService.addCleanupAction({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", portForwardInputData.deviceIdentifier, "forward", "--remove", `tcp:${localPort}`] }); return localPort && +localPort; } From ce2a801d6a57e62e316bb56c69b9195bc06b0c82 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 12 Apr 2019 14:00:48 +0300 Subject: [PATCH 09/16] feat: allow setting timeout when using childProcess In Node.js you can set timeout option to child processes started with `exec`. However, there's no support for this when using `spawn`. Add support for passing timeout in our abstraction of childProcess, when using `spawnFromEvent` method. When this method is used, it blocks the operation until the process finishes its execution. So it is appropriate to pass timeout and if it passes, send `SIGTERM` signal to the child process. This is the same signal used by Node.js when the timeout is hit in `child_process.exec`. --- lib/common/child-process.ts | 29 ++++++++++++++++++++++++----- lib/common/declarations.d.ts | 3 ++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/common/child-process.ts b/lib/common/child-process.ts index f253af2c1a..125ec99592 100644 --- a/lib/common/child-process.ts +++ b/lib/common/child-process.ts @@ -62,7 +62,15 @@ export class ChildProcess extends EventEmitter implements IChildProcess { let isResolved = false; let capturedOut = ""; let capturedErr = ""; - + let killTimer: NodeJS.Timer = null; + + if (spawnFromEventOptions && spawnFromEventOptions.timeout) { + this.$logger.trace(`Setting maximum time for execution of current child process to ${spawnFromEventOptions.timeout}`); + killTimer = setTimeout(() => { + this.$logger.trace(`Sending SIGTERM to current child process as maximum time for execution ${spawnFromEventOptions.timeout} had passed.`); + childProcess.kill('SIGTERM'); + }, spawnFromEventOptions.timeout); + } if (childProcess.stdout) { childProcess.stdout.on("data", (data: string) => { if (spawnFromEventOptions && spawnFromEventOptions.emitOptions && spawnFromEventOptions.emitOptions.eventName) { @@ -91,17 +99,27 @@ export class ChildProcess extends EventEmitter implements IChildProcess { exitCode: exitCode }; + const clearKillTimer = () => { + if (killTimer) { + clearTimeout(killTimer); + } + }; + + const resolveAction = () => { + isResolved = true; + resolve(result); + clearKillTimer(); + }; + if (spawnFromEventOptions && spawnFromEventOptions.throwError === false) { if (!isResolved) { this.$logger.trace("Result when throw error is false:"); this.$logger.trace(result); - isResolved = true; - resolve(result); + resolveAction(); } } else { if (exitCode === 0) { - isResolved = true; - resolve(result); + resolveAction(); } else { let errorMessage = `Command ${command} failed with exit code ${exitCode}`; if (capturedErr) { @@ -111,6 +129,7 @@ export class ChildProcess extends EventEmitter implements IChildProcess { if (!isResolved) { isResolved = true; reject(new Error(errorMessage)); + clearKillTimer(); } } } diff --git a/lib/common/declarations.d.ts b/lib/common/declarations.d.ts index e65cae28e4..823adfe9fc 100644 --- a/lib/common/declarations.d.ts +++ b/lib/common/declarations.d.ts @@ -648,7 +648,8 @@ interface ISpawnFromEventOptions { throwError: boolean; emitOptions?: { eventName: string; - } + }, + timeout?: number; } interface IProjectDir { From 9a6b0f2e170474f31a70528bb0aba10b1315dc69 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 12 Apr 2019 14:14:41 +0300 Subject: [PATCH 10/16] feat: add timeout for each cleanup process Add default timeout for each cleanup operation. Set it to 3000ms by default. For each action, you can set the timeout when adding it to the cleanup action list by passing timeout. --- lib/detached-processes/cleanup-process.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/detached-processes/cleanup-process.ts b/lib/detached-processes/cleanup-process.ts index aa66a6da62..8b1c8d65b7 100644 --- a/lib/detached-processes/cleanup-process.ts +++ b/lib/detached-processes/cleanup-process.ts @@ -27,7 +27,7 @@ const executeCleanup = async () => { fileLogService.logData({ message: `Start executing action: ${JSON.stringify(action)}` }); // TODO: Add timeout for each action here - await $childProcess.trySpawnFromCloseEvent(action.command, action.args); + await $childProcess.trySpawnFromCloseEvent(action.command, action.args, {}, { throwError: true, timeout: action.timeout || 3000 }); fileLogService.logData({ message: `Successfully executed action: ${JSON.stringify(action)}` }); } catch (err) { fileLogService.logData({ message: `Unable to execute action: ${JSON.stringify(action)}`, type: FileLogMessageType.Error }); From 8084cac6987178d147c68c8ffd31ebf95056174a Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 Apr 2019 09:11:00 +0300 Subject: [PATCH 11/16] chore: remove unused object from httpClient --- lib/common/http-client.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/common/http-client.ts b/lib/common/http-client.ts index 66e6ca647e..6aaec22c5d 100644 --- a/lib/common/http-client.ts +++ b/lib/common/http-client.ts @@ -15,13 +15,11 @@ export class HttpClient implements Server.IHttpClient { // We receive multiple response packets every ms but we don't need to be very aggressive here. private static STUCK_RESPONSE_CHECK_INTERVAL = 10000; - private defaultUserAgent: string; - private cleanupData: ICleanupRequestData[]; +private defaultUserAgent: string; constructor(private $logger: ILogger, private $proxyService: IProxyService, private $staticConfig: Config.IStaticConfig) { - this.cleanupData = []; } public async httpRequest(options: any, proxySettings?: IProxySettings): Promise { @@ -101,7 +99,6 @@ export class HttpClient implements Server.IHttpClient { const result = new Promise((resolve, reject) => { let timerId: NodeJS.Timer; const cleanupRequestData: ICleanupRequestData = Object.create({ timers: [] }); - this.cleanupData.push(cleanupRequestData); const promiseActions: IPromiseActions = { resolve, From 531bfdac95027b4682edafcaf0bee4ba9e03ed2e Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 Apr 2019 09:11:22 +0300 Subject: [PATCH 12/16] chore: remove forgotten TODO --- lib/detached-processes/cleanup-process.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/detached-processes/cleanup-process.ts b/lib/detached-processes/cleanup-process.ts index 8b1c8d65b7..dc0c9ef52b 100644 --- a/lib/detached-processes/cleanup-process.ts +++ b/lib/detached-processes/cleanup-process.ts @@ -26,7 +26,6 @@ const executeCleanup = async () => { try { fileLogService.logData({ message: `Start executing action: ${JSON.stringify(action)}` }); - // TODO: Add timeout for each action here await $childProcess.trySpawnFromCloseEvent(action.command, action.args, {}, { throwError: true, timeout: action.timeout || 3000 }); fileLogService.logData({ message: `Successfully executed action: ${JSON.stringify(action)}` }); } catch (err) { From 5744075b99d9ce1ce8b8e3d1168800fd05d39d35 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 Apr 2019 09:11:32 +0300 Subject: [PATCH 13/16] fix: cleanup all resources used by lockService `lockService` creates a file to be locked and next to it - directory with the same name and `.lock` extension. Both should be cleaned when CLI process is dies, so handle both of them. --- lib/common/services/lock-service.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/common/services/lock-service.ts b/lib/common/services/lock-service.ts index 5cb057ecf7..50298e8668 100644 --- a/lib/common/services/lock-service.ts +++ b/lib/common/services/lock-service.ts @@ -43,7 +43,11 @@ export class LockService implements ILockService { public async lock(lockFilePath?: string, lockOpts?: ILockOptions): Promise<() => void> { const { filePath, fileOpts } = this.getLockFileSettings(lockFilePath, lockOpts); - await this.$cleanupService.addCleanupDeleteAction(filePath); + + for (const pathToClean of this.getPathsForCleanupAction(filePath)) { + await this.$cleanupService.addCleanupDeleteAction(pathToClean); + } + this.$fs.writeFile(filePath, ""); try { @@ -65,7 +69,18 @@ export class LockService implements ILockService { private async cleanLock(lockPath: string): Promise { this.$fs.deleteFile(lockPath); - await this.$cleanupService.removeCleanupDeleteAction(lockPath); + for (const filePath of this.getPathsForCleanupAction(lockPath)) { + await this.$cleanupService.removeCleanupDeleteAction(filePath); + } + } + + private getPathsForCleanupAction(lockPath: string): string[] { + // The proper-lockfile creates directory with the passed name and `.lock` at the end. + // Ensure we will take care of it as well. + return [ + lockPath, + `${lockPath}.lock` + ]; } private getLockFileSettings(filePath?: string, fileOpts?: ILockOptions): { filePath: string, fileOpts: ILockOptions } { From 1b8a954c031f38dceea633606f70196d37b7dcd5 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 Apr 2019 09:46:49 +0300 Subject: [PATCH 14/16] chore: rename interfaces for cleanup Improve interfaces and enums for cleanup services. --- .../mobile-core/android-process-service.ts | 2 +- lib/definitions/cleanup-service.d.ts | 8 +-- .../cleanup-process-definitions.d.ts | 17 +++--- lib/detached-processes/cleanup-process.ts | 54 +++++++++---------- .../detached-process-enums.d.ts | 18 +++---- lib/services/android-device-debug-service.ts | 2 +- lib/services/cleanup-service.ts | 12 ++--- ...android-device-livesync-sockets-service.ts | 8 +-- 8 files changed, 61 insertions(+), 60 deletions(-) diff --git a/lib/common/mobile/mobile-core/android-process-service.ts b/lib/common/mobile/mobile-core/android-process-service.ts index abf19d115e..c912165968 100644 --- a/lib/common/mobile/mobile-core/android-process-service.ts +++ b/lib/common/mobile/mobile-core/android-process-service.ts @@ -122,7 +122,7 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { } this._forwardedLocalPorts[portForwardInputData.deviceIdentifier] = localPort; - await this.$cleanupService.addCleanupAction({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", portForwardInputData.deviceIdentifier, "forward", "--remove", `tcp:${localPort}`] }); + await this.$cleanupService.addCleanupCommand({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", portForwardInputData.deviceIdentifier, "forward", "--remove", `tcp:${localPort}`] }); return localPort && +localPort; } diff --git a/lib/definitions/cleanup-service.d.ts b/lib/definitions/cleanup-service.d.ts index cae0837bd9..59e56a350f 100644 --- a/lib/definitions/cleanup-service.d.ts +++ b/lib/definitions/cleanup-service.d.ts @@ -5,18 +5,18 @@ interface ICleanupService extends IShouldDispose, IDisposable { /** * Add new action to be executed when CLI process exits. - * @param {ICleanupAction} action The action that should be executed, including command and args. + * @param {ISpawnCommandInfo} commandInfo The command that should be executed, including command and args. * @returns {Promise} */ - addCleanupAction(action: ICleanupAction): Promise; + addCleanupCommand(commandInfo: ISpawnCommandInfo): Promise; /** * Remove action to be executed when CLI process exits. * NOTE: The action should be added in the action list by calling `addCleanupAction` first. - * @param {ICleanupAction} action The action that should be removed from cleanup execution, including command and args. + * @param {ISpawnCommandInfo} commandInfo The command that should be removed from cleanup execution, including command and args. * @returns {Promise} */ - removeCleanupAction(action: ICleanupAction): Promise + removeCleanupCommand(commandInfo: ISpawnCommandInfo): Promise /** * Sets the file in which the cleanup process will write its logs. diff --git a/lib/detached-processes/cleanup-process-definitions.d.ts b/lib/detached-processes/cleanup-process-definitions.d.ts index 578d0d01a4..72ed39579a 100644 --- a/lib/detached-processes/cleanup-process-definitions.d.ts +++ b/lib/detached-processes/cleanup-process-definitions.d.ts @@ -1,4 +1,4 @@ -interface ICleanupAction { +interface ISpawnCommandInfo { /** * Executable to be started. */ @@ -8,27 +8,28 @@ interface ICleanupAction { * Arguments that will be passed to the child process */ args: string[]; + /** * Timeout to execute the action. */ timeout?: number; } -interface ICleanupProcessMessage { +interface ICleanupMessageBase { /** - * Type of the action + * Type of the message */ - actionType: CleanupProcessMessageType; + messageType: CleanupProcessMessage; } -interface ICleanupActionMessage extends ICleanupProcessMessage { +interface ISpawnCommandCleanupMessage extends ICleanupMessageBase { /** - * Describes the action that must be executed + * Describes the command that must be executed */ - action: ICleanupAction; + commandInfo: ISpawnCommandInfo; } -interface ICleanupDeleteActionMessage extends ICleanupProcessMessage { +interface IDeleteFileCleanupMessage extends ICleanupMessageBase { /** * Path to file/directory to be deleted. */ diff --git a/lib/detached-processes/cleanup-process.ts b/lib/detached-processes/cleanup-process.ts index dc0c9ef52b..705c266c6f 100644 --- a/lib/detached-processes/cleanup-process.ts +++ b/lib/detached-processes/cleanup-process.ts @@ -17,19 +17,19 @@ require(pathToBootstrap); const fileLogService = $injector.resolve(FileLogService, { logFile }); fileLogService.logData({ message: "Initializing Cleanup process." }); -const actionsToExecute: ICleanupAction[] = []; +const commandsInfos: ISpawnCommandInfo[] = []; const filesToDelete: string[] = []; const executeCleanup = async () => { const $childProcess = $injector.resolve("childProcess"); - for (const action of actionsToExecute) { + for (const commandInfo of commandsInfos) { try { - fileLogService.logData({ message: `Start executing action: ${JSON.stringify(action)}` }); + fileLogService.logData({ message: `Start executing command: ${JSON.stringify(commandInfo)}` }); - await $childProcess.trySpawnFromCloseEvent(action.command, action.args, {}, { throwError: true, timeout: action.timeout || 3000 }); - fileLogService.logData({ message: `Successfully executed action: ${JSON.stringify(action)}` }); + await $childProcess.trySpawnFromCloseEvent(commandInfo.command, commandInfo.args, {}, { throwError: true, timeout: commandInfo.timeout || 3000 }); + fileLogService.logData({ message: `Successfully executed command: ${JSON.stringify(commandInfo)}` }); } catch (err) { - fileLogService.logData({ message: `Unable to execute action: ${JSON.stringify(action)}`, type: FileLogMessageType.Error }); + fileLogService.logData({ message: `Unable to execute command: ${JSON.stringify(commandInfo)}`, type: FileLogMessageType.Error }); } } @@ -42,21 +42,21 @@ const executeCleanup = async () => { process.exit(); }; -const addCleanupAction = (newAction: ICleanupAction): void => { - if (_.some(actionsToExecute, currentAction => _.isEqual(currentAction, newAction))) { - fileLogService.logData({ message: `cleanup-process will not add action for execution as it has been added already: ${JSON.stringify(newAction)}` }); +const addCleanupAction = (commandInfo: ISpawnCommandInfo): void => { + if (_.some(commandsInfos, currentCommandInfo => _.isEqual(currentCommandInfo, commandInfo))) { + fileLogService.logData({ message: `cleanup-process will not add command for execution as it has been added already: ${JSON.stringify(commandInfo)}` }); } else { - fileLogService.logData({ message: `cleanup-process added action for execution: ${JSON.stringify(newAction)}` }); - actionsToExecute.push(newAction); + fileLogService.logData({ message: `cleanup-process added command for execution: ${JSON.stringify(commandInfo)}` }); + commandsInfos.push(commandInfo); } }; -const removeCleanupAction = (actionToRemove: ICleanupAction): void => { - if (_.some(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove))) { - _.remove(actionsToExecute, currentAction => _.isEqual(currentAction, actionToRemove)); - fileLogService.logData({ message: `cleanup-process removed action for execution: ${JSON.stringify(actionToRemove)}` }); +const removeCleanupAction = (commandInfo: ISpawnCommandInfo): void => { + if (_.some(commandsInfos, currentCommandInfo => _.isEqual(currentCommandInfo, commandInfo))) { + _.remove(commandsInfos, currentCommandInfo => _.isEqual(currentCommandInfo, commandInfo)); + fileLogService.logData({ message: `cleanup-process removed command for execution: ${JSON.stringify(commandInfo)}` }); } else { - fileLogService.logData({ message: `cleanup-process cannot remove action for execution as it has note been added before: ${JSON.stringify(actionToRemove)}` }); + fileLogService.logData({ message: `cleanup-process cannot remove command for execution as it has note been added before: ${JSON.stringify(commandInfo)}` }); } }; @@ -82,24 +82,24 @@ const removeDeleteAction = (filePath: string): void => { } }; -process.on("message", async (cleanupProcessMessage: ICleanupProcessMessage) => { +process.on("message", async (cleanupProcessMessage: ICleanupMessageBase) => { fileLogService.logData({ message: `cleanup-process received message of type: ${JSON.stringify(cleanupProcessMessage)}` }); - switch (cleanupProcessMessage.actionType) { - case CleanupProcessMessageType.AddCleanAction: - addCleanupAction((cleanupProcessMessage).action); + switch (cleanupProcessMessage.messageType) { + case CleanupProcessMessage.AddCleanCommand: + addCleanupAction((cleanupProcessMessage).commandInfo); break; - case CleanupProcessMessageType.RemoveCleanAction: - removeCleanupAction((cleanupProcessMessage).action); + case CleanupProcessMessage.RemoveCleanCommand: + removeCleanupAction((cleanupProcessMessage).commandInfo); break; - case CleanupProcessMessageType.AddDeleteAction: - addDeleteAction((cleanupProcessMessage).filePath); + case CleanupProcessMessage.AddDeleteFileAction: + addDeleteAction((cleanupProcessMessage).filePath); break; - case CleanupProcessMessageType.RemoveDeleteAction: - removeDeleteAction((cleanupProcessMessage).filePath); + case CleanupProcessMessage.RemoveDeleteFileAction: + removeDeleteAction((cleanupProcessMessage).filePath); break; default: - fileLogService.logData({ message: `Unable to handle message of type ${cleanupProcessMessage.actionType}. Full message is ${JSON.stringify(cleanupProcessMessage)}`, type: FileLogMessageType.Error }); + fileLogService.logData({ message: `Unable to handle message of type ${cleanupProcessMessage.messageType}. Full message is ${JSON.stringify(cleanupProcessMessage)}`, type: FileLogMessageType.Error }); break; } diff --git a/lib/detached-processes/detached-process-enums.d.ts b/lib/detached-processes/detached-process-enums.d.ts index 2ee9c41cb9..1e3a16f5d6 100644 --- a/lib/detached-processes/detached-process-enums.d.ts +++ b/lib/detached-processes/detached-process-enums.d.ts @@ -23,25 +23,25 @@ declare const enum FileLogMessageType { Error = "Error" } -declare const enum CleanupProcessMessageType { +declare const enum CleanupProcessMessage { /** - * This type of action defines that cleanup procedure should execute specific action. + * This type of message defines that cleanup procedure should execute specific command. */ - AddCleanAction = "AddCleanAction", + AddCleanCommand = "AddCleanCommand", /** - * This type of action defines that cleanup procedure should not execute previously defined cleanup action. + * This type of message defines that cleanup procedure should not execute previously defined cleanup command. */ - RemoveCleanAction = "RemoveCleanAction", + RemoveCleanCommand = "RemoveCleanCommand", /** - * This type of action defines that cleanup procedure should delete specified files. + * This type of message defines that cleanup procedure should delete specified files. */ - AddDeleteAction = "AddDeleteAction", + AddDeleteFileAction = "AddDeleteFileAction", /** - * This type of action defines the cleanup procedure should not delete previously specified file. + * This type of message defines the cleanup procedure should not delete previously specified file. */ - RemoveDeleteAction = "RemoveDeleteAction", + RemoveDeleteFileAction = "RemoveDeleteFileAction", } diff --git a/lib/services/android-device-debug-service.ts b/lib/services/android-device-debug-service.ts index 017fcb8cd5..446571690c 100644 --- a/lib/services/android-device-debug-service.ts +++ b/lib/services/android-device-debug-service.ts @@ -71,7 +71,7 @@ export class AndroidDeviceDebugService extends DebugServiceBase implements IDevi await this.unixSocketForward(port, `${unixSocketName}`); } - await this.$cleanupService.addCleanupAction({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", deviceId, "forward", "--remove", `tcp:${port}`] }); + await this.$cleanupService.addCleanupCommand({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", deviceId, "forward", "--remove", `tcp:${port}`] }); return port; } diff --git a/lib/services/cleanup-service.ts b/lib/services/cleanup-service.ts index 2be9be3d08..137f95f85a 100644 --- a/lib/services/cleanup-service.ts +++ b/lib/services/cleanup-service.ts @@ -15,24 +15,24 @@ export class CleanupService implements ICleanupService { public shouldDispose = false; - public async addCleanupAction(action: ICleanupAction): Promise { + public async addCleanupCommand(commandInfo: ISpawnCommandInfo): Promise { const cleanupProcess = await this.getCleanupProcess(); - cleanupProcess.send({ actionType: CleanupProcessMessageType.AddCleanAction, action }); + cleanupProcess.send({ messageType: CleanupProcessMessage.AddCleanCommand, commandInfo }); } - public async removeCleanupAction(action: ICleanupAction): Promise { + public async removeCleanupCommand(commandInfo: ISpawnCommandInfo): Promise { const cleanupProcess = await this.getCleanupProcess(); - cleanupProcess.send({ actionType: CleanupProcessMessageType.RemoveCleanAction, action }); + cleanupProcess.send({ messageType: CleanupProcessMessage.RemoveCleanCommand, commandInfo }); } public async addCleanupDeleteAction(filePath: string): Promise { const cleanupProcess = await this.getCleanupProcess(); - cleanupProcess.send({ actionType: CleanupProcessMessageType.AddDeleteAction, filePath }); + cleanupProcess.send({ messageType: CleanupProcessMessage.AddDeleteFileAction, filePath }); } public async removeCleanupDeleteAction(filePath: string): Promise { const cleanupProcess = await this.getCleanupProcess(); - cleanupProcess.send({ actionType: CleanupProcessMessageType.RemoveDeleteAction, filePath }); + cleanupProcess.send({ messageType: CleanupProcessMessage.RemoveDeleteFileAction, filePath }); } @exported("cleanupService") diff --git a/lib/services/livesync/android-device-livesync-sockets-service.ts b/lib/services/livesync/android-device-livesync-sockets-service.ts index d001b4b133..f9efc9d11a 100644 --- a/lib/services/livesync/android-device-livesync-sockets-service.ts +++ b/lib/services/livesync/android-device-livesync-sockets-service.ts @@ -59,7 +59,7 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe } } - private async getCleanupAction(appIdentifier: string): Promise { + private async getCleanupCommand(appIdentifier: string): Promise { return { command: await this.$staticConfig.getAdbFilePath(), args: [ "-s", @@ -86,14 +86,14 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe } }, AndroidDeviceSocketsLiveSyncService.STATUS_UPDATE_INTERVAL); - const cleanupAction = await this.getCleanupAction(liveSyncInfo.deviceAppData.appIdentifier); + const cleanupCommand = await this.getCleanupCommand(liveSyncInfo.deviceAppData.appIdentifier); const actionOnEnd = async () => { clearInterval(syncInterval); await this.device.fileSystem.deleteFile(this.getPathToLiveSyncFileOnDevice(liveSyncInfo.deviceAppData.appIdentifier), liveSyncInfo.deviceAppData.appIdentifier); - await this.$cleanupService.removeCleanupAction(cleanupAction); + await this.$cleanupService.removeCleanupCommand(cleanupCommand); }; - await this.$cleanupService.addCleanupAction(cleanupAction); + await this.$cleanupService.addCleanupCommand(cleanupCommand); // We need to clear resources when the action fails // But we also need the real result of the action. await doSyncPromise.then(actionOnEnd.bind(this), actionOnEnd.bind(this)); From 29060209208d8cdc24a3aca8e0548addc9f2d92b Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 Apr 2019 09:54:37 +0300 Subject: [PATCH 15/16] fix: do not remove ports forwarding for Android on cleanup Comment the code that removes the port forwarding for Android as this is a breaking change in the behavior. Uncomment the code for 6.0.0 release. --- lib/common/mobile/mobile-core/android-process-service.ts | 7 +++---- lib/services/android-device-debug-service.ts | 7 +++---- test/services/android-device-debug-service.ts | 6 ++---- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/common/mobile/mobile-core/android-process-service.ts b/lib/common/mobile/mobile-core/android-process-service.ts index c912165968..ee6b4e5c88 100644 --- a/lib/common/mobile/mobile-core/android-process-service.ts +++ b/lib/common/mobile/mobile-core/android-process-service.ts @@ -9,9 +9,7 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { constructor(private $errors: IErrors, private $injector: IInjector, - private $net: INet, - private $cleanupService: ICleanupService, - private $staticConfig: IStaticConfig) { + private $net: INet) { this._devicesAdbs = {}; this._forwardedLocalPorts = {}; } @@ -122,7 +120,8 @@ export class AndroidProcessService implements Mobile.IAndroidProcessService { } this._forwardedLocalPorts[portForwardInputData.deviceIdentifier] = localPort; - await this.$cleanupService.addCleanupCommand({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", portForwardInputData.deviceIdentifier, "forward", "--remove", `tcp:${localPort}`] }); + // TODO: Uncomment for 6.0.0 release + // await this.$cleanupService.addCleanupCommand({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", portForwardInputData.deviceIdentifier, "forward", "--remove", `tcp:${localPort}`] }); return localPort && +localPort; } diff --git a/lib/services/android-device-debug-service.ts b/lib/services/android-device-debug-service.ts index 446571690c..b079862778 100644 --- a/lib/services/android-device-debug-service.ts +++ b/lib/services/android-device-debug-service.ts @@ -17,9 +17,7 @@ export class AndroidDeviceDebugService extends DebugServiceBase implements IDevi private $logger: ILogger, private $androidProcessService: Mobile.IAndroidProcessService, private $net: INet, - private $cleanupService: ICleanupService, - private $deviceLogProvider: Mobile.IDeviceLogProvider, - private $staticConfig: IStaticConfig) { + private $deviceLogProvider: Mobile.IDeviceLogProvider) { super(device, $devicesService); this.deviceIdentifier = device.deviceInfo.identifier; @@ -71,7 +69,8 @@ export class AndroidDeviceDebugService extends DebugServiceBase implements IDevi await this.unixSocketForward(port, `${unixSocketName}`); } - await this.$cleanupService.addCleanupCommand({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", deviceId, "forward", "--remove", `tcp:${port}`] }); + // TODO: Uncomment for 6.0.0 release + // await this.$cleanupService.addCleanupCommand({ command: await this.$staticConfig.getAdbFilePath(), args: ["-s", deviceId, "forward", "--remove", `tcp:${port}`] }); return port; } diff --git a/test/services/android-device-debug-service.ts b/test/services/android-device-debug-service.ts index 622e55658a..e3b208008c 100644 --- a/test/services/android-device-debug-service.ts +++ b/test/services/android-device-debug-service.ts @@ -11,10 +11,8 @@ class AndroidDeviceDebugServiceInheritor extends AndroidDeviceDebugService { $logger: ILogger, $androidProcessService: Mobile.IAndroidProcessService, $net: INet, - $deviceLogProvider: Mobile.IDeviceLogProvider, - $cleanupService: ICleanupService, - $staticConfig: IStaticConfig) { - super({ deviceInfo: { identifier: "123" } }, $devicesService, $errors, $logger, $androidProcessService, $net, $cleanupService, $deviceLogProvider, $staticConfig); + $deviceLogProvider: Mobile.IDeviceLogProvider) { + super({ deviceInfo: { identifier: "123" } }, $devicesService, $errors, $logger, $androidProcessService, $net, $deviceLogProvider); } public getChromeDebugUrl(debugOptions: IDebugOptions, port: number): string { From 2d32053675d222d6bbfd901363d84cd27571890f Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 Apr 2019 11:18:45 +0300 Subject: [PATCH 16/16] fix: handle disposing of cleanup-process Currently the cleanup-process is never disconnected from CLI as in the `commandsService` we set the `shouldDispose` property to false. This breaks short living commands like `tns create`, `tns devices`, etc. Handle the disposing similar to the other long living commands in CLI. --- lib/commands/debug.ts | 4 +++- lib/commands/preview.ts | 6 ++++-- lib/commands/test.ts | 8 ++++++-- lib/common/commands/device/device-log-stream.ts | 4 +++- lib/common/services/commands-service.ts | 4 +--- lib/helpers/livesync-command-helper.ts | 15 ++++++++++----- lib/services/cleanup-service.ts | 2 +- 7 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lib/commands/debug.ts b/lib/commands/debug.ts index 95457625e0..ab62e8ac56 100644 --- a/lib/commands/debug.ts +++ b/lib/commands/debug.ts @@ -91,7 +91,8 @@ export class DebugIOSCommand implements ICommand { private $sysInfo: ISysInfo, private $projectData: IProjectData, $iosDeviceOperations: IIOSDeviceOperations, - $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider) { + $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider, + $cleanupService: ICleanupService) { this.$projectData.initializeProjectData(); // Do not dispose ios-device-lib, so the process will remain alive and the debug application (NativeScript Inspector or Chrome DevTools) will be able to connect to the socket. // In case we dispose ios-device-lib, the socket will be closed and the code will fail when the debug application tries to read/send data to device socket. @@ -99,6 +100,7 @@ export class DebugIOSCommand implements ICommand { // In case we do not set it to false, the dispose will be called once the command finishes its execution, which will prevent the debugging. $iosDeviceOperations.setShouldDispose(false); $iOSSimulatorLogProvider.setShouldDispose(false); + $cleanupService.setShouldDispose(false); } public execute(args: string[]): Promise { diff --git a/lib/commands/preview.ts b/lib/commands/preview.ts index 1dddd24a3c..bb2aab8c35 100644 --- a/lib/commands/preview.ts +++ b/lib/commands/preview.ts @@ -13,8 +13,10 @@ export class PreviewCommand implements ICommand { private $projectData: IProjectData, private $options: IOptions, private $previewAppLogProvider: IPreviewAppLogProvider, - private $previewQrCodeService: IPreviewQrCodeService) { - this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); + private $previewQrCodeService: IPreviewQrCodeService, + $cleanupService: ICleanupService) { + this.$analyticsService.setShouldDispose(false); + $cleanupService.setShouldDispose(false); } public async execute(): Promise { diff --git a/lib/commands/test.ts b/lib/commands/test.ts index 62cc07418d..e96d981793 100644 --- a/lib/commands/test.ts +++ b/lib/commands/test.ts @@ -10,6 +10,7 @@ abstract class TestCommandBase { protected abstract $options: IOptions; protected abstract $platformEnvironmentRequirements: IPlatformEnvironmentRequirements; protected abstract $errors: IErrors; + protected abstract $cleanupService: ICleanupService; async execute(args: string[]): Promise { await this.$testExecutionService.startKarmaServer(this.platform, this.$projectData, this.projectFilesConfig); @@ -18,6 +19,7 @@ abstract class TestCommandBase { async canExecute(args: string[]): Promise { this.$projectData.initializeProjectData(); this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); + this.$cleanupService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); this.projectFilesConfig = helpers.getProjectFilesConfig({ isReleaseBuild: this.$options.release }); const output = await this.$platformEnvironmentRequirements.checkEnvironmentRequirements({ @@ -51,7 +53,8 @@ class TestAndroidCommand extends TestCommandBase implements ICommand { protected $analyticsService: IAnalyticsService, protected $options: IOptions, protected $platformEnvironmentRequirements: IPlatformEnvironmentRequirements, - protected $errors: IErrors) { + protected $errors: IErrors, + protected $cleanupService: ICleanupService) { super(); } @@ -65,7 +68,8 @@ class TestIosCommand extends TestCommandBase implements ICommand { protected $analyticsService: IAnalyticsService, protected $options: IOptions, protected $platformEnvironmentRequirements: IPlatformEnvironmentRequirements, - protected $errors: IErrors) { + protected $errors: IErrors, + protected $cleanupService: ICleanupService) { super(); } diff --git a/lib/common/commands/device/device-log-stream.ts b/lib/common/commands/device/device-log-stream.ts index 52154e28a9..37b4ad0e2d 100644 --- a/lib/common/commands/device/device-log-stream.ts +++ b/lib/common/commands/device/device-log-stream.ts @@ -7,8 +7,10 @@ export class OpenDeviceLogStreamCommand implements ICommand { private $options: IOptions, private $deviceLogProvider: Mobile.IDeviceLogProvider, private $loggingLevels: Mobile.ILoggingLevels, - $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider) { + $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider, + $cleanupService: ICleanupService) { $iOSSimulatorLogProvider.setShouldDispose(false); + $cleanupService.setShouldDispose(false); } allowedParameters: ICommandParameter[] = []; diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index 1f9ccfde2e..d441a2959a 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -28,8 +28,7 @@ export class CommandsService implements ICommandsService { private $helpService: IHelpService, private $extensibilityService: IExtensibilityService, private $optionsTracker: IOptionsTracker, - private $projectDataService: IProjectDataService, - private $cleanupService: ICleanupService) { + private $projectDataService: IProjectDataService) { let projectData = null; try { projectData = this.$projectDataService.getProjectData(); @@ -38,7 +37,6 @@ export class CommandsService implements ICommandsService { } this.$options.setupOptions(projectData); - this.$cleanupService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); } public allCommands(opts: { includeDevCommands: boolean }): string[] { diff --git a/lib/helpers/livesync-command-helper.ts b/lib/helpers/livesync-command-helper.ts index 17c77326f2..8b5c648298 100644 --- a/lib/helpers/livesync-command-helper.ts +++ b/lib/helpers/livesync-command-helper.ts @@ -15,8 +15,8 @@ export class LiveSyncCommandHelper implements ILiveSyncCommandHelper { private $bundleValidatorHelper: IBundleValidatorHelper, private $errors: IErrors, private $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider, - private $logger: ILogger) { - this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch); + private $logger: ILogger, + private $cleanupService: ICleanupService) { } public getPlatformsForOperation(platform: string): string[] { @@ -59,9 +59,14 @@ export class LiveSyncCommandHelper implements ILiveSyncCommandHelper { const workingWithiOSDevices = !platform || this.$mobileHelper.isiOSPlatform(platform); const shouldKeepProcessAlive = this.$options.watch || !this.$options.justlaunch; - if (workingWithiOSDevices && shouldKeepProcessAlive) { - this.$iosDeviceOperations.setShouldDispose(false); - this.$iOSSimulatorLogProvider.setShouldDispose(false); + if (shouldKeepProcessAlive) { + this.$analyticsService.setShouldDispose(false); + this.$cleanupService.setShouldDispose(false); + + if (workingWithiOSDevices) { + this.$iosDeviceOperations.setShouldDispose(false); + this.$iOSSimulatorLogProvider.setShouldDispose(false); + } } if (this.$options.release) { diff --git a/lib/services/cleanup-service.ts b/lib/services/cleanup-service.ts index 137f95f85a..671f6afed6 100644 --- a/lib/services/cleanup-service.ts +++ b/lib/services/cleanup-service.ts @@ -13,7 +13,7 @@ export class CleanupService implements ICleanupService { this.pathToCleanupLogFile = $options.cleanupLogFile; } - public shouldDispose = false; + public shouldDispose = true; public async addCleanupCommand(commandInfo: ISpawnCommandInfo): Promise { const cleanupProcess = await this.getCleanupProcess();