Skip to content

Commit

Permalink
lib: ensure no holey array in fixed_queue
Browse files Browse the repository at this point in the history
Co-authored-by: Jake Yuesong Li <jake.yuesong@gmail.com>
PR-URL: #54537
Fixes: #54472
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
  • Loading branch information
2 people authored and targos committed Oct 2, 2024
1 parent fe3155c commit b8c06dc
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
29 changes: 15 additions & 14 deletions lib/internal/fixed_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const {
Array,
ArrayPrototypeFill,
} = primordials;

// Currently optimal queue size, tested on V8 6.0 - 6.6. Must be power of two.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
}

Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-fixed-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit b8c06dc

Please sign in to comment.