From 167e99b9a16645e2cf2ad9b2495e47211064a551 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 25 Oct 2018 04:02:23 -0700 Subject: [PATCH] timers: fix priority queue removeAt fn PR-URL: https://github.com/nodejs/node/pull/23870 Fixes: https://github.com/nodejs/node/issues/23860 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Matteo Collina Reviewed-By: Gus Caplan Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: James M Snell --- lib/internal/priority_queue.js | 7 +++++- test/parallel/test-priority-queue.js | 36 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/internal/priority_queue.js b/lib/internal/priority_queue.js index cb046507a667d9..2f1db934b43cc8 100644 --- a/lib/internal/priority_queue.js +++ b/lib/internal/priority_queue.js @@ -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) { diff --git a/test/parallel/test-priority-queue.js b/test/parallel/test-priority-queue.js index 5b8f53a1766bb5..702b5528ba9611 100644 --- a/test/parallel/test-priority-queue.js +++ b/test/parallel/test-priority-queue.js @@ -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); +}