From d660706588b2019b924dd80b6ff8f33d2971b926 Mon Sep 17 00:00:00 2001
From: Chris Vest <christianvest_hansen@apple.com>
Date: Wed, 2 Dec 2020 13:06:00 +0100
Subject: [PATCH] Fix a bug in LongPriorityQueue internal remove (#10832)

Motivation:
We rely on this functionality in PoolChunk, and a bug was caught by a non-deterministic test failure

Modification:
Went back to the Algorithms book, and reimplemented remove() the way it was meant to.

Result:
No test failures after 200.000 runs, so we have some confidence the code is correct now.
---
 .../io/netty/buffer/LongPriorityQueue.java    | 22 ++++++++++---------
 .../netty/buffer/LongPriorityQueueTest.java   | 10 ++++++++-
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/LongPriorityQueue.java b/buffer/src/main/java/io/netty/buffer/LongPriorityQueue.java
index 6d1da2ffdb..e830e0e730 100644
--- a/buffer/src/main/java/io/netty/buffer/LongPriorityQueue.java
+++ b/buffer/src/main/java/io/netty/buffer/LongPriorityQueue.java
@@ -36,24 +36,27 @@ final class LongPriorityQueue {
             array = Arrays.copyOf(array, 1 + (array.length - 1) * 2);
         }
         array[size] = handle;
-        lift();
+        lift(size);
     }
 
     public void remove(long value) {
         for (int i = 1; i <= size; i++) {
             if (array[i] == value) {
-                if (i == size) {
-                    array[i] = 0;
-                } else {
-                    array[i] = array[size];
-                    sink(i);
-                }
-                size--;
+                array[i] = array[size--];
+                lift(i);
+                sink(i);
                 return;
             }
         }
     }
 
+    public long peek() {
+        if (size == 0) {
+            return NO_VALUE;
+        }
+        return array[1];
+    }
+
     public long poll() {
         if (size == 0) {
             return NO_VALUE;
@@ -70,8 +73,7 @@ final class LongPriorityQueue {
         return size == 0;
     }
 
-    private void lift() {
-        int index = size;
+    private void lift(int index) {
         int parentIndex;
         while (index > 1 && subord(parentIndex = index >> 1, index)) {
             swap(index, parentIndex);
diff --git a/buffer/src/test/java/io/netty/buffer/LongPriorityQueueTest.java b/buffer/src/test/java/io/netty/buffer/LongPriorityQueueTest.java
index 38f364cfb8..529f41385d 100644
--- a/buffer/src/test/java/io/netty/buffer/LongPriorityQueueTest.java
+++ b/buffer/src/test/java/io/netty/buffer/LongPriorityQueueTest.java
@@ -95,7 +95,7 @@ class LongPriorityQueueTest {
     @Test
     public void internalRemoveMustPreserveOrder() {
         ThreadLocalRandom tlr = ThreadLocalRandom.current();
-        int initialValues = tlr.nextInt(5, 30);
+        int initialValues = tlr.nextInt(1, 30);
         ArrayList<Long> values = new ArrayList<Long>();
         LongPriorityQueue pq = new LongPriorityQueue();
         for (int i = 0; i < initialValues; i++) {
@@ -127,13 +127,21 @@ class LongPriorityQueueTest {
         pq.offer(10);
         pq.offer(6);
         pq.remove(10);
+        assertThat(pq.peek()).isEqualTo(5);
+        assertThat(pq.peek()).isEqualTo(5);
         assertThat(pq.poll()).isEqualTo(5);
+        assertThat(pq.peek()).isEqualTo(5);
         assertThat(pq.poll()).isEqualTo(5);
+        assertThat(pq.peek()).isEqualTo(6);
         assertThat(pq.poll()).isEqualTo(6);
+        assertThat(pq.peek()).isEqualTo(6);
+        assertThat(pq.peek()).isEqualTo(6);
         assertThat(pq.poll()).isEqualTo(6);
+        assertThat(pq.peek()).isEqualTo(10);
         assertThat(pq.poll()).isEqualTo(10);
         assertThat(pq.poll()).isEqualTo(10);
         assertTrue(pq.isEmpty());
         assertThat(pq.poll()).isEqualTo(LongPriorityQueue.NO_VALUE);
+        assertThat(pq.peek()).isEqualTo(LongPriorityQueue.NO_VALUE);
     }
 }