On 2/5/15 12:46 AM, Paul Sandoz wrote:
On Feb 5, 2015, at 1:36 AM, Brent Christian
<brent.christ...@oracle.com> wrote:
I prefer this approach of discouraging/preventing side-effects via
CME, rather than allowing them.
...
Regarding the default methods:
Would we be able to make a "best-effort" detection of
comodification...
>>
My inclination is to leave these as is and focus on the most common
concrete implementations.
Fair enough. So, spec-wise...
For Map, I'd like to add some "words of discouragement" regarding
modifying the map from function code. Maybe also encourage implementing
classes to detect and throw CME? (The default method will not do so.)
For HashMap: update the doc to say concurrent mods will results in a
CME, add @throws.
For ConcurrentMap: It does not discourage modifying the map from
function code, but it should IMO. It should not encourage subclasses to
throw CME.
ConcurrentHashMap: Does it need any changes? It already mentions that
one "must not attempt to update any other mappings of this map." FWIW,
giving the side-effecty mappingFunction from 8071667 to a CHM hangs for
me; I've not investigated. :\
I don't think any changes are needed for concrete Map impls that inherit
the default Map methods (e.g. TreeMap, IdentityHashMap)
Here's an initial rough cut, for computeIfAbsent():
diff -r 330dcd651f3b src/java.base/share/classes/java/util/Map.java
--- a/src/java.base/share/classes/java/util/Map.java Mon Feb 02
12:35:18 2015 -0800
+++ b/src/java.base/share/classes/java/util/Map.java Fri Feb 06
12:49:19 2015 -0800
@@ -925,6 +925,11 @@
* }
* }</pre>
*
+ * <p>The mappingFunction itself should not make changes to this map.
+ * Implementing classes are encouraged to detect such modifications and
+ * throw ConcurrentModificationException. The default
implementation does
+ * not do so.
+ *
* <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
diff -r 330dcd651f3b src/java.base/share/classes/java/util/HashMap.java
--- a/src/java.base/share/classes/java/util/HashMap.java Mon Feb
02 12:35:18 2015 -0800
+++ b/src/java.base/share/classes/java/util/HashMap.java Fri Feb
06 12:49:19 2015 -0800
@@ -1082,6 +1082,17 @@
return null;
}
+ /**
+ * {@inheritDoc}
+ *
+ * <p>The mappingFunction itself should not make changes to this map.
+ * If the function causes a structural modification to the map, a
+ * ConcurrentModificationException will be thrown. As with
iterators, this
+ * exception is thrown on a best-effort basis.
+ *
+ * @throws ConcurrentModificationException if a structural change was
+ * detected while executing the mappingFunction
+ */
@Override
public V computeIfAbsent(K key,
Function<? super K, ? extends V>
mappingFunction) {
diff -r 330dcd651f3b
src/java.base/share/classes/java/util/concurrent/ConcurrentMap.java
---
a/src/java.base/share/classes/java/util/concurrent/ConcurrentMap.java
Mon Feb 02 12:35:18 2015 -0800
+++
b/src/java.base/share/classes/java/util/concurrent/ConcurrentMap.java
Fri Feb 06 12:49:19 2015 -0800
@@ -305,6 +305,8 @@
* threads attempt updates including potentially calling the mapping
* function multiple times.
*
+ * <p>The mappingFunction itself should not make changes to this map.
+ *
* <p>This implementation assumes that the ConcurrentMap cannot
contain null
* values and {@code get()} returning null unambiguously means the
key is
Thanks,
-Brent
