From 45dbbb14328a173cad05ddcf21b5acf7f6460bb8 Mon Sep 17 00:00:00 2001 From: Shinigami Date: Sun, 14 Feb 2021 00:05:23 +0100 Subject: [PATCH 1/1] Clean worker from pool after it was destroyed (#146) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Co-authored-by: Jérôme Benoit --- src/pools/abstract-pool.ts | 7 +++---- src/pools/cluster/dynamic.ts | 1 - src/pools/cluster/fixed.ts | 1 - src/pools/thread/dynamic.ts | 1 - src/pools/thread/fixed.ts | 1 - tests/pools/cluster/fixed.test.js | 2 +- 6 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 6e4465ed..f6ed280c 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -24,6 +24,7 @@ export interface IWorker { on(event: 'error', handler: ErrorHandler): void on(event: 'online', handler: OnlineHandler): void on(event: 'exit', handler: ExitHandler): void + once(event: 'exit', handler: ExitHandler): void } /** @@ -161,9 +162,7 @@ export abstract class AbstractPool< } public async destroy (): Promise { - for (const worker of this.workers) { - await this.destroyWorker(worker) - } + await Promise.all(this.workers.map(worker => this.destroyWorker(worker))) } /** @@ -288,8 +287,8 @@ export abstract class AbstractPool< worker.on('error', this.opts.errorHandler ?? (() => {})) worker.on('online', this.opts.onlineHandler ?? (() => {})) - // TODO handle properly when a worker exit worker.on('exit', this.opts.exitHandler ?? (() => {})) + worker.once('exit', () => this.removeWorker(worker)) this.workers.push(worker) diff --git a/src/pools/cluster/dynamic.ts b/src/pools/cluster/dynamic.ts index 6d665cef..432feb3a 100644 --- a/src/pools/cluster/dynamic.ts +++ b/src/pools/cluster/dynamic.ts @@ -68,7 +68,6 @@ export class DynamicClusterPool< if (message.kill) { this.sendToWorker(worker, { kill: 1 }) void this.destroyWorker(worker) - this.removeWorker(worker) } }) return worker diff --git a/src/pools/cluster/fixed.ts b/src/pools/cluster/fixed.ts index 0d8021eb..6620d5f3 100644 --- a/src/pools/cluster/fixed.ts +++ b/src/pools/cluster/fixed.ts @@ -60,7 +60,6 @@ export class FixedClusterPool< protected destroyWorker (worker: Worker): void { worker.kill() - // FIXME: The tests are currently failing, so these must be changed first } protected sendToWorker (worker: Worker, message: MessageValue): void { diff --git a/src/pools/thread/dynamic.ts b/src/pools/thread/dynamic.ts index c1033315..9ab6bf30 100644 --- a/src/pools/thread/dynamic.ts +++ b/src/pools/thread/dynamic.ts @@ -68,7 +68,6 @@ export class DynamicThreadPool< if (message.kill) { this.sendToWorker(worker, { kill: 1 }) void this.destroyWorker(worker) - this.removeWorker(worker) } }) return worker diff --git a/src/pools/thread/fixed.ts b/src/pools/thread/fixed.ts index 74c14ff2..bfd6f016 100644 --- a/src/pools/thread/fixed.ts +++ b/src/pools/thread/fixed.ts @@ -48,7 +48,6 @@ export class FixedThreadPool< worker: ThreadWorkerWithMessageChannel ): Promise { await worker.terminate() - // FIXME: The tests are currently failing, so these must be changed first } protected sendToWorker ( diff --git a/tests/pools/cluster/fixed.test.js b/tests/pools/cluster/fixed.test.js index aae9127f..d22f697a 100644 --- a/tests/pools/cluster/fixed.test.js +++ b/tests/pools/cluster/fixed.test.js @@ -120,7 +120,7 @@ describe('Fixed cluster pool test suite ', () => { closedWorkers++ }) }) - pool.destroy() + await pool.destroy() await new Promise(resolve => setTimeout(resolve, 200)) expect(closedWorkers).toBe(numberOfWorkers) }) -- 2.34.1