diff --git a/lib/processors/minifier.js b/lib/processors/minifier.js index b70d4dd02..ae6796d30 100644 --- a/lib/processors/minifier.js +++ b/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; diff --git a/lib/tasks/buildThemes.js b/lib/tasks/buildThemes.js index 4660b1d15..ad7753162 100644 --- a/lib/tasks/buildThemes.js +++ b/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; diff --git a/test/lib/processors/minifier.js b/test/lib/processors/minifier.js index a2ebf6a3d..de9d0a4c5 100644 --- a/test/lib/processors/minifier.js +++ b/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} diff --git a/test/lib/tasks/buildThemes.js b/test/lib/tasks/buildThemes.js index 48fd06a8f..a016f2b98 100644 --- a/test/lib/tasks/buildThemes.js +++ b/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."); +});