Skip to content

Commit 167e99b

Browse files
apapirovskitargos
authored andcommitted
timers: fix priority queue removeAt fn
PR-URL: #23870 Fixes: #23860 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent e241398 commit 167e99b

File tree

2 files changed

+42
-1
lines changed

2 files changed

+42
-1
lines changed

lib/internal/priority_queue.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,13 @@ module.exports = class PriorityQueue {
8383
heap[pos] = heap[size + 1];
8484
heap[size + 1] = undefined;
8585

86-
if (size > 0)
86+
if (size > 0) {
87+
// If not removing the last item, update the shifted item's position.
88+
if (pos <= size && this[kSetPosition] !== undefined)
89+
this[kSetPosition](heap[pos], pos);
90+
8791
this.percolateDown(1);
92+
}
8893
}
8994

9095
remove(value) {

test/parallel/test-priority-queue.js

+36
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,39 @@ const PriorityQueue = require('internal/priority_queue');
9595

9696
assert.strictEqual(queue.peek(), undefined);
9797
}
98+
99+
{
100+
const queue = new PriorityQueue((a, b) => {
101+
return a.value - b.value;
102+
}, (node, pos) => (node.position = pos));
103+
104+
queue.insert({ value: 1, position: null });
105+
queue.insert({ value: 2, position: null });
106+
queue.insert({ value: 3, position: null });
107+
queue.insert({ value: 4, position: null });
108+
queue.insert({ value: 5, position: null });
109+
110+
queue.insert({ value: 2, position: null });
111+
const secondLargest = { value: 10, position: null };
112+
queue.insert(secondLargest);
113+
const largest = { value: 15, position: null };
114+
queue.insert(largest);
115+
116+
queue.removeAt(5);
117+
assert.strictEqual(largest.position, 5);
118+
119+
// check that removing 2nd to last item works fine
120+
queue.removeAt(6);
121+
assert.strictEqual(secondLargest.position, 6);
122+
123+
// check that removing the last item doesn't throw
124+
queue.removeAt(6);
125+
126+
assert.strictEqual(queue.shift().value, 1);
127+
assert.strictEqual(queue.shift().value, 2);
128+
assert.strictEqual(queue.shift().value, 2);
129+
assert.strictEqual(queue.shift().value, 4);
130+
assert.strictEqual(queue.shift().value, 15);
131+
132+
assert.strictEqual(queue.shift(), undefined);
133+
}

0 commit comments

Comments
 (0)