From 1e845ac91f472847e7c8068385d2db632f0000cb Mon Sep 17 00:00:00 2001 From: Mike Duigou Date: Fri, 13 Dec 2013 13:35:35 -0800 Subject: [PATCH] 8030016: HashMap.computeIfAbsent generates spurious access event Reviewed-by: psandoz, bchristi --- jdk/src/share/classes/java/util/HashMap.java | 6 +-- .../ComputeIfAbsentAccessOrder.java | 47 +++++++++++++++++++ jdk/test/java/util/Map/Defaults.java | 30 +++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 jdk/test/java/util/LinkedHashMap/ComputeIfAbsentAccessOrder.java diff --git a/jdk/src/share/classes/java/util/HashMap.java b/jdk/src/share/classes/java/util/HashMap.java index 1183952eb06..2bb38f31be7 100644 --- a/jdk/src/share/classes/java/util/HashMap.java +++ b/jdk/src/share/classes/java/util/HashMap.java @@ -1116,13 +1116,13 @@ public class HashMap extends AbstractMap } } 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 { diff --git a/jdk/test/java/util/LinkedHashMap/ComputeIfAbsentAccessOrder.java b/jdk/test/java/util/LinkedHashMap/ComputeIfAbsentAccessOrder.java new file mode 100644 index 00000000000..532f2f78512 --- /dev/null +++ b/jdk/test/java/util/LinkedHashMap/ComputeIfAbsentAccessOrder.java @@ -0,0 +1,47 @@ +/* + * 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 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); + } + } +} diff --git a/jdk/test/java/util/Map/Defaults.java b/jdk/test/java/util/Map/Defaults.java index 46a49a63a51..ed8d2992cec 100644 --- a/jdk/test/java/util/Map/Defaults.java +++ b/jdk/test/java/util/Map/Defaults.java @@ -80,18 +80,22 @@ public class Defaults { @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testPutIfAbsentNulls(String description, Map 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 -> 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 rw=true keys=all values=all") public void testPutIfAbsent(String description, Map 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 -> 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 rw=all keys=all values=all") @@ -268,14 +276,28 @@ public class Defaults { @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testComputeIfAbsentNulls(String description, Map 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 -> + 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 rw=true keys=all values=all") public void testComputeIfAbsent(String description, Map 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 -> + 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); }