Skip to content

Commit

Permalink
timers: fix priority queue removeAt fn
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
apapirovski authored and targos committed Nov 1, 2018
1 parent e241398 commit 167e99b
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/internal/priority_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,13 @@ module.exports = class PriorityQueue {
heap[pos] = heap[size + 1];
heap[size + 1] = undefined;

if (size > 0)
if (size > 0) {
// If not removing the last item, update the shifted item's position.
if (pos <= size && this[kSetPosition] !== undefined)
this[kSetPosition](heap[pos], pos);

this.percolateDown(1);
}
}

remove(value) {
Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-priority-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,39 @@ const PriorityQueue = require('internal/priority_queue');

assert.strictEqual(queue.peek(), undefined);
}

{
const queue = new PriorityQueue((a, b) => {
return a.value - b.value;
}, (node, pos) => (node.position = pos));

queue.insert({ value: 1, position: null });
queue.insert({ value: 2, position: null });
queue.insert({ value: 3, position: null });
queue.insert({ value: 4, position: null });
queue.insert({ value: 5, position: null });

queue.insert({ value: 2, position: null });
const secondLargest = { value: 10, position: null };
queue.insert(secondLargest);
const largest = { value: 15, position: null };
queue.insert(largest);

queue.removeAt(5);
assert.strictEqual(largest.position, 5);

// check that removing 2nd to last item works fine
queue.removeAt(6);
assert.strictEqual(secondLargest.position, 6);

// check that removing the last item doesn't throw
queue.removeAt(6);

assert.strictEqual(queue.shift().value, 1);
assert.strictEqual(queue.shift().value, 2);
assert.strictEqual(queue.shift().value, 2);
assert.strictEqual(queue.shift().value, 4);
assert.strictEqual(queue.shift().value, 15);

assert.strictEqual(queue.shift(), undefined);
}

0 comments on commit 167e99b

Please sign in to comment.