From 3086ee0480ed456f327a91fce16b224153784ed1 Mon Sep 17 00:00:00 2001
From: Larry Booker <lbooker42@gmail.com>
Date: Thu, 23 Jan 2025 09:01:10 -0800
Subject: [PATCH 1/6] Initial commit of bug-fix and tests.

---
 .../io/deephaven/hash/KeyedIntObjectHash.java |  8 ++-
 .../deephaven/hash/KeyedLongObjectHash.java   |  8 ++-
 .../io/deephaven/hash/KeyedObjectHash.java    |  7 +-
 .../hash/TestKeyedIntObjectHashMap.java       | 71 +++++++++++++++++++
 .../hash/TestKeyedLongObjectHashMap.java      | 71 +++++++++++++++++++
 .../hash/TestKeyedObjectHashMap.java          | 65 +++++++++++++++++
 6 files changed, 221 insertions(+), 9 deletions(-)

diff --git a/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java b/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
index 5dcd9d7..74e00c1 100644
--- a/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
@@ -12,6 +12,7 @@ of the License, or (at your option) any later version.
 package io.deephaven.hash;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 /**
  * This collection implements a hashed set of objects identified by a key; the characteristics of
@@ -252,7 +253,7 @@ public synchronized boolean replace(Integer key, V oldValue, V newValue) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + intKeyDef.getIntKey(newValue));
     }
-    return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue) != null;
+    return Objects.equals(internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue), oldValue);
   }
 
   public synchronized boolean replace(int key, V oldValue, V newValue) {
@@ -260,7 +261,7 @@ public synchronized boolean replace(int key, V oldValue, V newValue) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + intKeyDef.getIntKey(newValue));
     }
-    return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue) != null;
+    return Objects.equals(internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue), oldValue);
   }
 
   private static final int NORMAL = 0;
@@ -304,7 +305,8 @@ protected V internalPut(V value, int mode, V oldValue) {
           }
           return null;
         } else if (candidate != DELETED && intKeyDef.equalIntKey(key, candidate)) {
-          if (mode != KeyedIntObjectHash.IF_ABSENT) {
+          if (mode != KeyedIntObjectHash.IF_ABSENT
+              && (oldValue == null || candidate.equals(oldValue))) {
             state[index] = value;
             _indexableList = null;
           }
diff --git a/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java b/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
index 534d343..d325c2e 100644
--- a/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
@@ -12,6 +12,7 @@ of the License, or (at your option) any later version.
 package io.deephaven.hash;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 /**
  * This collection implements a hashed set of objects identified by a key; the characteristics of
@@ -253,7 +254,7 @@ public synchronized boolean replace(Long key, V oldValue, V newValue) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + longKeyDef.getLongKey(newValue));
     }
-    return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue) != null;
+    return Objects.equals(internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue), oldValue);
   }
 
   public synchronized boolean replace(long key, V oldValue, V newValue) {
@@ -261,7 +262,7 @@ public synchronized boolean replace(long key, V oldValue, V newValue) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + longKeyDef.getLongKey(newValue));
     }
-    return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue) != null;
+    return Objects.equals(internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue), oldValue);
   }
 
   private static final int NORMAL = 0;
@@ -305,7 +306,8 @@ protected V internalPut(V value, int mode, V oldValue) {
           }
           return null;
         } else if (candidate != DELETED && longKeyDef.equalLongKey(key, candidate)) {
-          if (mode != KeyedLongObjectHash.IF_ABSENT) {
+          if (mode != KeyedLongObjectHash.IF_ABSENT
+              && (oldValue == null || candidate.equals(oldValue))) {
             state[index] = value;
             _indexableList = null;
           }
diff --git a/src/main/java/io/deephaven/hash/KeyedObjectHash.java b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
index a81ecc7..b9297f8 100644
--- a/src/main/java/io/deephaven/hash/KeyedObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
@@ -22,6 +22,7 @@ of the License, or (at your option) any later version.
 import java.util.Iterator;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Predicate;
 
@@ -170,7 +171,7 @@ public KeyedObjectHash(int initialCapacity, float loadFactor, KeyedObjectKey<K,
    */
   protected int setUp(int initialCapacity) {
     int capacity = super.setUp(initialCapacity);
-    storage = (V[]) new Object[capacity];
+      storage = (V[]) new Object[capacity];
     return capacity;
   }
 
@@ -704,7 +705,7 @@ public synchronized boolean replace(K key, V oldValue, V newValue) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + keyDef.getKey(newValue));
     }
-    return internalPut(newValue, REPLACE, oldValue) != null;
+    return Objects.equals(internalPut(newValue, REPLACE, oldValue), oldValue);
   }
 
   public synchronized boolean add(V value) {
@@ -761,7 +762,7 @@ protected V internalPut(V value, int mode, V oldValue) {
           }
           return null;
         } else if (candidate != DELETED && keyDef.equalKey(key, candidate)) {
-          if (mode != IF_ABSENT) {
+          if (mode != IF_ABSENT && (oldValue == null || candidate.equals(oldValue))) {
             state[index] = value;
             _indexableList = null;
           }
diff --git a/src/test/java/io/deephaven/hash/TestKeyedIntObjectHashMap.java b/src/test/java/io/deephaven/hash/TestKeyedIntObjectHashMap.java
index 2fb4b1e..6272f88 100644
--- a/src/test/java/io/deephaven/hash/TestKeyedIntObjectHashMap.java
+++ b/src/test/java/io/deephaven/hash/TestKeyedIntObjectHashMap.java
@@ -118,6 +118,77 @@ public void testSimpleUnboxedPutIfAbsent() {
     assertTrue(m.get(o2.getId()) == o1);
   }
 
+  /*
+   ** tests the unboxed replace call
+   */
+  public void testSimpleUnboxedReplace() {
+    KeyedIntTestObject result;
+
+    final KeyedIntTestObjectMap m = new KeyedIntTestObjectMap(10);
+
+    final KeyedIntTestObject o1 = new KeyedIntTestObject(0);
+    final KeyedIntTestObject o2 = new KeyedIntTestObject(0);
+    final KeyedIntTestObject o3 = new KeyedIntTestObject(0);
+
+    result = m.putIfAbsent(0, o1);
+    assertNull(result);
+    assertSame(m.get(0), o1); // strict equality test
+
+    result = m.putIfAbsent(0, o2);
+    assertNotNull(result);
+    assertSame(m.get(0), o1); // strict equality test
+
+    result = m.put(0, o2);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+    assertSame(m.get(0), o2); // strict equality test
+
+    assertFalse(m.replace(0, new KeyedIntTestObject(10), o3));
+    assertSame(m.get(0), o2); // strict equality test
+
+    assertTrue(m.replace(0, new KeyedIntTestObject(0), o3));
+    assertSame(m.get(0), o3); // strict equality test
+  }
+
+  /*
+   * Reproducer for bug documented in DH-18265
+   */
+  public void testDH18265() {
+    KeyedIntTestObject result;
+
+    final KeyedIntTestObjectMap m = new KeyedIntTestObjectMap(10);
+
+    // Setup the conditions for the bug to be triggered.
+    final int capacity = m.capacity();
+
+    // This will hash to 0 internally
+    final KeyedIntTestObject o1 = new KeyedIntTestObject(capacity);
+    result = m.putIfAbsent(capacity, o1);
+    assertNull(result);
+    assertSame(m.get(capacity), o1); // strict equality test
+
+    // This will also initially hash to 0, but will be double hashed to an empty slot.
+    final KeyedIntTestObject o2 = new KeyedIntTestObject(0);
+    result = m.putIfAbsent(0, o2);
+    assertNull(result);
+    assertSame(m.get(0), o2); // strict equality test
+
+    // Remove the first object, leaving a DELETED tombstone at the 0 slot.
+    result = m.remove(capacity);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+
+    // This replace should fail, since we do not match old values.
+    final KeyedIntTestObject o3 = new KeyedIntTestObject(10);
+    final KeyedIntTestObject o4 = new KeyedIntTestObject(0);
+    assertFalse(m.replace(0, o3, o4));
+    assertSame(m.get(0), o2); // strict equality test
+
+    // This replace should succeed, since we match the old value.
+    assertTrue(m.replace(0, o2, o4));
+    assertSame(m.get(0), o4); // strict equality test
+  }
+
   /*
    ** tests for KeyedIntObjectMaps -- putIfAbsent(K, ValueFactory)
    */
diff --git a/src/test/java/io/deephaven/hash/TestKeyedLongObjectHashMap.java b/src/test/java/io/deephaven/hash/TestKeyedLongObjectHashMap.java
index 1b753ad..70ca19a 100644
--- a/src/test/java/io/deephaven/hash/TestKeyedLongObjectHashMap.java
+++ b/src/test/java/io/deephaven/hash/TestKeyedLongObjectHashMap.java
@@ -121,6 +121,77 @@ public void testSimpleUnboxedPutIfAbsent() {
     assertTrue(m.get(o2.getId()) == o1);
   }
 
+  /*
+   ** tests the unboxed replace call
+   */
+  public void testSimpleUnboxedReplace() {
+    KeyedLongTestObject result;
+
+    final KeyedLongTestObjectMap m = new KeyedLongTestObjectMap(10);
+
+    final KeyedLongTestObject o1 = new KeyedLongTestObject(0);
+    final KeyedLongTestObject o2 = new KeyedLongTestObject(0);
+    final KeyedLongTestObject o3 = new KeyedLongTestObject(0);
+
+    result = m.putIfAbsent(0, o1);
+    assertNull(result);
+    assertSame(m.get(0), o1); // strict equality test
+
+    result = m.putIfAbsent(0, o2);
+    assertNotNull(result);
+    assertSame(m.get(0), o1); // strict equality test
+
+    result = m.put(0, o2);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+    assertSame(m.get(0), o2); // strict equality test
+
+    assertFalse(m.replace(0, new KeyedLongTestObject(10), o3));
+    assertSame(m.get(0), o2); // strict equality test
+
+    assertTrue(m.replace(0, new KeyedLongTestObject(0), o3));
+    assertSame(m.get(0), o3); // strict equality test
+  }
+
+  /*
+   * Reproducer for bug documented in DH-18265
+   */
+  public void testDH18265() {
+    KeyedLongTestObject result;
+
+    final KeyedLongTestObjectMap m = new KeyedLongTestObjectMap(10);
+
+    // Setup the conditions for the bug to be triggered.
+    final int capacity = m.capacity();
+
+    // This will hash to 0 internally
+    final KeyedLongTestObject o1 = new KeyedLongTestObject(capacity);
+    result = m.putIfAbsent(capacity, o1);
+    assertNull(result);
+    assertSame(m.get(capacity), o1); // strict equality test
+
+    // This will also initially hash to 0, but will be double hashed to an empty slot.
+    final KeyedLongTestObject o2 = new KeyedLongTestObject(0);
+    result = m.putIfAbsent(0, o2);
+    assertNull(result);
+    assertSame(m.get(0), o2); // strict equality test
+
+    // Remove the first object, leaving a DELETED tombstone at the 0 slot.
+    result = m.remove(capacity);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+
+    // This replace should fail, since we do not match old values.
+    final KeyedLongTestObject o3 = new KeyedLongTestObject(10);
+    final KeyedLongTestObject o4 = new KeyedLongTestObject(0);
+    assertFalse(m.replace(0, o3, o4));
+    assertSame(m.get(0), o2); // strict equality test
+
+    // This replace should succeed, since we match the old value.
+    assertTrue(m.replace(0, o2, o4));
+    assertSame(m.get(0), o4); // strict equality test
+  }
+
   /*
    ** tests for KeyedLongObjectMaps -- putIfAbsent(K, ValueFactory)
    */
diff --git a/src/test/java/io/deephaven/hash/TestKeyedObjectHashMap.java b/src/test/java/io/deephaven/hash/TestKeyedObjectHashMap.java
index 6fce52d..b1d573f 100644
--- a/src/test/java/io/deephaven/hash/TestKeyedObjectHashMap.java
+++ b/src/test/java/io/deephaven/hash/TestKeyedObjectHashMap.java
@@ -274,4 +274,69 @@ public void run() {
       fail("Unhandled exception: " + x);
     }
   }
+
+  public void testSimpleKOMReplace() {
+    KeyedTestObject result;
+    final KeyedTestObjectMap m = new KeyedTestObjectMap(10);
+
+    final KeyedTestObject o1 = new KeyedTestObject("0");
+    final KeyedTestObject o2 = new KeyedTestObject("0");
+    final KeyedTestObject o3 = new KeyedTestObject("0");
+
+    result = m.putIfAbsent("0", o1);
+    assertNull(result);
+    assertSame(m.get("0"), o1); // strict equality test
+
+    result = m.putIfAbsent("0", o2);
+    assertNotNull(result);
+    assertSame(m.get("0"), o1); // strict equality test
+
+    result = m.put("0", o2);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+    assertSame(m.get("0"), o2); // strict equality test
+
+    assertFalse(m.replace("0", new KeyedTestObject("10"), o3));
+    assertSame(m.get("0"), o2); // strict equality test
+
+    assertTrue(m.replace("0", new KeyedTestObject("0"), o3));
+    assertSame(m.get("0"), o3); // strict equality test
+  }
+
+  /*
+   * Reproducer for bug documented in DH-18265
+   */
+  public void testDH18265() {
+    KeyedTestObject result;
+
+    final KeyedTestObjectMap m = new KeyedTestObjectMap(10);
+
+    // This test relies on the fact that `FB` and `Ea` hash to the same value.
+
+    final KeyedTestObject o1 = new KeyedTestObject("FB");
+    result = m.putIfAbsent("FB", o1);
+    assertNull(result);
+    assertSame(m.get("FB"), o1); // strict equality test
+
+    // This hash collides with `FB`, but will be double hashed to an empty slot.
+    final KeyedTestObject o2 = new KeyedTestObject("Ea");
+    result = m.putIfAbsent("Ea", o2);
+    assertNull(result);
+    assertSame(m.get("Ea"), o2); // strict equality test
+
+    // Remove the first object, leaving a DELETED tombstone at the first slot.
+    result = m.remove("FB");
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+
+    // This replace should fail, since we do not match old values.
+    final KeyedTestObject o3 = new KeyedTestObject("AB");
+    final KeyedTestObject o4 = new KeyedTestObject("Ea");
+    assertFalse(m.replace("Ea", o3, o4));
+    assertSame(m.get("Ea"), o2); // strict equality test
+
+    // This replace should succeed, since we match the old value.
+    assertTrue(m.replace("Ea", o2, o4));
+    assertSame(m.get("Ea"), o4); // strict equality test
+  }
 }

From 5f8ffd7bacf8247cbe6ddad6ad6df99514de5ce2 Mon Sep 17 00:00:00 2001
From: Larry Booker <lbooker42@gmail.com>
Date: Fri, 24 Jan 2025 08:25:13 -0800
Subject: [PATCH 2/6] Correct accidental edit.

---
 src/main/java/io/deephaven/hash/KeyedObjectHash.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/io/deephaven/hash/KeyedObjectHash.java b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
index b9297f8..d01f8c7 100644
--- a/src/main/java/io/deephaven/hash/KeyedObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
@@ -171,7 +171,7 @@ public KeyedObjectHash(int initialCapacity, float loadFactor, KeyedObjectKey<K,
    */
   protected int setUp(int initialCapacity) {
     int capacity = super.setUp(initialCapacity);
-      storage = (V[]) new Object[capacity];
+    storage = (V[]) new Object[capacity];
     return capacity;
   }
 

From daa0b1cf78f7fec0259dc22b3b7c40604a3fab14 Mon Sep 17 00:00:00 2001
From: Larry Booker <lbooker42@gmail.com>
Date: Fri, 24 Jan 2025 08:47:04 -0800
Subject: [PATCH 3/6] Add exception for replace() with oldValue==null

---
 .../java/io/deephaven/hash/KeyedDoubleObjectHash.java  | 10 ++++++++--
 .../java/io/deephaven/hash/KeyedIntObjectHash.java     | 10 ++++++++--
 .../java/io/deephaven/hash/KeyedLongObjectHash.java    | 10 ++++++++--
 src/main/java/io/deephaven/hash/KeyedObjectHash.java   |  5 ++++-
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java b/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java
index 9c827c8..047bd0b 100644
--- a/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java
@@ -251,19 +251,25 @@ public synchronized V replace(double key, V value) {
   }
 
   public synchronized boolean replace(Double key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!doubleKeyDef.equalDoubleKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + doubleKeyDef.getDoubleKey(newValue));
     }
-    return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue) != null;
+    return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue).equals(oldValue);
   }
 
   public synchronized boolean replace(double key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!doubleKeyDef.equalDoubleKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + doubleKeyDef.getDoubleKey(newValue));
     }
-    return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue) != null;
+    return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue).equals(oldValue);
   }
 
   private static final int NORMAL = 0;
diff --git a/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java b/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
index 74e00c1..118c545 100644
--- a/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
@@ -249,19 +249,25 @@ public synchronized V replace(int key, V value) {
   }
 
   public synchronized boolean replace(Integer key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!intKeyDef.equalIntKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + intKeyDef.getIntKey(newValue));
     }
-    return Objects.equals(internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue), oldValue);
+    return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue).equals(oldValue);
   }
 
   public synchronized boolean replace(int key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!intKeyDef.equalIntKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + intKeyDef.getIntKey(newValue));
     }
-    return Objects.equals(internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue), oldValue);
+    return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue).equals(oldValue);
   }
 
   private static final int NORMAL = 0;
diff --git a/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java b/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
index d325c2e..66a89fb 100644
--- a/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
@@ -250,19 +250,25 @@ public synchronized V replace(long key, V value) {
   }
 
   public synchronized boolean replace(Long key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!longKeyDef.equalLongKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + longKeyDef.getLongKey(newValue));
     }
-    return Objects.equals(internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue), oldValue);
+    return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue).equals(oldValue);
   }
 
   public synchronized boolean replace(long key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!longKeyDef.equalLongKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + longKeyDef.getLongKey(newValue));
     }
-    return Objects.equals(internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue), oldValue);
+    return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue).equals(oldValue);
   }
 
   private static final int NORMAL = 0;
diff --git a/src/main/java/io/deephaven/hash/KeyedObjectHash.java b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
index d01f8c7..6ce2424 100644
--- a/src/main/java/io/deephaven/hash/KeyedObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
@@ -701,11 +701,14 @@ public synchronized V replace(K key, V value) {
   }
 
   public synchronized boolean replace(K key, V oldValue, V newValue) {
+    if (oldValue == null) {
+      throw new NullPointerException("oldValue is null, but this map cannot hold null values");
+    }
     if (!keyDef.equalKey(key, newValue)) {
       throw new IllegalArgumentException(
           "key and value are inconsistent:" + key + " and " + keyDef.getKey(newValue));
     }
-    return Objects.equals(internalPut(newValue, REPLACE, oldValue), oldValue);
+    return internalPut(newValue, REPLACE, oldValue).equals(oldValue);
   }
 
   public synchronized boolean add(V value) {

From 517c4da0ae7a14cd632fa1314856922a67b8def6 Mon Sep 17 00:00:00 2001
From: Larry Booker <lbooker42@gmail.com>
Date: Fri, 24 Jan 2025 08:55:09 -0800
Subject: [PATCH 4/6] Spotless applied

---
 src/main/java/io/deephaven/hash/KeyedIntObjectHash.java  | 1 -
 src/main/java/io/deephaven/hash/KeyedLongObjectHash.java | 1 -
 src/main/java/io/deephaven/hash/KeyedObjectHash.java     | 1 -
 3 files changed, 3 deletions(-)

diff --git a/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java b/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
index 118c545..4dc4f78 100644
--- a/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedIntObjectHash.java
@@ -12,7 +12,6 @@ of the License, or (at your option) any later version.
 package io.deephaven.hash;
 
 import java.io.Serializable;
-import java.util.Objects;
 
 /**
  * This collection implements a hashed set of objects identified by a key; the characteristics of
diff --git a/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java b/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
index 66a89fb..708a30d 100644
--- a/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedLongObjectHash.java
@@ -12,7 +12,6 @@ of the License, or (at your option) any later version.
 package io.deephaven.hash;
 
 import java.io.Serializable;
-import java.util.Objects;
 
 /**
  * This collection implements a hashed set of objects identified by a key; the characteristics of
diff --git a/src/main/java/io/deephaven/hash/KeyedObjectHash.java b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
index 6ce2424..7c5b440 100644
--- a/src/main/java/io/deephaven/hash/KeyedObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedObjectHash.java
@@ -22,7 +22,6 @@ of the License, or (at your option) any later version.
 import java.util.Iterator;
 import java.util.Map;
 import java.util.NoSuchElementException;
-import java.util.Objects;
 import java.util.Set;
 import java.util.function.Predicate;
 

From 15188b8aa734dcd4a38b73fe709a5418ec41fc73 Mon Sep 17 00:00:00 2001
From: Larry Booker <lbooker42@gmail.com>
Date: Fri, 24 Jan 2025 14:06:27 -0800
Subject: [PATCH 5/6] Implemented correction to Double variant, added double to
 existing tests including the bug fix test cases.

---
 .../deephaven/hash/KeyedDoubleObjectHash.java |   3 +-
 .../deephaven/hash/KeyedDoubleTestObject.java |  93 +++++
 .../hash/KeyedDoubleTestObjectMap.java        |  31 ++
 .../hash/TestKeyedDoubleObjectHashMap.java    | 343 ++++++++++++++++++
 4 files changed, 469 insertions(+), 1 deletion(-)
 create mode 100644 src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java
 create mode 100644 src/test/java/io/deephaven/hash/KeyedDoubleTestObjectMap.java
 create mode 100644 src/test/java/io/deephaven/hash/TestKeyedDoubleObjectHashMap.java

diff --git a/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java b/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java
index 047bd0b..4cbf526 100644
--- a/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java
+++ b/src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java
@@ -313,7 +313,8 @@ protected V internalPut(V value, int mode, V oldValue) {
           }
           return null;
         } else if (candidate != DELETED && doubleKeyDef.equalDoubleKey(key, candidate)) {
-          if (mode != KeyedDoubleObjectHash.IF_ABSENT) {
+          if (mode != KeyedDoubleObjectHash.IF_ABSENT
+              && (oldValue == null || candidate.equals(oldValue))) {
             state[index] = value;
             _indexableList = null;
           }
diff --git a/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java b/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java
new file mode 100644
index 0000000..52b9bdb
--- /dev/null
+++ b/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java
@@ -0,0 +1,93 @@
+/*
+ Copyright (C) 2021 Deephaven Data Labs (https://deephaven.io).
+
+ This program is free software: you can redistribute it and/or modify it under the terms of the
+ GNU Lesser General Public License as published by the Free Software Foundation, either version 3
+ of the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+*/
+package io.deephaven.hash;
+
+/** Test class. */
+class KeyedDoubleTestObject {
+  private final double id;
+
+  public KeyedDoubleTestObject(double id) {
+    this.id = id;
+  }
+
+  public double getId() {
+    return id;
+  }
+
+  public boolean equals(Object other) {
+    return other instanceof KeyedDoubleTestObject && id == ((KeyedDoubleTestObject) other).id;
+  }
+
+  public int hashCode() {
+    return ~Double.hashCode(id); // do something different that gnu.trove.HashFunctions.hash(double)
+  }
+
+  public String toString() {
+    return "[KeyedDoubleTestObject:" + id + "]";
+  }
+
+  public static final KeyedDoubleObjectKey<KeyedDoubleTestObject> keyDef =
+      new KeyedDoubleObjectKey<>() {
+        public Double getKey(KeyedDoubleTestObject v) {
+          return v.getId();
+        }
+
+        public double getDoubleKey(KeyedDoubleTestObject v) {
+          return v.getId();
+        }
+
+        public int hashKey(Double k) {
+          return k.hashCode();
+        }
+
+        @Override
+        public boolean equalKey(Double k, KeyedDoubleTestObject v) {
+          return v.getId() == k;
+        }
+
+        public int hashDoubleKey(double k) {
+          return Double.hashCode(k);
+        }
+
+        public boolean equalDoubleKey(double k, KeyedDoubleTestObject v) {
+          return v.getId() == k;
+        }
+      };
+
+  public static final KeyedIntObjectHash.ValueFactory<KeyedDoubleTestObject> factory =
+      new KeyedIntObjectHash.ValueFactory<>() {
+        public KeyedDoubleTestObject newValue(Integer key) {
+          return new KeyedDoubleTestObject(key);
+        }
+
+        public KeyedDoubleTestObject newValue(int key) {
+          return new KeyedDoubleTestObject(key);
+        }
+      };
+
+  // for intrusive chained maps
+
+  private KeyedDoubleTestObject next;
+
+  public static final IntrusiveChainedHashAdapter<KeyedDoubleTestObject> adapter =
+      new IntrusiveChainedHashAdapter<>() {
+        @Override
+        public KeyedDoubleTestObject getNext(KeyedDoubleTestObject self) {
+          return self.next;
+        }
+
+        @Override
+        public void setNext(KeyedDoubleTestObject self, KeyedDoubleTestObject next) {
+          self.next = next;
+        }
+      };
+}
diff --git a/src/test/java/io/deephaven/hash/KeyedDoubleTestObjectMap.java b/src/test/java/io/deephaven/hash/KeyedDoubleTestObjectMap.java
new file mode 100644
index 0000000..8d59ad0
--- /dev/null
+++ b/src/test/java/io/deephaven/hash/KeyedDoubleTestObjectMap.java
@@ -0,0 +1,31 @@
+/*
+ Copyright (C) 2021 Deephaven Data Labs (https://deephaven.io).
+
+ This program is free software: you can redistribute it and/or modify it under the terms of the
+ GNU Lesser General Public License as published by the Free Software Foundation, either version 3
+ of the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+*/
+package io.deephaven.hash;
+
+/** Instantiation of KeyedDoubleObjectHashMap on the test class. */
+class KeyedDoubleTestObjectMap extends KeyedDoubleObjectHashMap<KeyedDoubleTestObject> {
+  public KeyedDoubleTestObjectMap() {
+    super(KeyedDoubleTestObject.keyDef);
+  }
+
+  public KeyedDoubleTestObjectMap(int initialCapacity) {
+    super(initialCapacity, KeyedDoubleTestObject.keyDef);
+  }
+
+  public KeyedDoubleTestObjectMap(int initialCapacity, float loadFactor) {
+    super(initialCapacity, loadFactor, KeyedDoubleTestObject.keyDef);
+  }
+
+  public final double getId(KeyedDoubleTestObject obj) {
+    return obj.getId();
+  }
+}
diff --git a/src/test/java/io/deephaven/hash/TestKeyedDoubleObjectHashMap.java b/src/test/java/io/deephaven/hash/TestKeyedDoubleObjectHashMap.java
new file mode 100644
index 0000000..bfb3df6
--- /dev/null
+++ b/src/test/java/io/deephaven/hash/TestKeyedDoubleObjectHashMap.java
@@ -0,0 +1,343 @@
+/*
+ Copyright (C) 2021 Deephaven Data Labs (https://deephaven.io).
+
+ This program is free software: you can redistribute it and/or modify it under the terms of the
+ GNU Lesser General Public License as published by the Free Software Foundation, either version 3
+ of the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+*/
+package io.deephaven.hash;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Random;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestKeyedDoubleObjectHashMap
+    extends AbstractTestGenericMap<Double, KeyedDoubleTestObject> {
+  private static Logger log = LoggerFactory.getLogger(TestKeyedDoubleObjectHashMap.class);
+
+  public TestKeyedDoubleObjectHashMap(String name) {
+    super(name, 100);
+  }
+
+  private static Random random = new Random(101763);
+
+  public HashMap<Double, KeyedDoubleTestObject> generateUniqueRandomHashMap(
+      int size, int min_key, int max_key) {
+    HashMap<Double, KeyedDoubleTestObject> m = new HashMap<>(size);
+    assert min_key < max_key;
+    assert max_key - min_key > size;
+    while (m.size() != size) {
+      double key = (double) random.nextInt(max_key - min_key) + min_key;
+      if (!m.containsKey(key)) {
+        m.put(key, new KeyedDoubleTestObject(key));
+      }
+    }
+    return m;
+  }
+
+  protected Map<Double, KeyedDoubleTestObject> newTestMap(
+      int initialSize, Map<Double, KeyedDoubleTestObject> from) {
+    KeyedDoubleTestObjectMap map = new KeyedDoubleTestObjectMap(initialSize);
+    if (from != null) {
+      for (KeyedDoubleTestObject o : from.values()) {
+        map.put(o.getId(), o);
+      }
+    }
+    return map;
+  }
+
+  protected void compact(Map map) {
+    assert map instanceof KeyedDoubleTestObjectMap;
+    ((KeyedDoubleTestObjectMap) map).compact();
+  }
+
+  protected KeyedDoubleTestObject[] newValueArray(int n) {
+    return new KeyedDoubleTestObject[n];
+  }
+
+  protected Double[] newKeyArray(int n) {
+    return new Double[n];
+  }
+
+  protected KeyedDoubleTestObject newValue(Double key) {
+    return new KeyedDoubleTestObject(key);
+  }
+
+  protected Double getKey(KeyedDoubleTestObject value) {
+    return value.getId();
+  }
+
+  /**
+   * If the test subject is an indexable map, make sure the getByIndex method returns identical
+   * objects
+   */
+  protected <K, V> void assertConsistency(Map<K, V> subject) {
+    assert subject instanceof KeyedDoubleTestObjectMap;
+    IndexableMap<Integer, KeyedDoubleTestObject> imap =
+        (IndexableMap<Integer, KeyedDoubleTestObject>) subject;
+    for (int i = 0; i < imap.size(); ++i) {
+      KeyedDoubleTestObject o = imap.getByIndex(i);
+      assertTrue("values are identical", o == subject.get(o.getId()));
+    }
+  }
+
+  @Override
+  public void setUp() throws Exception {
+    super.setUp();
+  }
+
+  @Override
+  public void tearDown() throws Exception {
+    super.tearDown();
+  }
+
+  /*
+   ** tests the unboxed putIfAbsent call
+   */
+  public void testSimpleUnboxedPutIfAbsent() {
+    final KeyedDoubleTestObjectMap m = new KeyedDoubleTestObjectMap();
+
+    // create two objects that are equals() but not identical
+    KeyedDoubleTestObject o1 = new KeyedDoubleTestObject(42);
+    KeyedDoubleTestObject o2 = new KeyedDoubleTestObject(42);
+
+    KeyedDoubleTestObject result;
+
+    result = m.putIfAbsent(o1.getId(), o1);
+    assertTrue(result == null);
+
+    result = m.putIfAbsent(o2.getId(), o2);
+    assertTrue(result == o1);
+
+    assertTrue(m.get(o2.getId()) == o1);
+  }
+
+  /*
+   ** tests the unboxed replace call
+   */
+  public void testSimpleUnboxedReplace() {
+    KeyedDoubleTestObject result;
+
+    final KeyedDoubleTestObjectMap m = new KeyedDoubleTestObjectMap(10);
+
+    final KeyedDoubleTestObject o1 = new KeyedDoubleTestObject(0);
+    final KeyedDoubleTestObject o2 = new KeyedDoubleTestObject(0);
+    final KeyedDoubleTestObject o3 = new KeyedDoubleTestObject(0);
+
+    result = m.putIfAbsent(0, o1);
+    assertNull(result);
+    assertSame(m.get(0), o1); // strict equality test
+
+    result = m.putIfAbsent(0, o2);
+    assertNotNull(result);
+    assertSame(m.get(0), o1); // strict equality test
+
+    result = m.put(0, o2);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+    assertSame(m.get(0), o2); // strict equality test
+
+    assertFalse(m.replace(0, new KeyedDoubleTestObject(10), o3));
+    assertSame(m.get(0), o2); // strict equality test
+
+    assertTrue(m.replace(0, new KeyedDoubleTestObject(0), o3));
+    assertSame(m.get(0), o3); // strict equality test
+  }
+
+  /*
+   * Reproducer for bug documented in DH-18265
+   */
+  public void testDH18265() {
+    KeyedDoubleTestObject result;
+
+    final KeyedDoubleTestObjectMap m = new KeyedDoubleTestObjectMap(2);
+
+    // Setup the conditions for the bug to be triggered. Not all powers of two work, but this does.
+    final double initial = 2 << 16;
+    final double collision = initial + m.capacity();
+
+    final KeyedDoubleTestObject o1 = new KeyedDoubleTestObject(initial);
+    result = m.putIfAbsent(initial, o1);
+    assertNull(result);
+    assertSame(m.get(initial), o1); // strict equality test
+
+    // This will also initially collide, but will be double hashed to an empty slot.
+    final KeyedDoubleTestObject o2 = new KeyedDoubleTestObject(collision);
+    result = m.putIfAbsent(collision, o2);
+    assertNull(result);
+    assertSame(m.get(collision), o2); // strict equality test
+
+    // Remove the first object, leaving a DELETED tombstone at the slot.
+    result = m.remove(initial);
+    assertNotNull(result);
+    assertSame(result, o1); // strict equality test
+
+    // This replace should fail, since we do not match old values.
+    final KeyedDoubleTestObject o3 = new KeyedDoubleTestObject(10);
+    final KeyedDoubleTestObject o4 = new KeyedDoubleTestObject(collision);
+    assertFalse(m.replace(collision, o3, o4));
+    assertSame(m.get(collision), o2); // strict equality test
+
+    // This replace should succeed, since we match the old value.
+    assertTrue(m.replace(collision, o2, o4));
+    assertSame(m.get(collision), o4); // strict equality test
+  }
+
+  /*
+   ** tests for KeyedIntObjectMaps -- putIfAbsent(K, ValueFactory)
+   */
+  public class KIOMPutIfAbsent<V> extends Thread {
+    public final int numRuns;
+    public final HashMap<Double, V> objects;
+    public final KeyedDoubleObjectHash<V> map;
+    public final KeyedDoubleObjectHash.ValueFactory<V> factory;
+
+    public KIOMPutIfAbsent(
+        int numRuns,
+        HashMap<Double, V> objects,
+        KeyedDoubleObjectHash<V> map,
+        KeyedDoubleObjectHash.ValueFactory<V> factory) {
+      this.numRuns = numRuns;
+      this.objects = objects;
+      this.map = map;
+      this.factory = factory;
+    }
+
+    public int numRemoves = 0;
+
+    public void run() {
+      for (int i = 0; i < numRuns; ++i) {
+        for (Double k : objects.keySet()) {
+          map.putIfAbsent(k.intValue(), factory); // make sure we call the right method!
+        }
+      }
+      for (Double k : objects.keySet()) {
+        if (random.nextDouble() < 0.4) {
+          if (map.removeKey(k.intValue()) != null) { // make sure we call the right method!
+            ++numRemoves;
+          }
+        }
+        if (random.nextDouble() < 0.01) {
+          compact((Map<Integer, V>) map);
+        }
+      }
+      for (Double k : objects.keySet()) {
+        map.putIfAbsent(k.intValue(), factory); // make sure we call the right method!
+      }
+    }
+  }
+
+  private static class KIOMPutIfAbsentFactory<V> implements KeyedDoubleObjectHash.ValueFactory<V> {
+    public final HashMap<Double, V> objects;
+
+    public KIOMPutIfAbsentFactory(HashMap<Double, V> objects) {
+      this.objects = objects;
+    }
+
+    public int numCalls = 0;
+
+    public V newValue(Double key) {
+      ++numCalls;
+      return objects.get(key);
+    }
+
+    public V newValue(double key) {
+      ++numCalls;
+      return objects.get(key);
+    }
+  }
+
+  public void testKIOMPutIfAbsent() {
+    final Map<Double, KeyedDoubleTestObject> map = newTestMap(10, null);
+    if (!(map instanceof KeyedDoubleObjectHash)) {
+      return;
+    }
+    KeyedDoubleObjectHash<KeyedDoubleTestObject> KIOM =
+        ((KeyedDoubleObjectHash<KeyedDoubleTestObject>) map);
+
+    final HashMap<Double, KeyedDoubleTestObject> objects =
+        generateUniqueRandomHashMap(SIZE * 10, MIN_KEY, MAX_KEY);
+    final KIOMPutIfAbsentFactory<KeyedDoubleTestObject> factory =
+        new KIOMPutIfAbsentFactory<>(objects);
+    final int NUM_THREADS = 5;
+    final int NUM_RUNS = 100;
+
+    ArrayList<KIOMPutIfAbsent> mutators = new ArrayList<KIOMPutIfAbsent>(NUM_THREADS);
+    for (int i = 0; i < NUM_THREADS; ++i) {
+      mutators.add(new KIOMPutIfAbsent(NUM_RUNS, objects, KIOM, factory));
+    }
+    for (int i = 0; i < NUM_THREADS; ++i) {
+      mutators.get(i).start();
+    }
+    int totalRemoves = 0;
+    for (int i = 0; i < NUM_THREADS; ++i) {
+      while (true) {
+        try {
+          KIOMPutIfAbsent m = mutators.get(i);
+          m.join();
+          totalRemoves += m.numRemoves;
+          log.info("testKIOMPutIfAbsent: mutator " + i + " had " + m.numRemoves + " removes");
+          break;
+        } catch (InterruptedException x) {
+          // don't care
+        }
+      }
+    }
+
+    log.info(
+        "Factory had "
+            + factory.numCalls
+            + " calls, objects.size() is "
+            + objects.size()
+            + " total successful removes was "
+            + totalRemoves);
+    assertMapsEqual(map, objects);
+    assertConsistency(map);
+    assertEquals(objects.size() + totalRemoves, factory.numCalls);
+  }
+
+  public void testKIOMConcurrentGet() {
+    final Map<Double, KeyedDoubleTestObject> map = newTestMap(10, null);
+    if (!(map instanceof KeyedIntObjectHash)) {
+      return;
+    }
+    final KeyedIntObjectHash<KeyedDoubleTestObject> SUT =
+        ((KeyedIntObjectHash<KeyedDoubleTestObject>) map);
+
+    final long MILLIS = 1000;
+
+    Thread setter =
+        new Thread() {
+          @Override
+          public void run() {
+            long t0 = System.currentTimeMillis();
+            while (System.currentTimeMillis() < t0 + MILLIS) {
+              for (int i = 0; i < SUT.capacity(); ++i) {
+                SUT.put(i, new KeyedDoubleTestObject(i));
+                SUT.removeKey(i);
+              }
+            }
+          }
+        };
+
+    setter.start();
+
+    long t0 = System.currentTimeMillis();
+    try {
+      while (System.currentTimeMillis() < t0 + MILLIS) {
+        // doesn't matter what key we get here - just has to be
+        SUT.get(1);
+      }
+      setter.join();
+    } catch (Exception x) {
+      fail("Unhandled exception: " + x);
+    }
+  }
+}

From 7a2fbaa3f1a1cb25449df244b00f76c63982d0d3 Mon Sep 17 00:00:00 2001
From: Larry Booker <lbooker42@gmail.com>
Date: Fri, 24 Jan 2025 14:13:17 -0800
Subject: [PATCH 6/6] Fix test failure

---
 src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java b/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java
index 52b9bdb..f78b606 100644
--- a/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java
+++ b/src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java
@@ -36,7 +36,7 @@ public String toString() {
   }
 
   public static final KeyedDoubleObjectKey<KeyedDoubleTestObject> keyDef =
-      new KeyedDoubleObjectKey<>() {
+      new KeyedDoubleObjectKey<KeyedDoubleTestObject>() {
         public Double getKey(KeyedDoubleTestObject v) {
           return v.getId();
         }
@@ -64,7 +64,7 @@ public boolean equalDoubleKey(double k, KeyedDoubleTestObject v) {
       };
 
   public static final KeyedIntObjectHash.ValueFactory<KeyedDoubleTestObject> factory =
-      new KeyedIntObjectHash.ValueFactory<>() {
+      new KeyedIntObjectHash.ValueFactory<KeyedDoubleTestObject>() {
         public KeyedDoubleTestObject newValue(Integer key) {
           return new KeyedDoubleTestObject(key);
         }
@@ -79,7 +79,7 @@ public KeyedDoubleTestObject newValue(int key) {
   private KeyedDoubleTestObject next;
 
   public static final IntrusiveChainedHashAdapter<KeyedDoubleTestObject> adapter =
-      new IntrusiveChainedHashAdapter<>() {
+      new IntrusiveChainedHashAdapter<KeyedDoubleTestObject>() {
         @Override
         public KeyedDoubleTestObject getNext(KeyedDoubleTestObject self) {
           return self.next;