KObjectHashMap PrimitiveIterator NPE

Motivation:
KObjectHashMap.remove(int index) attempts to move back items which may have been displaced because their spot in the hash based array was taken by another item. If this happens the nextIndex reference in PrimitiveIterator will not be updated. At this time the PrimitiveEntry will reference the incorrect index and may result in a NPE.

Modifications:
- If KObjectHashMap.remove(int index) moves entries back then PrimitiveIterator should adjust its nextIndex

Result:
PrimitiveIterator.remove() updates its internal state to reference the new nextIndex and will not NPE.
Fixes https://github.com/netty/netty/issues/5198

common/src/main/templates/io/netty/util/collection/KObjectHashMap.template

@@ -403,8 +403,9 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
      * if necessary to not break conflict chains.
      *
      * @param index the index position of the element to remove.
+     * @return {@code true} if the next item was moved back. {@code false} otherwise.
      */
-    private void removeAt(int index) {
+    private boolean removeAt(final int index) {
         --size;
         // Clearing the key is not strictly necessary (for GC like in a regular collection),
         // but recommended for security. The memory location is still fresh in the cache anyway.
@@ -416,6 +417,7 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
         // entries and move them back if possible, optimizing future lookups.
         // Knuth Section 6.4 Algorithm R, also used by the JDK's IdentityHashMap.
 
+        boolean movedBack = false;
         int nextFree = index;
         for (int i = probeNext(index); values[i] != null; i = probeNext(i)) {
             int bucket = hashIndex(keys[i]);
@@ -424,12 +426,14 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
                 // Move the displaced entry "back" to the first available position.
                 keys[nextFree] = keys[i];
                 values[nextFree] = values[i];
+                movedBack = true;
                 // Put the first entry after the displaced entry
                 keys[i] = 0;
                 values[i] = null;
                 nextFree = i;
             }
         }
+        return movedBack;
     }
 
     /**
@@ -629,7 +633,12 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
             if (prevIndex < 0) {
                 throw new IllegalStateException("next must be called before each remove.");
             }
-            removeAt(prevIndex);
+            if (removeAt(prevIndex)) {
+                // removeAt may move elements "back" in the array if they have been displaced because their spot in the
+                // array was occupied when they were inserted. If this occurs then the nextIndex is now invalid and
+                // should instead point to the prevIndex which now holds an element which was "moved back".
+                nextIndex = prevIndex;
+            }
             prevIndex = -1;
         }
 

common/src/test/templates/io/netty/util/collection/KObjectHashMapTest.template

@@ -14,6 +14,7 @@
  */
 package io.netty.util.collection;
 
+import io.netty.util.collection.@K@ObjectMap.PrimitiveEntry;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -77,6 +78,24 @@ public class @K@ObjectHashMapTest {
         map = new @K@ObjectHashMap<Value>();
     }
 
+    @Test
+    public void iteartorRemoveShouldNotNPE() {
+        map = new @K@ObjectHashMap<Value>(4, 1);
+        map.put((@O@)(@k@) 0, new Value("A"));
+        map.put((@O@)(@k@) 1, new Value("B"));
+        map.put((@O@)(@k@) 4, new Value("C"));
+        map.remove((@O@)(@k@) 1);
+        Iterator<PrimitiveEntry<Value>> itr = map.entries().iterator();
+        while (itr.hasNext()) {
+            PrimitiveEntry<Value> entry = itr.next();
+            assertNotNull(entry.key());
+            assertNotNull(entry.value());
+            itr.remove();
+        }
+        assertTrue(map.isEmpty());
+        assertEquals(0, map.size());
+    }
+
     @Test
     public void putNewMappingShouldSucceed() {
         Value v = new Value("v");