[FIX] Handle graceful termination of workerpool for parallel builds (#…

…953)

fixes: SAP/ui5-tooling#894
JIRA: CPOUI5FOUNDATION-751
depends on: SAP/ui5-project#677

Workerpool needs to wait for all the active tasks to complete before
terminating.

---------

Co-authored-by: Merlin Beutlberger <m.beutlberger@sap.com>

lib/processors/minifier.js

@@ -6,6 +6,7 @@ import workerpool from "workerpool";
 import Resource from "@ui5/fs/Resource";
 import {getLogger} from "@ui5/logger";
 const log = getLogger("builder:processors:minifier");
+import {setTimeout as setTimeoutPromise} from "node:timers/promises";
 
 const debugFileRegex = /((?:\.view|\.fragment|\.controller|\.designtime|\.support)?\.js)$/;
 
@@ -28,11 +29,28 @@ function getPool(taskUtil) {
 			workerType: "auto",
 			maxWorkers
 		});
-		taskUtil.registerCleanupTask(() => {
-			log.verbose(`Terminating workerpool`);
-			const poolToBeTerminated = pool;
-			pool = null;
-			poolToBeTerminated.terminate();
+		taskUtil.registerCleanupTask((force) => {
+			const attemptPoolTermination = async () => {
+				log.verbose(`Attempt to terminate the workerpool...`);
+
+				if (!pool) {
+					return;
+				}
+
+				// There are many stats that could be used, but these ones seem the most
+				// convenient. When all the (available) workers are idle, then it's safe to terminate.
+				let {idleWorkers, totalWorkers} = pool.stats();
+				while (idleWorkers !== totalWorkers && !force) {
+					await setTimeoutPromise(100); // Wait a bit workers to finish and try again
+					({idleWorkers, totalWorkers} = pool.stats());
+				}
+
+				const poolToBeTerminated = pool;
+				pool = null;
+				return poolToBeTerminated.terminate(force);
+			};
+
+			return attemptPoolTermination();
 		});
 	}
 	return pool;

lib/tasks/buildThemes.js

@@ -7,6 +7,7 @@ import {fileURLToPath} from "node:url";
 import os from "node:os";
 import workerpool from "workerpool";
 import {deserializeResources, serializeResources, FsMainThreadInterface} from "../processors/themeBuilderWorker.js";
+import {setTimeout as setTimeoutPromise} from "node:timers/promises";
 
 let pool;
 
@@ -23,11 +24,28 @@ function getPool(taskUtil) {
 			workerType: "thread",
 			maxWorkers
 		});
-		taskUtil.registerCleanupTask(() => {
-			log.verbose(`Terminating workerpool`);
-			const poolToBeTerminated = pool;
-			pool = null;
-			poolToBeTerminated.terminate();
+		taskUtil.registerCleanupTask((force) => {
+			const attemptPoolTermination = async () => {
+				log.verbose(`Attempt to terminate the workerpool...`);
+
+				if (!pool) {
+					return;
+				}
+
+				// There are many stats that could be used, but these ones seem the most
+				// convenient. When all the (available) workers are idle, then it's safe to terminate.
+				let {idleWorkers, totalWorkers} = pool.stats();
+				while (idleWorkers !== totalWorkers && !force) {
+					await setTimeoutPromise(100); // Wait a bit workers to finish and try again
+					({idleWorkers, totalWorkers} = pool.stats());
+				}
+
+				const poolToBeTerminated = pool;
+				pool = null;
+				return poolToBeTerminated.terminate(force);
+			};
+
+			return attemptPoolTermination();
 		});
 	}
 	return pool;

test/lib/processors/minifier.js

@@ -109,6 +109,43 @@ ${SOURCE_MAPPING_URL}=test.controller.js.map`;
 	t.is(taskUtilMock.registerCleanupTask.callCount, 1, "taskUtil#registerCleanupTask got called once");
 });
 
+test("Basic minifier with taskUtil and unexpected termination of the workerpool", async (t) => {
+	const taskUtilMock = {
+		registerCleanupTask: sinon.stub().callsFake((cb) => {
+			// Terminate the workerpool in a timeout, so that
+			// the task is already in the queue, but not yet finished.
+			setTimeout(cb);
+		})
+	};
+
+	// Create more resources so there to be some pending tasks in the pool
+	const testResources = [];
+	for (let i = 0; i < 50; i++) {
+		const content = `/*!
+ * \${copyright}
+ */
+ function myFunc${i}(myArg) {
+ 	jQuery.sap.require("something");
+ 	console.log("Something required")
+ }
+myFunc${i}();
+`;
+		testResources.push(createResource({
+			path: "/test.controller.js",
+			string: content
+		}));
+	}
+	await minifier({
+		resources: testResources,
+		taskUtil: taskUtilMock,
+		options: {
+			useWorkers: true
+		}
+	});
+
+	t.pass("No exception from an earlier workerpool termination attempt.");
+});
+
 test("minifier with useWorkers: true and missing taskUtil", async (t) => {
 	const content = `/*!
  * \${copyright}

test/lib/tasks/buildThemes.js

@@ -549,3 +549,65 @@ test.serial("buildThemes (useWorkers = true)", async (t) => {
 	t.true(workspace.write.calledWithExactly(transferredResources[1]));
 	t.true(workspace.write.calledWithExactly(transferredResources[2]));
 });
+
+
+test.serial("buildThemes with taskUtil and unexpected termination of the workerpool", async (t) => {
+	const taskUtilMock = {
+		registerCleanupTask: sinon.stub().callsFake((cb) => {
+			// Terminate the workerpool in a timeout, so that
+			// the task is already in the queue, but not yet finished.
+			setTimeout(cb);
+		})
+	};
+	const lessResources = [];
+
+	// Create more resources so there to be some pending tasks in the pool
+	for (let i = 0; i < 50; i++) {
+		lessResources.push({
+			getPath: () => `/resources/test${i}/themes/${i}/library.source.less`,
+			getBuffer: () => Buffer.from(`/** test comment N ${i} */`),
+		});
+	}
+
+	const workspace = {
+		byGlob: async (globPattern) => {
+			if (globPattern === "/resources/test*/themes/**/library.source.less") {
+				return lessResources;
+			} else {
+				return [];
+			}
+		},
+		write: sinon.stub()
+	};
+
+	const cssResource = {path: "/cssResource", buffer: new Uint8Array(2)};
+	const cssRtlResource = {path: "/cssRtlResource", buffer: new Uint8Array(2)};
+	const jsonParametersResource = {path: "/jsonParametersResource", buffer: new Uint8Array(2)};
+
+	t.context.themeBuilderStub.returns([cssResource, cssRtlResource, jsonParametersResource]);
+	t.context.comboByGlob.resolves(lessResources);
+
+	t.context.fsInterfaceStub.returns({
+		readFile: (...args) => {
+			if (/\/resources\/test.*\/themes\/.*\/library\.source\.less/i.test(args[0])) {
+				args[args.length - 1](null, "/** */");
+			} else {
+				args[args.length - 1](null, "{}");
+			}
+		},
+		stat: (...args) => args[args.length - 1](null, {}),
+		readdir: (...args) => args[args.length - 1](null, {}),
+		mkdir: (...args) => args[args.length - 1](null, {}),
+	});
+
+	await buildThemes({
+		workspace,
+		taskUtil: taskUtilMock,
+		options: {
+			projectName: "sap.ui.demo.app",
+			inputPattern: "/resources/test*/themes/**/library.source.less"
+		}
+	});
+
+	t.pass("No exception from an earlier workerpool termination attempt.");
+});