RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

[Brent Christian]

Hi,

Here is an updated treatment of computeIfAbsent():

http://cr.openjdk.java.net/~bchristi/8071667/webrev.1/

There's a webrev, as well as built docs for Map[1], ConcurrentMap[2], 
and HashMap[3] (and I now realize there should be Hashtable as well).

I've made use of @implSpec and friends in a way I thought was sensible 
(though things could be arranged differently - I can see where some 
might find the additional headings in the rendered JavaDoc a little 
distracting ).  Let me know what you think.

Thanks,
-Brent

1. 
http://cr.openjdk.java.net/~bchristi/8071667/webrev.1/docs/api/java/util/Map.html#computeIfAbsent-K-java.util.function.Function-

2. 
http://cr.openjdk.java.net/~bchristi/8071667/webrev.1/docs/api/java/util/concurrent/ConcurrentMap.html#computeIfAbsent-K-java.util.function.Function-

3. 
http://cr.openjdk.java.net/~bchristi/8071667/webrev.1/docs/api/java/util/HashMap.html#computeIfAbsent-K-java.util.function.Function-


On 3/11/15 2:05 AM, Paul Sandoz wrote:
>
> On Mar 11, 2015, at 12:34 AM, Brent Christian <brent.christian at oracle.com> wrote:
>
>> Hi, Paul
>>
>> On 3/10/15 8:29 AM, Paul Sandoz wrote:
>>>
>>> On the Map.compute* methods.
>>>
>>> Perhaps we can reuse similar language to that we added for Matcher:
>>>
>>> * The mapping function should not modify this map during computation.
>>> * This method will, on a best-effort basis, throw a
>>> * ConcurrentModification if such modification is detected.
>>>
>>> It's tempting to place the second sentence on the @throws
>>> ConcurrentModificationException as i believe will not show up for
>>> methods on ConcurrentMap, so keeps it somewhat contained
>>> documentation wise.
>>
>> The language in Matcher is nice, and we can use it in HashMap.  But the default code in Map.compute* won't throw a CME (no iteration).
>
> That's ok, we can call that out if necessary in the @implSpec.
>
>
>>   For the compute* methods in the Map interface, I think we should stick to recommending against modifying the Map during the computation,
>
> Yes.
>
>
>> and encouraging implementing classes to check for CME.
>>
>
> Yes. My suggestion was to try and push much of the CME-related specification onto the @throws CME docs. That way less of this will appear on ConcurrentMap where CME does not make sense.
>
>
>>>> In Map/HashMap, there are already two methods that accept lambdas
>>>> and throw a CME: forEach(BiConsumer) replaceAll(BiFunction)
>>>>
>>>> (The default code does not throw a CME ifitems are added).
>>>
>>> Right, although the iterator might still be fail-fast.
>>
>> Ahh - I needed to update my test code (if a Map only has a single entry, the iterator doesn't CME when items are added because a second "iteration" never happens to detect the changed modCount).  A Map with >= 2 entries is needed for the default code to CME due to items being added during forEach()/replaceAll().
>>
>
> Yeah, it's all "best-effort".
>
> Paul.
>