Merge pull request #4779 from NativeScript/kddimitrov/fix-hanging-webpack-process

Kddimitrov/fix hanging webpack process

Author: rosen-vladimirov

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

lib/controllers/prepare-controller.ts

@@ -55,7 +55,7 @@ export class PrepareController extends EventEmitter {
 		return result;
 	}
 
-	public stopWatchers(projectDir: string, platform: string): void {
+	public async stopWatchers(projectDir: string, platform: string): Promise<void> {
 		const platformLowerCase = platform.toLowerCase();
 
 		if (this.watchersData && this.watchersData[projectDir] && this.watchersData[projectDir][platformLowerCase] && this.watchersData[projectDir][platformLowerCase].nativeFilesWatcher) {
@@ -64,7 +64,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;
 		}
 	}

lib/controllers/run-controller.ts

@@ -68,22 +68,23 @@ 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) {
 				if (liveSyncProcessInfo.timer) {
 					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;
 

lib/definitions/cleanup-service.d.ts

@@ -56,4 +56,18 @@ interface ICleanupService extends IShouldDispose, IDisposable {
 	 * @returns {Promise<void>}
 	 */
 	removeCleanupJS(jsCommand: IJSCommand): Promise<void>;
+
+	/**
+	 * Adds a kill action for the process
+	 * @param pid the pid of the process to be killed
+	 * @returns {Promise<void>}
+	 */
+	addKillProcess(pid: string): Promise<void>;
+
+	/**
+	 * Removes the kill action for the process
+	 * @param pid the pid of the process to be killed
+	 * @returns {Promise<void>}
+	 */
+	removeKillProcess(pid: string): Promise<void>;
 }

lib/definitions/prepare.d.ts

@@ -23,7 +23,7 @@ declare global {
 
 	interface IPrepareController extends EventEmitter {
 		prepare(prepareData: IPrepareData): Promise<IPrepareResultData>;
-		stopWatchers(projectDir: string, platform: string): void;
+		stopWatchers(projectDir: string, platform: string): Promise<void>;
 	}
 
 	interface IPrepareResultData {

lib/services/cleanup-service.ts

@@ -45,6 +45,16 @@ export class CleanupService implements ICleanupService {
 		cleanupProcess.send(<IJSCleanupMessage>{ messageType: CleanupProcessMessage.RemoveJSFileToRequire, jsCommand});
 	}
 
+	public async addKillProcess(pid: string): Promise<void> {
+		const killSpawnCommandInfo = this.getKillProcesSpawnInfo(pid);
+		await this.addCleanupCommand(killSpawnCommandInfo);
+	}
+
+	public async removeKillProcess(pid: string): Promise<void> {
+		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);

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<any> {
@@ -70,15 +71,19 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp
 				}
 			});
 
-			childProcess.on("close", (arg: any) => {
+			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;
-				if (exitCode === 0) {
-					resolve(childProcess);
-				} else {
-					const error = new Error(`Executing webpack failed with exit code ${exitCode}.`);
-					error.code = exitCode;
-					reject(error);
-				}
+				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);
 			});
 		});
 	}
@@ -91,7 +96,14 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp
 			}
 
 			const childProcess = await this.startWebpackProcess(platformData, projectData, prepareData);
-			childProcess.on("close", (arg: any) => {
+			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();
@@ -104,11 +116,15 @@ export class WebpackCompilerService extends EventEmitter implements IWebpackComp
 		});
 	}
 
-	public stopWebpackCompiler(platform: string): void {
+	public async stopWebpackCompiler(platform: string): Promise<void> {
 		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]);
+			}
 		}
 	}
 
@@ -136,9 +152,10 @@ 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());
 
 		return childProcess;
 	}
@@ -233,9 +250,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];

lib/services/webpack/webpack.d.ts

@@ -6,7 +6,7 @@ declare global {
 	interface IWebpackCompilerService extends EventEmitter {
 		compileWithWatch(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise<any>;
 		compileWithoutWatch(platformData: IPlatformData, projectData: IProjectData, prepareData: IPrepareData): Promise<void>;
-		stopWebpackCompiler(platform: string): void;
+		stopWebpackCompiler(platform: string): Promise<void>;
 	}
 
 	interface IWebpackEnvOptions {