Wrong implementation of Oracle Java ConcurrentHashMap?

Question

I am testing ConcurrentHashMap on Oracle's Java 8 implementation:

ConcurrentMap<String, String> concurrentMap = new ConcurrentHashMap<>();
String result = concurrentMap.computeIfAbsent("A", k -> "B");
System.out.println(result);  // "B"
result = concurrentMap.putIfAbsent("AA", "BB");
System.out.println(result);  // null

The Javadoc of computeIfAbsent does say that

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);
}

It said then returning the current value or null if now absent. So shouldn't it be returning null? Given that putIfAbsent is also returning null.

What am I missing here?

Answer 1

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:

@Override
default 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;
}