Wrong implementation of Oracle Java ConcurrentHashMap? Wrong implementation of Oracle Java ConcurrentHashMap? oracle oracle

Wrong implementation of Oracle Java ConcurrentHashMap?


The code example of ConcurrentMap.computeIfAbsent is not reflecting the actual intention, most likely a mistake caused by the non-intuitive behavior of putIfAbsent, while the implementation obeys the documented intention. This has been reported in JDK-8174087 and fixed in Java 9

Note that the contract for Map.computeIfAbsent is

Implementation Requirements:

The default implementation is equivalent to the following steps for this map, then returning the current value or null if now absent:

if (map.get(key) == null) {    V newValue = mappingFunction.apply(key);    if (newValue != null)        map.put(key, newValue);}

omitting the return statement. But clearly says

Returns:

the current (existing or computed) value associated with the specified key, or null if the computed value is null

It is the documentation of ConcurrentMap.computeIfAbsent that tries to incorporate the concurrency aspect, falling for the non-inuitive behavior of putIfAbsent:

Implementation Requirements:

The default implementation is equivalent to the following steps for this map, then returning the current value or null if now absent:

if (map.get(key) == null) {    V newValue = mappingFunction.apply(key);    if (newValue != null)        return map.putIfAbsent(key, newValue);}

but it still says

Returns:

the current (existing or computed) value associated with the specified key, or null if the computed value is null

and the documented intention should have precedence over a code example. Note that the actual default implementation of ConcurrentMap.computeIfAbsent is in line with the documented intention:

@Overridedefault V computeIfAbsent(K key,        Function<? super K, ? extends V> mappingFunction) {    Objects.requireNonNull(mappingFunction);    V v, newValue;    return ((v = get(key)) == null &&            (newValue = mappingFunction.apply(key)) != null &&            (v = putIfAbsent(key, newValue)) == null) ? newValue : v;}

So the implementation of ConcurrentHashMap.computeIfAbsent does conform to the documented intention of both, ConcurrentMap.computeIfAbsent and Map.computeIfAbsent regarding the returned value and is also equivalent to the default implementation provided by the interfaces.

For completeness, the default implementation of Map.computeIfAbsent is

default V computeIfAbsent(K key,        Function<? super K, ? extends V> mappingFunction) {    Objects.requireNonNull(mappingFunction);    V v;    if ((v = get(key)) == null) {        V newValue;        if ((newValue = mappingFunction.apply(key)) != null) {            put(key, newValue);            return newValue;        }    }    return v;}


Actual code from javadoc:

 if (map.get(key) == null) { V newValue = mappingFunction.apply(key);     if (newValue != null)         map.put(key, newValue);  // <-     } }

As you can see there is no return keyword in the marked line.

Section "return" also says:

Returns: the current (existing or computed) value associated with the specified key, or null if the computed value is null