8030016: HashMap.computeIfAbsent generates spurious access event

Reviewed-by: psandoz, bchristi

src/share/classes/java/util/HashMap.java

@@ -1116,13 +1116,13 @@ public class HashMap<K,V> extends AbstractMap<K,V>
             }
         }
         V v = mappingFunction.apply(key);
-        if (old != null) {
+        if (v == null) {
+            return null;
+        } else if (old != null) {
             old.value = v;
             afterNodeAccess(old);
             return v;
         }
-        else if (v == null)
-            return null;
         else if (t != null)
             t.putTreeVal(this, tab, hash, key, v);
         else {

test/java/util/LinkedHashMap/ComputeIfAbsentAccessOrder.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code 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 General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8030016
+ * @summary computeIfAbsent would generate spurious access
+ */
+
+import java.util.*;
+
+public class ComputeIfAbsentAccessOrder {
+    public static void main(String args[]) throws Throwable {
+        LinkedHashMap<String,Object> map = new LinkedHashMap<>(2, 0.75f, true);
+        map.put("first", null);
+        map.put("second", null);
+
+        map.computeIfAbsent("first", l -> null); // should do nothing
+
+        String key = map.keySet().stream()
+                .findFirst()
+                .orElseThrow(() -> new RuntimeException("no value"));
+        if(!"first".equals(key)) {
+            throw new RuntimeException("not expected value " + "first" + "!=" + key);
+        }
+    }
+}

test/java/util/Map/Defaults.java

@@ -80,18 +80,22 @@ public class Defaults {
 
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=withNull values=withNull")
     public void testPutIfAbsentNulls(String description, Map<IntegerEnum, String> map) {
+        // null -> null
         assertTrue(map.containsKey(null), "null key absent");
         assertNull(map.get(null), "value not null");
         assertNull(map.putIfAbsent(null, EXTRA_VALUE), "previous not null");
+        // null -> EXTRA_VALUE
         assertTrue(map.containsKey(null), "null key absent");
         assertSame(map.get(null), EXTRA_VALUE, "unexpected value");
         assertSame(map.putIfAbsent(null, null), EXTRA_VALUE, "previous not expected value");
         assertTrue(map.containsKey(null), "null key absent");
         assertSame(map.get(null), EXTRA_VALUE, "unexpected value");
         assertSame(map.remove(null), EXTRA_VALUE, "removed unexpected value");
+        // null -> <absent>
 
         assertFalse(map.containsKey(null), description + ": key present after remove");
         assertNull(map.putIfAbsent(null, null), "previous not null");
+        // null -> null
         assertTrue(map.containsKey(null), "null key absent");
         assertNull(map.get(null), "value not null");
         assertNull(map.putIfAbsent(null, EXTRA_VALUE), "previous not null");
@@ -100,15 +104,19 @@ public class Defaults {
 
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all")
     public void testPutIfAbsent(String description, Map<IntegerEnum, String> map) {
+        // 1 -> 1
         assertTrue(map.containsKey(KEYS[1]));
         Object expected = map.get(KEYS[1]);
         assertTrue(null == expected || expected == VALUES[1]);
         assertSame(map.putIfAbsent(KEYS[1], EXTRA_VALUE), expected);
         assertSame(map.get(KEYS[1]), expected);
 
+        // EXTRA_KEY -> <absent>
         assertFalse(map.containsKey(EXTRA_KEY));
         assertSame(map.putIfAbsent(EXTRA_KEY, EXTRA_VALUE), null);
         assertSame(map.get(EXTRA_KEY), EXTRA_VALUE);
+        assertSame(map.putIfAbsent(EXTRA_KEY, VALUES[2]), EXTRA_VALUE);
+        assertSame(map.get(EXTRA_KEY), EXTRA_VALUE);
     }
 
     @Test(dataProvider = "Map<IntegerEnum,String> rw=all keys=all values=all")
@@ -268,14 +276,28 @@ public class Defaults {
 
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=withNull values=withNull")
     public void testComputeIfAbsentNulls(String description, Map<IntegerEnum, String> map) {
+        // null -> null
         assertTrue(map.containsKey(null), "null key absent");
         assertNull(map.get(null), "value not null");
-        assertSame(map.computeIfAbsent(null, (k) -> EXTRA_VALUE), EXTRA_VALUE, description);
-        assertSame(map.get(null), EXTRA_VALUE, description);
+        assertSame(map.computeIfAbsent(null, (k) -> null), null,  "not expected result");
+        assertTrue(map.containsKey(null), "null key absent");
+        assertNull(map.get(null), "value not null");
+        assertSame(map.computeIfAbsent(null, (k) -> EXTRA_VALUE), EXTRA_VALUE, "not mapped to result");
+        // null -> EXTRA_VALUE
+        assertTrue(map.containsKey(null), "null key absent");
+        assertSame(map.get(null), EXTRA_VALUE,  "not expected value");
+        assertSame(map.remove(null), EXTRA_VALUE, "removed unexpected value");
+        // null -> <absent>
+        assertFalse(map.containsKey(null), "null key present");
+        assertSame(map.computeIfAbsent(null, (k) -> EXTRA_VALUE), EXTRA_VALUE, "not mapped to result");
+        // null -> EXTRA_VALUE
+        assertTrue(map.containsKey(null), "null key absent");
+        assertSame(map.get(null), EXTRA_VALUE,  "not expected value");
     }
 
     @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all")
     public void testComputeIfAbsent(String description, Map<IntegerEnum, String> map) {
+        // 1 -> 1
         assertTrue(map.containsKey(KEYS[1]));
         Object expected = map.get(KEYS[1]);
         assertTrue(null == expected || expected == VALUES[1], description + String.valueOf(expected));
@@ -283,8 +305,12 @@ public class Defaults {
         assertSame(map.computeIfAbsent(KEYS[1], (k) -> EXTRA_VALUE), expected, description);
         assertSame(map.get(KEYS[1]), expected, description);
 
+        // EXTRA_KEY -> <absent>
+        assertFalse(map.containsKey(EXTRA_KEY));
+        assertNull(map.computeIfAbsent(EXTRA_KEY, (k) -> null));
         assertFalse(map.containsKey(EXTRA_KEY));
         assertSame(map.computeIfAbsent(EXTRA_KEY, (k) -> EXTRA_VALUE), EXTRA_VALUE);
+        // EXTRA_KEY -> EXTRA_VALUE
         assertSame(map.get(EXTRA_KEY), EXTRA_VALUE);
     }