From 42447ec132088d117c9b6a3f99adc7da21828c4a Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Mon, 7 Jul 2025 18:18:21 +0200 Subject: [PATCH] fix: plug more ressource leaks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/pools/worker-node.ts | 1 + src/queues/abstract-fixed-queue.ts | 16 ++++++- src/queues/priority-queue.ts | 52 ++++++++++++---------- src/worker/abstract-worker.ts | 8 +++- tests/queues/fixed-priority-queue.test.mjs | 25 +++++------ tests/queues/fixed-queue.test.mjs | 25 +++++------ 6 files changed, 71 insertions(+), 56 deletions(-) diff --git a/src/pools/worker-node.ts b/src/pools/worker-node.ts index 057c206a9..840b3d42f 100644 --- a/src/pools/worker-node.ts +++ b/src/pools/worker-node.ts @@ -187,6 +187,7 @@ export class WorkerNode break } await waitWorkerExit + this.worker.removeAllListeners() } private closeMessageChannel (): void { diff --git a/src/queues/abstract-fixed-queue.ts b/src/queues/abstract-fixed-queue.ts index cd8969f67..54dee480e 100644 --- a/src/queues/abstract-fixed-queue.ts +++ b/src/queues/abstract-fixed-queue.ts @@ -32,6 +32,16 @@ export abstract class AbstractFixedQueue implements IFixedQueue { /** @inheritdoc */ public clear (): void { + if (this.size > 0) { + let index = this.start + for (let i = 0; i < this.size; i++) { + this.nodeArray[index] = undefined + ++index + if (index === this.capacity) { + index = 0 + } + } + } this.start = 0 this.size = 0 } @@ -78,13 +88,15 @@ export abstract class AbstractFixedQueue implements IFixedQueue { return undefined } const index = this.start + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const data = this.nodeArray[index]!.data + this.nodeArray[index] = undefined ++this.start if (this.start === this.capacity) { this.start = 0 } --this.size - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return this.nodeArray[index]!.data + return data } /** @inheritdoc */ diff --git a/src/queues/priority-queue.ts b/src/queues/priority-queue.ts index cfcf5ca0c..7ae8e6d8e 100644 --- a/src/queues/priority-queue.ts +++ b/src/queues/priority-queue.ts @@ -97,18 +97,8 @@ export class PriorityQueue { let prev: PriorityQueueNode | undefined while (node != null) { if (node.delete(data)) { - if (node.empty() && this.head !== this.tail) { - if (node === this.tail) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.tail = node.next! - } else { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - prev!.next = node.next - if (node === this.head) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.head = prev! - } - } + if (node.empty()) { + this.removePriorityQueueNode(node, prev) } --this.size return true @@ -153,18 +143,9 @@ export class PriorityQueue { const data = targetNode!.dequeue() --this.size // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - if (targetNode!.empty() && this.head !== this.tail) { - if (targetNode === this.tail) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.tail = this.tail.next! - } else { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - prev!.next = targetNode!.next - if (targetNode === this.head) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.head = prev! - } - } + if (targetNode!.empty()) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.removePriorityQueueNode(targetNode!, prev) } return data } @@ -231,4 +212,27 @@ export class PriorityQueue { } return fixedQueue } + + private removePriorityQueueNode ( + nodeToRemove: PriorityQueueNode, + previousNode?: PriorityQueueNode + ): void { + if (this.head === this.tail) { + return + } + + if (nodeToRemove === this.tail) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.tail = nodeToRemove.next! + } else if (nodeToRemove === this.head) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.head = previousNode! + this.head.next = undefined + } else { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + previousNode!.next = nodeToRemove.next + } + + nodeToRemove.next = undefined + } } diff --git a/src/worker/abstract-worker.ts b/src/worker/abstract-worker.ts index f3c36bceb..49536b0b7 100644 --- a/src/worker/abstract-worker.ts +++ b/src/worker/abstract-worker.ts @@ -116,8 +116,12 @@ export abstract class AbstractWorker< >() this.checkWorkerOptions(this.opts) if (!this.isMain) { - // Should be once() but Node.js on windows has a bug that prevents it from working - this.getMainWorker().on('message', this.handleReadyMessage.bind(this)) + if (process.platform === 'win32') { + // Node.js on windows has a bug at worker side message counting + this.getMainWorker().on('message', this.handleReadyMessage.bind(this)) + } else { + this.getMainWorker().once('message', this.handleReadyMessage.bind(this)) + } } } diff --git a/tests/queues/fixed-priority-queue.test.mjs b/tests/queues/fixed-priority-queue.test.mjs index 1f1eb8c31..42d2ec30e 100644 --- a/tests/queues/fixed-priority-queue.test.mjs +++ b/tests/queues/fixed-priority-queue.test.mjs @@ -101,7 +101,7 @@ describe('Fixed priority queue test suite', () => { expect(fixedPriorityQueue.size).toBe(2) expect(rtItem).toBe(2) expect(fixedPriorityQueue.nodeArray).toMatchObject([ - { data: 2, priority: -1 }, + undefined, { data: 1, priority: 0 }, { data: 3, priority: 0 }, ]) @@ -111,8 +111,8 @@ describe('Fixed priority queue test suite', () => { expect(fixedPriorityQueue.size).toBe(1) expect(rtItem).toBe(1) expect(fixedPriorityQueue.nodeArray).toMatchObject([ - { data: 2, priority: -1 }, - { data: 1, priority: 0 }, + undefined, + undefined, { data: 3, priority: 0 }, ]) expect(fixedPriorityQueue.capacity).toBe(queueSize) @@ -121,9 +121,9 @@ describe('Fixed priority queue test suite', () => { expect(fixedPriorityQueue.size).toBe(0) expect(rtItem).toBe(3) expect(fixedPriorityQueue.nodeArray).toMatchObject([ - { data: 2, priority: -1 }, - { data: 1, priority: 0 }, - { data: 3, priority: 0 }, + undefined, + undefined, + undefined, ]) expect(fixedPriorityQueue.capacity).toBe(queueSize) rtItem = fixedPriorityQueue.dequeue() @@ -131,9 +131,9 @@ describe('Fixed priority queue test suite', () => { expect(fixedPriorityQueue.size).toBe(0) expect(rtItem).toBe(undefined) expect(fixedPriorityQueue.nodeArray).toMatchObject([ - { data: 2, priority: -1 }, - { data: 1, priority: 0 }, - { data: 3, priority: 0 }, + undefined, + undefined, + undefined, ]) expect(fixedPriorityQueue.capacity).toBe(queueSize) }) @@ -212,7 +212,7 @@ describe('Fixed priority queue test suite', () => { }) it('Verify clear() behavior', () => { - const fixedPriorityQueue = new FixedPriorityQueue() + const fixedPriorityQueue = new FixedPriorityQueue(2) fixedPriorityQueue.start = 1 fixedPriorityQueue.size = 2 fixedPriorityQueue.nodeArray = [ @@ -222,9 +222,6 @@ describe('Fixed priority queue test suite', () => { fixedPriorityQueue.clear() expect(fixedPriorityQueue.start).toBe(0) expect(fixedPriorityQueue.size).toBe(0) - expect(fixedPriorityQueue.nodeArray).toMatchObject([ - { data: 2, priority: 0 }, - { data: 3, priority: 0 }, - ]) + expect(fixedPriorityQueue.nodeArray).toStrictEqual([undefined, undefined]) }) }) diff --git a/tests/queues/fixed-queue.test.mjs b/tests/queues/fixed-queue.test.mjs index d1231643f..faf10a16d 100644 --- a/tests/queues/fixed-queue.test.mjs +++ b/tests/queues/fixed-queue.test.mjs @@ -99,7 +99,7 @@ describe('Fixed queue test suite', () => { expect(fixedQueue.size).toBe(2) expect(rtItem).toBe(1) expect(fixedQueue.nodeArray).toMatchObject([ - { data: 1, priority: 0 }, + undefined, { data: 2, priority: -1 }, { data: 3, priority: 0 }, ]) @@ -109,8 +109,8 @@ describe('Fixed queue test suite', () => { expect(fixedQueue.size).toBe(1) expect(rtItem).toBe(2) expect(fixedQueue.nodeArray).toMatchObject([ - { data: 1, priority: 0 }, - { data: 2, priority: -1 }, + undefined, + undefined, { data: 3, priority: 0 }, ]) expect(fixedQueue.capacity).toBe(queueSize) @@ -119,9 +119,9 @@ describe('Fixed queue test suite', () => { expect(fixedQueue.size).toBe(0) expect(rtItem).toBe(3) expect(fixedQueue.nodeArray).toMatchObject([ - { data: 1, priority: 0 }, - { data: 2, priority: -1 }, - { data: 3, priority: 0 }, + undefined, + undefined, + undefined, ]) expect(fixedQueue.capacity).toBe(queueSize) rtItem = fixedQueue.dequeue() @@ -129,9 +129,9 @@ describe('Fixed queue test suite', () => { expect(fixedQueue.size).toBe(0) expect(rtItem).toBe(undefined) expect(fixedQueue.nodeArray).toMatchObject([ - { data: 1, priority: 0 }, - { data: 2, priority: -1 }, - { data: 3, priority: 0 }, + undefined, + undefined, + undefined, ]) expect(fixedQueue.capacity).toBe(queueSize) }) @@ -208,7 +208,7 @@ describe('Fixed queue test suite', () => { }) it('Verify clear() behavior', () => { - const fixedQueue = new FixedQueue() + const fixedQueue = new FixedQueue(2) fixedQueue.start = 1 fixedQueue.size = 2 fixedQueue.nodeArray = [ @@ -218,9 +218,6 @@ describe('Fixed queue test suite', () => { fixedQueue.clear() expect(fixedQueue.start).toBe(0) expect(fixedQueue.size).toBe(0) - expect(fixedQueue.nodeArray).toMatchObject([ - { data: 2, priority: 0 }, - { data: 3, priority: 0 }, - ]) + expect(fixedQueue.nodeArray).toStrictEqual([undefined, undefined]) }) }) -- 2.43.0