infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-03-03 04:30:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

8071667: HashMap.computeIfAbsent() adds entry that HashMap.get() does not find

Browse Source 
Throw ConcurrentModificationException from computeIfAbsent() & friends

Reviewed-by: chegar, psandoz
This commit is contained in:
Brent Christian 2015-04-02 12:33:03 -07:00
parent 7b1455163e
commit 6c5d70bfbc
4 changed files with 339 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
jdk/src/java.base/share/classes/java/util/HashMap.java
View File

@ -1082,6 +1082,16 @@ public class HashMap<K,V> extends AbstractMap<K,V>
return null;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link ConcurrentModificationException} if it is detected that the
* mapping function modifies this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* mapping function modified this map
*/
@Override
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction) {
@ -1115,7 +1125,9 @@ public class HashMap<K,V> extends AbstractMap<K,V>
return oldValue;
}
}
int mc = modCount;
V v = mappingFunction.apply(key);
if (mc != modCount) { throw new ConcurrentModificationException(); }
if (v == null) {
return null;
} else if (old != null) {
@ -1130,12 +1142,23 @@ public class HashMap<K,V> extends AbstractMap<K,V>
if (binCount >= TREEIFY_THRESHOLD - 1)
treeifyBin(tab, hash);
}
++modCount;
modCount = mc + 1;
++size;
afterNodeInsertion(true);
return v;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link ConcurrentModificationException} if it is detected that the
* remapping function modifies this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* remapping function modified this map
*/
@Override
public V computeIfPresent(K key,
BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
if (remappingFunction == null)
@ -1144,7 +1167,9 @@ public class HashMap<K,V> extends AbstractMap<K,V>
int hash = hash(key);
if ((e = getNode(hash, key)) != null &&
(oldValue = e.value) != null) {
int mc = modCount;
V v = remappingFunction.apply(key, oldValue);
if (mc != modCount) { throw new ConcurrentModificationException(); }
if (v != null) {
e.value = v;
afterNodeAccess(e);
@ -1156,6 +1181,16 @@ public class HashMap<K,V> extends AbstractMap<K,V>
return null;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link ConcurrentModificationException} if it is detected that the
* remapping function modifies this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* remapping function modified this map
*/
@Override
public V compute(K key,
BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
@ -1185,7 +1220,9 @@ public class HashMap<K,V> extends AbstractMap<K,V>
}
}
V oldValue = (old == null) ? null : old.value;
int mc = modCount;
V v = remappingFunction.apply(key, oldValue);
if (mc != modCount) { throw new ConcurrentModificationException(); }
if (old != null) {
if (v != null) {
old.value = v;
@ -1202,13 +1239,23 @@ public class HashMap<K,V> extends AbstractMap<K,V>
if (binCount >= TREEIFY_THRESHOLD - 1)
treeifyBin(tab, hash);
}
++modCount;
modCount = mc + 1;
++size;
afterNodeInsertion(true);
}
return v;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link ConcurrentModificationException} if it is detected that the
* remapping function modifies this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* remapping function modified this map
*/
@Override
public V merge(K key, V value,
BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
@ -1241,10 +1288,15 @@ public class HashMap<K,V> extends AbstractMap<K,V>
}
if (old != null) {
V v;
if (old.value != null)
if (old.value != null) {
int mc = modCount;
v = remappingFunction.apply(old.value, value);
else
if (mc != modCount) {
throw new ConcurrentModificationException();
}
} else {
v = value;
}
if (v != null) {
old.value = v;
afterNodeAccess(old);

63
jdk/src/java.base/share/classes/java/util/Hashtable.java
View File

@ -1000,6 +1000,16 @@ public class Hashtable<K,V>
return null;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link java.util.ConcurrentModificationException} if the mapping
* function modified this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* mapping function modified this map
*/
@Override
public synchronized V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) {
Objects.requireNonNull(mappingFunction);
@ -1016,7 +1026,9 @@ public class Hashtable<K,V>
}
}
int mc = modCount;
V newValue = mappingFunction.apply(key);
if (mc != modCount) { throw new ConcurrentModificationException(); }
if (newValue != null) {
addEntry(hash, key, newValue, index);
}
@ -1024,6 +1036,16 @@ public class Hashtable<K,V>
return newValue;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link java.util.ConcurrentModificationException} if the remapping
* function modified this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* remapping function modified this map
*/
@Override
public synchronized V computeIfPresent(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
Objects.requireNonNull(remappingFunction);
@ -1035,14 +1057,18 @@ public class Hashtable<K,V>
Entry<K,V> e = (Entry<K,V>)tab[index];
for (Entry<K,V> prev = null; e != null; prev = e, e = e.next) {
if (e.hash == hash && e.key.equals(key)) {
int mc = modCount;
V newValue = remappingFunction.apply(key, e.value);
if (mc != modCount) {
throw new ConcurrentModificationException();
}
if (newValue == null) {
if (prev != null) {
prev.next = e.next;
} else {
tab[index] = e.next;
}
modCount++;
modCount = mc + 1;
count--;
} else {
e.value = newValue;
@ -1052,7 +1078,16 @@ public class Hashtable<K,V>
}
return null;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link java.util.ConcurrentModificationException} if the remapping
* function modified this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* remapping function modified this map
*/
@Override
public synchronized V compute(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
Objects.requireNonNull(remappingFunction);
@ -1064,14 +1099,18 @@ public class Hashtable<K,V>
Entry<K,V> e = (Entry<K,V>)tab[index];
for (Entry<K,V> prev = null; e != null; prev = e, e = e.next) {
if (e.hash == hash && Objects.equals(e.key, key)) {
int mc = modCount;
V newValue = remappingFunction.apply(key, e.value);
if (mc != modCount) {
throw new ConcurrentModificationException();
}
if (newValue == null) {
if (prev != null) {
prev.next = e.next;
} else {
tab[index] = e.next;
}
modCount++;
modCount = mc + 1;
count--;
} else {
e.value = newValue;
@ -1080,7 +1119,9 @@ public class Hashtable<K,V>
}
}
int mc = modCount;
V newValue = remappingFunction.apply(key, null);
if (mc != modCount) { throw new ConcurrentModificationException(); }
if (newValue != null) {
addEntry(hash, key, newValue, index);
}
@ -1088,6 +1129,16 @@ public class Hashtable<K,V>
return newValue;
}
/**
* {@inheritDoc}
*
* <p>This method will, on a best-effort basis, throw a
* {@link java.util.ConcurrentModificationException} if the remapping
* function modified this map during computation.
*
* @throws ConcurrentModificationException if it is detected that the
* remapping function modified this map
*/
@Override
public synchronized V merge(K key, V value, BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
Objects.requireNonNull(remappingFunction);
@ -1099,14 +1150,18 @@ public class Hashtable<K,V>
Entry<K,V> e = (Entry<K,V>)tab[index];
for (Entry<K,V> prev = null; e != null; prev = e, e = e.next) {
if (e.hash == hash && e.key.equals(key)) {
int mc = modCount;
V newValue = remappingFunction.apply(e.value, value);
if (mc != modCount) {
throw new ConcurrentModificationException();
}
if (newValue == null) {
if (prev != null) {
prev.next = e.next;
} else {
tab[index] = e.next;
}
modCount++;
modCount = mc + 1;
count--;
} else {
e.value = newValue;

102
jdk/src/java.base/share/classes/java/util/Map.java
View File

@ -894,8 +894,8 @@ public interface Map<K,V> {
* to {@code null}), attempts to compute its value using the given mapping
* function and enters it into this map unless {@code null}.
*
* <p>If the function returns {@code null} no mapping is recorded. If
* the function itself throws an (unchecked) exception, the
* <p>If the mapping function returns {@code null}, no mapping is recorded.
* If the mapping function itself throws an (unchecked) exception, the
* exception is rethrown, and no mapping is recorded. The most
* common usage is to construct a new object serving as an initial
* mapped value or memoized result, as in:
@ -911,6 +911,7 @@ public interface Map<K,V> {
* map.computeIfAbsent(key, k -> new HashSet<V>()).add(v);
* }</pre>
*
* <p>The mapping function should not modify this map during computation.
*
* @implSpec
* The default implementation is equivalent to the following steps for this
@ -925,16 +926,27 @@ public interface Map<K,V> {
* }
* }</pre>
*
* <p>The default implementation makes no guarantees about detecting if the
* mapping function modifies this map during computation and, if
* appropriate, reporting an error. Non-concurrent implementations should
* override this method and, on a best-effort basis, throw a
* {@code ConcurrentModificationException} if it is detected that the
* mapping function modifies this map during computation. Concurrent
* implementations should override this method and, on a best-effort basis,
* throw an {@code IllegalStateException} if it is detected that the
* mapping function modifies this map during computation and as a result
* computation would never complete.
*
* <p>The default implementation makes no guarantees about synchronization
* or atomicity properties of this method. Any implementation providing
* atomicity guarantees must override this method and document its
* concurrency properties. In particular, all implementations of
* subinterface {@link java.util.concurrent.ConcurrentMap} must document
* whether the function is applied once atomically only if the value is not
* present.
* whether the mapping function is applied once atomically only if the value
* is not present.
*
* @param key key with which the specified value is to be associated
* @param mappingFunction the function to compute a value
* @param mappingFunction the mapping function to compute a value
* @return the current (existing or computed) value associated with
* the specified key, or null if the computed value is null
* @throws NullPointerException if the specified key is null and
@ -967,10 +979,12 @@ public interface Map<K,V> {
* If the value for the specified key is present and non-null, attempts to
* compute a new mapping given the key and its current mapped value.
*
* <p>If the function returns {@code null}, the mapping is removed. If the
* function itself throws an (unchecked) exception, the exception is
* rethrown, and the current mapping is left unchanged.
*
* <p>If the remapping function returns {@code null}, the mapping is removed.
* If the remapping function itself throws an (unchecked) exception, the
* exception is rethrown, and the current mapping is left unchanged.
*
* <p>The remapping function should not modify this map during computation.
*
* @implSpec
* The default implementation is equivalent to performing the following
* steps for this {@code map}, then returning the current value or
@ -987,16 +1001,27 @@ public interface Map<K,V> {
* }
* }</pre>
*
* <p>The default implementation makes no guarantees about detecting if the
* remapping function modifies this map during computation and, if
* appropriate, reporting an error. Non-concurrent implementations should
* override this method and, on a best-effort basis, throw a
* {@code ConcurrentModificationException} if it is detected that the
* remapping function modifies this map during computation. Concurrent
* implementations should override this method and, on a best-effort basis,
* throw an {@code IllegalStateException} if it is detected that the
* remapping function modifies this map during computation and as a result
* computation would never complete.
*
* <p>The default implementation makes no guarantees about synchronization
* or atomicity properties of this method. Any implementation providing
* atomicity guarantees must override this method and document its
* concurrency properties. In particular, all implementations of
* subinterface {@link java.util.concurrent.ConcurrentMap} must document
* whether the function is applied once atomically only if the value is not
* present.
* whether the remapping function is applied once atomically only if the
* value is not present.
*
* @param key key with which the specified value is to be associated
* @param remappingFunction the function to compute a value
* @param remappingFunction the remapping function to compute a value
* @return the new value associated with the specified key, or null if none
* @throws NullPointerException if the specified key is null and
* this map does not support null keys, or the
@ -1037,10 +1062,12 @@ public interface Map<K,V> {
* map.compute(key, (k, v) -> (v == null) ? msg : v.concat(msg))}</pre>
* (Method {@link #merge merge()} is often simpler to use for such purposes.)
*
* <p>If the function returns {@code null}, the mapping is removed (or
* remains absent if initially absent). If the function itself throws an
* (unchecked) exception, the exception is rethrown, and the current mapping
* is left unchanged.
* <p>If the remapping function returns {@code null}, the mapping is removed
* (or remains absent if initially absent). If the remapping function
* itself throws an (unchecked) exception, the exception is rethrown, and
* the current mapping is left unchanged.
*
* <p>The remapping function should not modify this map during computation.
*
* @implSpec
* The default implementation is equivalent to performing the following
@ -1063,16 +1090,27 @@ public interface Map<K,V> {
* }
* }</pre>
*
* <p>The default implementation makes no guarantees about detecting if the
* remapping function modifies this map during computation and, if
* appropriate, reporting an error. Non-concurrent implementations should
* override this method and, on a best-effort basis, throw a
* {@code ConcurrentModificationException} if it is detected that the
* remapping function modifies this map during computation. Concurrent
* implementations should override this method and, on a best-effort basis,
* throw an {@code IllegalStateException} if it is detected that the
* remapping function modifies this map during computation and as a result
* computation would never complete.
*
* <p>The default implementation makes no guarantees about synchronization
* or atomicity properties of this method. Any implementation providing
* atomicity guarantees must override this method and document its
* concurrency properties. In particular, all implementations of
* subinterface {@link java.util.concurrent.ConcurrentMap} must document
* whether the function is applied once atomically only if the value is not
* present.
* whether the remapping function is applied once atomically only if the
* value is not present.
*
* @param key key with which the specified value is to be associated
* @param remappingFunction the function to compute a value
* @param remappingFunction the remapping function to compute a value
* @return the new value associated with the specified key, or null if none
* @throws NullPointerException if the specified key is null and
* this map does not support null keys, or the
@ -1121,9 +1159,11 @@ public interface Map<K,V> {
* map.merge(key, msg, String::concat)
* }</pre>
*
* <p>If the function returns {@code null} the mapping is removed. If the
* function itself throws an (unchecked) exception, the exception is
* rethrown, and the current mapping is left unchanged.
* <p>If the remapping function returns {@code null}, the mapping is removed.
* If the remapping function itself throws an (unchecked) exception, the
* exception is rethrown, and the current mapping is left unchanged.
*
* <p>The remapping function should not modify this map during computation.
*
* @implSpec
* The default implementation is equivalent to performing the following
@ -1140,19 +1180,31 @@ public interface Map<K,V> {
* map.put(key, newValue);
* }</pre>
*
* <p>The default implementation makes no guarantees about detecting if the
* remapping function modifies this map during computation and, if
* appropriate, reporting an error. Non-concurrent implementations should
* override this method and, on a best-effort basis, throw a
* {@code ConcurrentModificationException} if it is detected that the
* remapping function modifies this map during computation. Concurrent
* implementations should override this method and, on a best-effort basis,
* throw an {@code IllegalStateException} if it is detected that the
* remapping function modifies this map during computation and as a result
* computation would never complete.
*
* <p>The default implementation makes no guarantees about synchronization
* or atomicity properties of this method. Any implementation providing
* atomicity guarantees must override this method and document its
* concurrency properties. In particular, all implementations of
* subinterface {@link java.util.concurrent.ConcurrentMap} must document
* whether the function is applied once atomically only if the value is not
* present.
* whether the remapping function is applied once atomically only if the
* value is not present.
*
* @param key key with which the resulting value is to be associated
* @param value the non-null value to be merged with the existing value
* associated with the key or, if no existing value or a null value
* is associated with the key, to be associated with the key
* @param remappingFunction the function to recompute a value if present
* @param remappingFunction the remapping function to recompute a value if
* present
* @return the new value associated with the specified key, or null if no
* value is associated with the key
* @throws UnsupportedOperationException if the {@code put} operation

147
jdk/test/java/util/Map/FunctionalCMEs.java Normal file
View File

@ -0,0 +1,147 @@
/*
* Copyright (c) 2015 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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.
*/
import java.util.Arrays;
import java.util.ConcurrentModificationException;
import java.util.HashMap;
import java.util.Hashtable;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.function.BiFunction;
import org.testng.annotations.Test;
import org.testng.annotations.DataProvider;
/**
* @test
* @bug 8071667
* @summary Ensure that ConcurrentModificationExceptions are thrown as specified from Map methods that accept Functions
* @author bchristi
* @build Defaults
* @run testng FunctionalCMEs
*/
public class FunctionalCMEs {
final static String KEY = "key";
@DataProvider(name = "Maps", parallel = true)
private static Iterator<Object[]> makeMaps() {
return Arrays.asList(
// Test maps that CME
new Object[]{new HashMap<>(), true},
new Object[]{new Hashtable<>(), true},
new Object[]{new LinkedHashMap<>(), true},
// Test default Map methods - no CME
new Object[]{new Defaults.ExtendsAbstractMap<>(), false}
).iterator();
}
@Test(dataProvider = "Maps")
public void testComputeIfAbsent(Map<String,String> map, boolean expectCME) {
checkCME(() -> {
map.computeIfAbsent(KEY, k -> {
putToForceRehash(map);
return "computedValue";
});
}, expectCME);
}
@Test(dataProvider = "Maps")
public void testCompute(Map<String,String> map, boolean expectCME) {
checkCME(() -> {
map.compute(KEY, mkBiFunc(map));
}, expectCME);
}
@Test(dataProvider = "Maps")
public void testComputeWithKeyMapped(Map<String,String> map, boolean expectCME) {
map.put(KEY, "firstValue");
checkCME(() -> {
map.compute(KEY, mkBiFunc(map));
}, expectCME);
}
@Test(dataProvider = "Maps")
public void testComputeIfPresent(Map<String,String> map, boolean expectCME) {
map.put(KEY, "firstValue");
checkCME(() -> {
map.computeIfPresent(KEY, mkBiFunc(map));
}, expectCME);
}
@Test(dataProvider = "Maps")
public void testMerge(Map<String,String> map, boolean expectCME) {
map.put(KEY, "firstValue");
checkCME(() -> {
map.merge(KEY, "nextValue", mkBiFunc(map));
}, expectCME);
}
@Test(dataProvider = "Maps")
public void testForEach(Map<String,String> map, boolean ignored) {
checkCME(() -> {
map.put(KEY, "firstValue");
putToForceRehash(map);
map.forEach((k,v) -> {
map.remove(KEY);
});
}, true);
}
@Test(dataProvider = "Maps")
public void testReplaceAll(Map<String,String> map, boolean ignored) {
checkCME(() -> {
map.put(KEY, "firstValue");
putToForceRehash(map);
map.replaceAll((k,v) -> {
map.remove(KEY);
return "computedValue";
});
},true);
}
private static void checkCME(Runnable code, boolean expectCME) {
try {
code.run();
} catch (ConcurrentModificationException cme) {
if (expectCME) { return; } else { throw cme; }
}
if (expectCME) {
throw new RuntimeException("Expected CME, but wasn't thrown");
}
}
private static BiFunction<String,String,String> mkBiFunc(Map<String,String> map) {
return (k,v) -> {
putToForceRehash(map);
return "computedValue";
};
}
private static void putToForceRehash(Map<String,String> map) {
for (int i = 0; i < 64; ++i) {
map.put(i + "", "value");
}
}
}