lib: ensure no holey array in fixed_queue

Co-authored-by: Jake Yuesong Li <[email protected]> PR-URL: #54537 Fixes: #54472 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
nodejs · Sep 2, 2024 · 981c701 · 981c701
1 parent 3d954dc
commit 981c701
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 14 deletions.
diff --git a/lib/internal/fixed_queue.js b/lib/internal/fixed_queue.js
@@ -2,6 +2,7 @@
 
 const {
   Array,
+  ArrayPrototypeFill,
 } = primordials;
 
 // Currently optimal queue size, tested on V8 6.0 - 6.6. Must be power of two.
@@ -17,18 +18,18 @@ const kMask = kSize - 1;
 // +-----------+ <-----\       +-----------+ <------\         +-----------+
 // |  [null]   |        \----- |   next    |         \------- |   next    |
 // +-----------+               +-----------+                  +-----------+
-// |   item    | <-- bottom    |   item    | <-- bottom       |  [empty]  |
-// |   item    |               |   item    |                  |  [empty]  |
-// |   item    |               |   item    |                  |  [empty]  |
-// |   item    |               |   item    |                  |  [empty]  |
+// |   item    | <-- bottom    |   item    | <-- bottom       | undefined |
+// |   item    |               |   item    |                  | undefined |
+// |   item    |               |   item    |                  | undefined |
+// |   item    |               |   item    |                  | undefined |
 // |   item    |               |   item    |       bottom --> |   item    |
 // |   item    |               |   item    |                  |   item    |
 // |    ...    |               |    ...    |                  |    ...    |
 // |   item    |               |   item    |                  |   item    |
 // |   item    |               |   item    |                  |   item    |
-// |  [empty]  | <-- top       |   item    |                  |   item    |
-// |  [empty]  |               |   item    |                  |   item    |
-// |  [empty]  |               |  [empty]  | <-- top  top --> |  [empty]  |
+// | undefined | <-- top       |   item    |                  |   item    |
+// | undefined |               |   item    |                  |   item    |
+// | undefined |               | undefined | <-- top  top --> | undefined |
 // +-----------+               +-----------+                  +-----------+
 //
 // Or, if there is only one circular buffer, it looks something
@@ -40,12 +41,12 @@ const kMask = kSize - 1;
 // +-----------+                               +-----------+
 // |  [null]   |                               |  [null]   |
 // +-----------+                               +-----------+
-// |  [empty]  |                               |   item    |
-// |  [empty]  |                               |   item    |
-// |   item    | <-- bottom            top --> |  [empty]  |
-// |   item    |                               |  [empty]  |
-// |  [empty]  | <-- top            bottom --> |   item    |
-// |  [empty]  |                               |   item    |
+// | undefined |                               |   item    |
+// | undefined |                               |   item    |
+// |   item    | <-- bottom            top --> | undefined |
+// |   item    |                               | undefined |
+// | undefined | <-- top            bottom --> |   item    |
+// | undefined |                               |   item    |
 // +-----------+                               +-----------+
 //
 // Adding a value means moving `top` forward by one, removing means
@@ -60,7 +61,7 @@ class FixedCircularBuffer {
   constructor() {
     this.bottom = 0;
     this.top = 0;
-    this.list = new Array(kSize);
+    this.list = ArrayPrototypeFill(new Array(kSize), undefined);
     this.next = null;
   }
 

diff --git a/test/parallel/test-fixed-queue.js b/test/parallel/test-fixed-queue.js
@@ -32,3 +32,15 @@ const FixedQueue = require('internal/fixed_queue');
   assert.strictEqual(queue.shift(), 'a');
   assert(queue.isEmpty());
 }
+
+{
+  // FixedQueue must not be holey array
+  // Refs: https://github.com/nodejs/node/issues/54472
+  const queue = new FixedQueue();
+  for (let i = 0; i < queue.head.list.length; i++) {
+    assert(i in queue.head.list);
+  }
+  for (let i = 0; i < queue.tail.list.length; i++) {
+    assert(i in queue.tail.list);
+  }
+}