-
Notifications
You must be signed in to change notification settings - Fork 30.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
lib: expose FixedQueue internally and fix nextTick bug
A bug was introduced together with the FixedQueue implementation for process.nextTick which meant that the queue wouldn't necessarily fully clear on each run through. Fix it and abstract the data structure into an internal module that can later be used elsewhere.
- Loading branch information
1 parent
a957f24
commit 69dd197
Showing
4 changed files
with
139 additions
and
116 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| 'use strict'; | ||
|
|
||
| // Currently optimal queue size, tested on V8 6.0 - 6.6. Must be power of two. | ||
| const kSize = 2048; | ||
| const kMask = kSize - 1; | ||
|
|
||
| // The FixedQueue is implemented as a singly-linked list of fixed-size | ||
| // circular buffers. It looks something like this: | ||
| // | ||
| // head tail | ||
| // | | | ||
| // v v | ||
| // +-----------+ <-----\ +-----------+ <------\ +-----------+ | ||
| // | [null] | \----- | next | \------- | next | | ||
| // +-----------+ +-----------+ +-----------+ | ||
| // | item | <-- bottom | item | <-- bottom | [empty] | | ||
| // | item | | item | | [empty] | | ||
| // | item | | item | | [empty] | | ||
| // | item | | item | | [empty] | | ||
| // | item | | item | bottom --> | item | | ||
| // | item | | item | | item | | ||
| // | ... | | ... | | ... | | ||
| // | item | | item | | item | | ||
| // | item | | item | | item | | ||
| // | [empty] | <-- top | item | | item | | ||
| // | [empty] | | item | | item | | ||
| // | [empty] | | item | | item | | ||
| // +-----------+ +-----------+ <-- top top --> +-----------+ | ||
| // | ||
| // Or, if there is only one circular buffer, it looks something | ||
| // like either of these: | ||
| // | ||
| // head tail head tail | ||
| // | | | | | ||
| // v v v v | ||
| // +-----------+ +-----------+ | ||
| // | [null] | | [null] | | ||
| // +-----------+ +-----------+ | ||
| // | [empty] | | item | | ||
| // | [empty] | | item | | ||
| // | item | <-- bottom top --> | [empty] | | ||
| // | item | | [empty] | | ||
| // | [empty] | <-- top bottom --> | item | | ||
| // | [empty] | | item | | ||
| // +-----------+ +-----------+ | ||
| // | ||
| // Adding a value means moving `top` forward by one, removing means | ||
| // moving `bottom` forward by one. | ||
| // | ||
| // We let `bottom` and `top` wrap around, so when `top` is conceptually | ||
| // pointing to the end of the list, that means that the actual value is `0`. | ||
| // | ||
| // In particular, when `top === bottom`, this can mean *either* that the | ||
| // current queue is empty or that it is full. We can differentiate by | ||
| // checking whether an entry in the queue is empty (a.k.a. `=== undefined`). | ||
|
|
||
| const FixedCircularBuffer = class FixedCircularBuffer { | ||
| constructor() { | ||
| this.bottom = 0; | ||
| this.top = 0; | ||
| this.list = new Array(kSize); | ||
| this.next = null; | ||
| } | ||
|
|
||
| isEmpty() { | ||
| return this.top === this.bottom && this.list[this.top] === undefined; | ||
| } | ||
|
|
||
| push(data) { | ||
| this.list[this.top] = data; | ||
| this.top = (this.top + 1) & kMask; | ||
| } | ||
|
|
||
| shift() { | ||
| const nextItem = this.list[this.bottom]; | ||
| if (nextItem === undefined) | ||
| return null; | ||
| this.list[this.bottom] = undefined; | ||
| this.bottom = (this.bottom + 1) & kMask; | ||
| return nextItem; | ||
| } | ||
| }; | ||
|
|
||
| module.exports = class FixedQueue { | ||
| constructor() { | ||
| this.tail = this.head = new FixedCircularBuffer(); | ||
| } | ||
|
|
||
| isEmpty() { | ||
| return this.head.isEmpty(); | ||
| } | ||
|
|
||
| push(data) { | ||
| let { head } = this; | ||
| if (head.bottom === head.top && head.list[head.top] !== undefined) { | ||
| // Head is full: Creates a new queue, sets the old queue's `.next` to it, | ||
| // and sets it as the new main queue. | ||
| this.head = head = head.next = new FixedCircularBuffer(); | ||
| } | ||
| head.push(data); | ||
| } | ||
|
|
||
| shift() { | ||
| const { tail } = this; | ||
| const next = tail.shift(); | ||
| if (tail.top === tail.bottom && tail.next !== null) { | ||
| // If there is another queue, it forms the new tail. | ||
| this.tail = tail.next; | ||
| } | ||
| return next; | ||
| } | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
|
|
||
| // This tests a highly specific regression tied to the FixedQueue size, which | ||
| // was introduced in Node.js 9.7.0: https://github.com/nodejs/node/pull/18617 | ||
| // More specifically, a nextTick list could potentially end up not fully | ||
| // clearing in one run through if exactly 2048 ticks were added after | ||
| // microtasks were executed within the nextTick loop. | ||
|
|
||
| process.nextTick(() => { | ||
| Promise.resolve(1).then(() => { | ||
| for (let i = 0; i < 2047; i++) | ||
| process.nextTick(common.mustCall()); | ||
| const immediate = setImmediate(common.mustNotCall()); | ||
| process.nextTick(common.mustCall(() => clearImmediate(immediate))); | ||
| }); | ||
| }); |