From 1adce66a7f0658ab4942fe3b308f9c4f88e6b94f Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Wed, 26 Jun 2019 19:33:48 +0300 Subject: [PATCH 1/3] fix: webpack process leaks --- lib/bash-scripts/terminateProcess.sh | 12 +++++++ lib/controllers/prepare-controller.ts | 4 +-- lib/controllers/run-controller.ts | 13 ++++---- lib/definitions/cleanup-service.d.ts | 14 +++++++++ lib/definitions/prepare.d.ts | 2 +- lib/services/cleanup-service.ts | 31 +++++++++++++++++++ .../webpack/webpack-compiler-service.ts | 17 +++++++--- lib/services/webpack/webpack.d.ts | 2 +- 8 files changed, 80 insertions(+), 15 deletions(-) create mode 100755 lib/bash-scripts/terminateProcess.sh diff --git a/lib/bash-scripts/terminateProcess.sh b/lib/bash-scripts/terminateProcess.sh new file mode 100755 index 0000000000..908a68287f --- /dev/null +++ b/lib/bash-scripts/terminateProcess.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +terminateTree() { + for cpid in $(/usr/bin/pgrep -P $1); do + terminateTree $cpid + done + kill -9 $1 > /dev/null 2>&1 +} + +for pid in $*; do + terminateTree $pid +done \ No newline at end of file diff --git a/lib/controllers/prepare-controller.ts b/lib/controllers/prepare-controller.ts index 03febd68ef..6c874e8289 100644 --- a/lib/controllers/prepare-controller.ts +++ b/lib/controllers/prepare-controller.ts @@ -54,7 +54,7 @@ export class PrepareController extends EventEmitter { return result; } - public stopWatchers(projectDir: string, platform: string): void { + public async stopWatchers(projectDir: string, platform: string): Promise { const platformLowerCase = platform.toLowerCase(); if (this.watchersData && this.watchersData[projectDir] && this.watchersData[projectDir][platformLowerCase] && this.watchersData[projectDir][platformLowerCase].nativeFilesWatcher) { @@ -63,7 +63,7 @@ export class PrepareController extends EventEmitter { } if (this.watchersData && this.watchersData[projectDir] && this.watchersData[projectDir][platformLowerCase] && this.watchersData[projectDir][platformLowerCase].webpackCompilerProcess) { - this.$webpackCompilerService.stopWebpackCompiler(platform); + await this.$webpackCompilerService.stopWebpackCompiler(platform); this.watchersData[projectDir][platformLowerCase].webpackCompilerProcess = null; } } diff --git a/lib/controllers/run-controller.ts b/lib/controllers/run-controller.ts index fad79b8705..9e1fb59621 100644 --- a/lib/controllers/run-controller.ts +++ b/lib/controllers/run-controller.ts @@ -70,12 +70,13 @@ export class RunController extends EventEmitter implements IRunController { .map(descriptor => descriptor.identifier); // Handle the case when no more devices left for any of the persisted platforms - _.each(liveSyncProcessInfo.platforms, platform => { + for (let i = 0; i < liveSyncProcessInfo.platforms.length; i++) { + const platform = liveSyncProcessInfo.platforms[i]; const devices = this.$devicesService.getDevicesForPlatform(platform); if (!devices || !devices.length) { - this.$prepareController.stopWatchers(projectDir, platform); + await this.$prepareController.stopWatchers(projectDir, platform); } - }); + } // In case deviceIdentifiers are not passed, we should stop the whole LiveSync. if (!deviceIdentifiers || !deviceIdentifiers.length || !liveSyncProcessInfo.deviceDescriptors || !liveSyncProcessInfo.deviceDescriptors.length) { @@ -83,9 +84,9 @@ export class RunController extends EventEmitter implements IRunController { clearTimeout(liveSyncProcessInfo.timer); } - _.each(liveSyncProcessInfo.platforms, platform => { - this.$prepareController.stopWatchers(projectDir, platform); - }); + for (let k = 0; k < liveSyncProcessInfo.platforms.length; k++) { + await this.$prepareController.stopWatchers(projectDir, liveSyncProcessInfo.platforms[k]); + } liveSyncProcessInfo.isStopped = true; diff --git a/lib/definitions/cleanup-service.d.ts b/lib/definitions/cleanup-service.d.ts index abec414790..0f050b5f3d 100644 --- a/lib/definitions/cleanup-service.d.ts +++ b/lib/definitions/cleanup-service.d.ts @@ -56,4 +56,18 @@ interface ICleanupService extends IShouldDispose, IDisposable { * @returns {Promise} */ removeCleanupJS(jsCommand: IJSCommand): Promise; + + /** + * Adds a kill action for the process + * @param pid the pid of the process to be killed + * @returns {Promise} + */ + addKillProcess(pid: string): Promise; + + /** + * Removes the kill action for the process + * @param pid the pid of the process to be killed + * @returns {Promise} + */ + removeKillProcess(pid: string): Promise; } diff --git a/lib/definitions/prepare.d.ts b/lib/definitions/prepare.d.ts index c8ca2e0012..43f93ae225 100644 --- a/lib/definitions/prepare.d.ts +++ b/lib/definitions/prepare.d.ts @@ -23,7 +23,7 @@ declare global { interface IPrepareController extends EventEmitter { prepare(prepareData: IPrepareData): Promise; - stopWatchers(projectDir: string, platform: string): void; + stopWatchers(projectDir: string, platform: string): Promise; } interface IPrepareResultData { diff --git a/lib/services/cleanup-service.ts b/lib/services/cleanup-service.ts index 953f330e25..723810bd0b 100644 --- a/lib/services/cleanup-service.ts +++ b/lib/services/cleanup-service.ts @@ -45,6 +45,16 @@ export class CleanupService implements ICleanupService { cleanupProcess.send({ messageType: CleanupProcessMessage.RemoveJSFileToRequire, jsCommand}); } + public async addKillProcess(pid: string): Promise { + const killSpawnCommandInfo = this.getKillProcesSpawnInfo(pid); + await this.addCleanupCommand(killSpawnCommandInfo); + } + + public async removeKillProcess(pid: string): Promise { + const killSpawnCommandInfo = this.getKillProcesSpawnInfo(pid); + await this.removeCleanupCommand(killSpawnCommandInfo); + } + @exported("cleanupService") public setCleanupLogFile(filePath: string): void { this.pathToCleanupLogFile = filePath; @@ -121,6 +131,27 @@ export class CleanupService implements ICleanupService { return cleanupProcessArgs; } + + private getKillProcesSpawnInfo(pid: string): ISpawnCommandInfo { + let command; + let args; + switch (process.platform) { + case 'win32': + command = "taskkill"; + args = ["/pid", pid, "/T", "/F"]; + break; + + default: + command = path.join(__dirname, '../bash-scripts/terminateProcess.sh'); + args = [pid]; + break; + } + + return { + command, + args + }; + } } $injector.register("cleanupService", CleanupService); diff --git a/lib/services/webpack/webpack-compiler-service.ts b/lib/services/webpack/webpack-compiler-service.ts index c79754c44f..d95dd877af 100644 --- a/lib/services/webpack/webpack-compiler-service.ts +++ b/lib/services/webpack/webpack-compiler-service.ts @@ -14,7 +14,8 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp public $hostInfo: IHostInfo, private $logger: ILogger, private $pluginsService: IPluginsService, - private $mobileHelper: Mobile.IMobileHelper + private $mobileHelper: Mobile.IMobileHelper, + private $cleanupService: ICleanupService ) { super(); } public async compileWithWatch(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise { @@ -104,11 +105,15 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp }); } - public stopWebpackCompiler(platform: string): void { + public async stopWebpackCompiler(platform: string): Promise { if (platform) { - this.stopWebpackForPlatform(platform); + await this.stopWebpackForPlatform(platform); } else { - Object.keys(this.webpackProcesses).forEach(pl => this.stopWebpackForPlatform(pl)); + const webpackedPlatforms = Object.keys(this.webpackProcesses); + + for (let i = 0; i < webpackedPlatforms.length; i++) { + await this.stopWebpackForPlatform(webpackedPlatforms[i]); + } } } @@ -139,6 +144,7 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp const childProcess = this.$childProcess.spawn("node", args, { cwd: projectData.projectDir, stdio }); this.webpackProcesses[platformData.platformNameLowerCase] = childProcess; + await this.$cleanupService.addKillProcess(childProcess.pid.toString()); return childProcess; } @@ -233,9 +239,10 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp }; } - private stopWebpackForPlatform(platform: string) { + private async stopWebpackForPlatform(platform: string) { this.$logger.trace(`Stopping webpack watch for platform ${platform}.`); const webpackProcess = this.webpackProcesses[platform]; + await this.$cleanupService.removeKillProcess(webpackProcess.pid.toString()); if (webpackProcess) { webpackProcess.kill("SIGINT"); delete this.webpackProcesses[platform]; diff --git a/lib/services/webpack/webpack.d.ts b/lib/services/webpack/webpack.d.ts index 16f232478c..7f03b1b3c4 100644 --- a/lib/services/webpack/webpack.d.ts +++ b/lib/services/webpack/webpack.d.ts @@ -6,7 +6,7 @@ declare global { interface IWebpackCompilerService extends EventEmitter { compileWithWatch(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise; compileWithoutWatch(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise; - stopWebpackCompiler(platform: string): void; + stopWebpackCompiler(platform: string): Promise; } interface IWebpackEnvOptions { From 3580ff155449250fbcbf5d1a40c56dd2f3d24ebe Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Thu, 27 Jun 2019 17:34:01 +0300 Subject: [PATCH 2/3] fix: clear kill process for wepback when exit on itself - build --- lib/services/webpack/webpack-compiler-service.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/services/webpack/webpack-compiler-service.ts b/lib/services/webpack/webpack-compiler-service.ts index d95dd877af..08ffcb7ba8 100644 --- a/lib/services/webpack/webpack-compiler-service.ts +++ b/lib/services/webpack/webpack-compiler-service.ts @@ -71,7 +71,7 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp } }); - childProcess.on("close", (arg: any) => { + childProcess.on("close", async (arg: any) => { const exitCode = typeof arg === "number" ? arg : arg && arg.code; if (exitCode === 0) { resolve(childProcess); @@ -80,6 +80,8 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp error.code = exitCode; reject(error); } + + await this.$cleanupService.removeKillProcess(childProcess.pid.toString()); }); }); } @@ -92,7 +94,7 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp } const childProcess = await this.startWebpackProcess(platformData, projectData, prepareData); - childProcess.on("close", (arg: any) => { + childProcess.on("close", async (arg: any) => { const exitCode = typeof arg === "number" ? arg : arg && arg.code; if (exitCode === 0) { resolve(); @@ -101,6 +103,8 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp error.code = exitCode; reject(error); } + + await this.$cleanupService.removeKillProcess(childProcess.pid.toString()); }); }); } From 97cacbffe58b84f6483306e168a4472c5718fbf2 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Fri, 28 Jun 2019 09:11:11 +0300 Subject: [PATCH 3/3] fix: handle correctly close event of webpack process When we run webpack in watch mode and we receive its close event, it means the process had exited and we are not in watch mode anymore. So in this case we have to reject the current promise. --- .../webpack/webpack-compiler-service.ts | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/lib/services/webpack/webpack-compiler-service.ts b/lib/services/webpack/webpack-compiler-service.ts index 08ffcb7ba8..d238bcc7a8 100644 --- a/lib/services/webpack/webpack-compiler-service.ts +++ b/lib/services/webpack/webpack-compiler-service.ts @@ -71,17 +71,19 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp } }); - childProcess.on("close", async (arg: any) => { - const exitCode = typeof arg === "number" ? arg : arg && arg.code; - if (exitCode === 0) { - resolve(childProcess); - } else { - const error = new Error(`Executing webpack failed with exit code ${exitCode}.`); - error.code = exitCode; - reject(error); - } + childProcess.on("error", (err) => { + this.$logger.trace(`Unable to start webpack process in watch mode. Error is: ${err}`); + reject(err); + }); + childProcess.on("close", async (arg: any) => { await this.$cleanupService.removeKillProcess(childProcess.pid.toString()); + + const exitCode = typeof arg === "number" ? arg : arg && arg.code; + this.$logger.trace(`Webpack process exited with code ${exitCode} when we expected it to be long living with watch.`); + const error = new Error(`Executing webpack failed with exit code ${exitCode}.`); + error.code = exitCode; + reject(error); }); }); } @@ -94,7 +96,14 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp } const childProcess = await this.startWebpackProcess(platformData, projectData, prepareData); + childProcess.on("error", (err) => { + this.$logger.trace(`Unable to start webpack process in non-watch mode. Error is: ${err}`); + reject(err); + }); + childProcess.on("close", async (arg: any) => { + await this.$cleanupService.removeKillProcess(childProcess.pid.toString()); + const exitCode = typeof arg === "number" ? arg : arg && arg.code; if (exitCode === 0) { resolve(); @@ -103,8 +112,6 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp error.code = exitCode; reject(error); } - - await this.$cleanupService.removeKillProcess(childProcess.pid.toString()); }); }); } @@ -145,7 +152,7 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp } const stdio = prepareData.watch ? ["inherit", "inherit", "inherit", "ipc"] : "inherit"; - const childProcess = this.$childProcess.spawn("node", args, { cwd: projectData.projectDir, stdio }); + const childProcess = this.$childProcess.spawn(process.execPath, args, { cwd: projectData.projectDir, stdio }); this.webpackProcesses[platformData.platformNameLowerCase] = childProcess; await this.$cleanupService.addKillProcess(childProcess.pid.toString());