Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lib: ensure no holey array in fixed_queue #54537

Merged
merged 2 commits into from
Sep 2, 2024
Merged

lib: ensure no holey array in fixed_queue #54537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f3a09d
lib: ensure no holey array in fixed_queue
jazelly Aug 25, 2024
348536f
add a test
jazelly Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions lib/internal/fixed_queue.js
View file
Open in desktop
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
View file
Open in desktop
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);
}
}
Loading