]> Piment Noir Git Repositories - poolifier.git/commitdiff
fix: plug more ressource leaks
authorJérôme Benoit <jerome.benoit@piment-noir.org>
Mon, 7 Jul 2025 16:18:21 +0000 (18:18 +0200)
committerJérôme Benoit <jerome.benoit@piment-noir.org>
Mon, 7 Jul 2025 16:18:21 +0000 (18:18 +0200)
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
src/pools/worker-node.ts
src/queues/abstract-fixed-queue.ts
src/queues/priority-queue.ts
src/worker/abstract-worker.ts
tests/queues/fixed-priority-queue.test.mjs
tests/queues/fixed-queue.test.mjs

index 057c206a96b14e8b69f385c2db03b9d8d3e00bc5..840b3d42f9d0898788c3d53e8714a0efdf202001 100644 (file)
@@ -187,6 +187,7 @@ export class WorkerNode<Worker extends IWorker, Data = unknown>
         break
     }
     await waitWorkerExit
+    this.worker.removeAllListeners()
   }
 
   private closeMessageChannel (): void {
index cd8969f67d0800c48a728920b9f2c3d4eab2cb55..54dee480ebd3e23adf56a7331a58e2d4749d498e 100644 (file)
@@ -32,6 +32,16 @@ export abstract class AbstractFixedQueue<T> implements IFixedQueue<T> {
 
   /** @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<T> implements IFixedQueue<T> {
       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 */
index cfcf5ca0c30ec72987ab5740fc97450491992305..7ae8e6d8e40ceb15a5c2d3dd015e486b3cf60275 100644 (file)
@@ -97,18 +97,8 @@ export class PriorityQueue<T> {
     let prev: PriorityQueueNode<T> | 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<T> {
     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<T> {
     }
     return fixedQueue
   }
+
+  private removePriorityQueueNode (
+    nodeToRemove: PriorityQueueNode<T>,
+    previousNode?: PriorityQueueNode<T>
+  ): 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
+  }
 }
index f3c36bceb9394991c04f2a776dfcf107d9da3900..49536b0b7bdf21600e63ed1387d665011bc3bac2 100644 (file)
@@ -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))
+      }
     }
   }
 
index 1f1eb8c31964d548a2830cc99e52170f6ca90bde..42d2ec30e6b68676cccb121612f5144cc95350cc 100644 (file)
@@ -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])
   })
 })
index d1231643f7496353def85b77ef80a9c130a3e5bd..faf10a16dd28238120ff0af396434edb24713b3e 100644 (file)
@@ -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])
   })
 })