On Jun 19, 2013, at 8:44 AM, Peter Levart <peter.lev...@gmail.com> wrote:
> On 06/19/2013 01:13 AM, Mike Duigou wrote: >> On Jun 18 2013, at 05:19 , Doug Lea wrote: >> >>> On 06/17/13 19:30, Mike Duigou wrote: >>> >>>> I had to add the improved default for ConcurrentMap which was present in >>>> the lambda repo in order to have correct behaviour. Since getOrDefault is >>>> already in ConcurrentMap I will include this but we have to be careful >>>> when we do a jsr 166 syncup to make sure that the defaults don't get lost. >>>> >>> Now synched up on my side. >>> >>> -Doug >>> >> >> >> Per a suggestion from Remi I updated the ConcurrentMap.replaceAll default to >> use forEach. This trades off the entrySet iterator overhead for creation of >> a capturing BiConsumer lambda. >> >> http://hg.openjdk.java.net/jdk8/tl/jdk/raw-diff/1f7cbe4829fe/src/share/classes/java/util/concurrent/ConcurrentMap.java >> >> Mike > > Hi Mike, Remi, > > Since forEach implementation can be taken from default Map.forEach in some > implementations of ConcurrentMap, and that implementation is based on > entrySet Iterator, isn't it dangerous for this to trigger > ConcurrentModificationException in some implementation of ConcurrentMap? I > see nothing in the spec. of ConcurrentMap that suggests it's entrySet > iterators are never fail-fast. They can be prepared for modifications from > other threads (synchronization), but may not tolerate re-entrant calls. > > For example some implementation of (Concurrent)Map could be structurally > modified as a result of Map.replace(key, old, new) - imagine a > ConcurrentWeakHashMap that expunges stale entries on each call - and forEach > iteration may not be prepared to handle such situations. > Or in general when explicitly iterating on the entrySet. for (Map.Entry e : cm.entrySet()) { cm.replace(e.getKey(), e.getValue(), newValue); } A concurrent map implementation that provides a fail-fast iterator for in-thread modification is asking for trouble IMHO! Instead i would expect the iterator to be weakly consistent and never throw a CME. -- This is another little oddity in Map.forEach: try { k = entry.getKey(); v = entry.getValue(); } catch(IllegalStateException ise) { throw new ConcurrentModificationException(ise); } I would presume the entries from CconcurrentMap.entrySet would not throw ISEs Paul.
