From 711623b84018591e3613b2d3520babc909a3aec1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 14 Nov 2023 21:30:45 +0100 Subject: [PATCH] fix: check pool statuses MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- CHANGELOG.md | 4 ++++ src/pools/abstract-pool.ts | 36 +++++++++++++++++++++++++++++- tests/pools/abstract-pool.test.mjs | 28 +++++++++++++++++++---- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47236a79..c1cad104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Ensure pool statuses are checked at initialization, `start()` or `destroy()` + ## [3.0.5] - 2023-10-27 ### Fixed diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 604c7af8..5392914d 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -113,6 +113,10 @@ export abstract class AbstractPool< * Whether the pool is starting or not. */ private starting: boolean + /** + * Whether the pool is destroying or not. + */ + private destroying: boolean /** * The start timestamp of the pool. */ @@ -162,6 +166,7 @@ export abstract class AbstractPool< this.started = false this.starting = false + this.destroying = false if (this.opts.startWorkers === true) { this.start() } @@ -886,6 +891,10 @@ export abstract class AbstractPool< reject(new Error('Cannot execute a task on not started pool')) return } + if (this.destroying) { + reject(new Error('Cannot execute a task on destroying pool')) + return + } if (name != null && typeof name !== 'string') { reject(new TypeError('name argument must be a string')) return @@ -931,6 +940,15 @@ export abstract class AbstractPool< /** @inheritdoc */ public start (): void { + if (this.started) { + throw new Error('Cannot start an already started pool') + } + if (this.starting) { + throw new Error('Cannot start an already starting pool') + } + if (this.destroying) { + throw new Error('Cannot start a destroying pool') + } this.starting = true while ( this.workerNodes.reduce( @@ -947,6 +965,16 @@ export abstract class AbstractPool< /** @inheritDoc */ public async destroy (): Promise { + if (!this.started) { + throw new Error('Cannot destroy an already destroyed pool') + } + if (this.starting) { + throw new Error('Cannot destroy an starting pool') + } + if (this.destroying) { + throw new Error('Cannot destroy an already destroying pool') + } + this.destroying = true await Promise.all( this.workerNodes.map(async (_, workerNodeKey) => { await this.destroyWorkerNode(workerNodeKey) @@ -954,6 +982,7 @@ export abstract class AbstractPool< ) this.emitter?.emit(PoolEvents.destroy, this.info) this.emitter?.emitDestroy() + this.destroying = false this.started = false } @@ -1228,6 +1257,7 @@ export abstract class AbstractPool< if ( this.started && !this.starting && + !this.destroying && this.opts.restartWorkerOnError === true ) { if (workerInfo.dynamic) { @@ -1236,7 +1266,11 @@ export abstract class AbstractPool< this.createAndSetupWorkerNode() } } - if (this.started && this.opts.enableTasksQueue === true) { + if ( + this.started && + !this.destroying && + this.opts.enableTasksQueue === true + ) { this.redistributeQueuedTasks(workerNodeKey) } }) diff --git a/tests/pools/abstract-pool.test.mjs b/tests/pools/abstract-pool.test.mjs index 41f4751f..535eb7e9 100644 --- a/tests/pools/abstract-pool.test.mjs +++ b/tests/pools/abstract-pool.test.mjs @@ -69,8 +69,9 @@ describe('Abstract pool test suite', () => { numberOfWorkers, './tests/worker-files/thread/testWorker.mjs' ) - expect(pool.starting).toBe(false) expect(pool.started).toBe(true) + expect(pool.starting).toBe(false) + expect(pool.destroying).toBe(false) await pool.destroy() }) @@ -886,6 +887,24 @@ describe('Abstract pool test suite', () => { await pool.destroy() }) + it('Verify that pool statuses are checked at start or destroy', async () => { + const pool = new FixedThreadPool( + numberOfWorkers, + './tests/worker-files/thread/testWorker.mjs' + ) + expect(pool.info.started).toBe(true) + expect(pool.info.ready).toBe(true) + expect(() => pool.start()).toThrow( + new Error('Cannot start an already started pool') + ) + await pool.destroy() + expect(pool.info.started).toBe(false) + expect(pool.info.ready).toBe(false) + await expect(pool.destroy()).rejects.toThrow( + new Error('Cannot destroy an already destroyed pool') + ) + }) + it('Verify that pool can be started after initialization', async () => { const pool = new FixedClusterPool( numberOfWorkers, @@ -1415,23 +1434,24 @@ describe('Abstract pool test suite', () => { './tests/worker-files/thread/testMultipleTaskFunctionsWorker.mjs' ) await waitPoolEvents(dynamicThreadPool, PoolEvents.ready, 1) + const workerId = dynamicThreadPool.workerNodes[0].info.id await expect(dynamicThreadPool.setDefaultTaskFunction(0)).rejects.toThrow( new Error( - "Task function operation 'default' failed on worker 33 with error: 'TypeError: name parameter is not a string'" + `Task function operation 'default' failed on worker ${workerId} with error: 'TypeError: name parameter is not a string'` ) ) await expect( dynamicThreadPool.setDefaultTaskFunction(DEFAULT_TASK_NAME) ).rejects.toThrow( new Error( - "Task function operation 'default' failed on worker 33 with error: 'Error: Cannot set the default task function reserved name as the default task function'" + `Task function operation 'default' failed on worker ${workerId} with error: 'Error: Cannot set the default task function reserved name as the default task function'` ) ) await expect( dynamicThreadPool.setDefaultTaskFunction('unknown') ).rejects.toThrow( new Error( - "Task function operation 'default' failed on worker 33 with error: 'Error: Cannot set the default task function to a non-existing task function'" + `Task function operation 'default' failed on worker ${workerId} with error: 'Error: Cannot set the default task function to a non-existing task function'` ) ) expect(dynamicThreadPool.listTaskFunctionNames()).toStrictEqual([ -- 2.34.1