ConcurrentMap::compute clarification

[Jaromir Hamala]

Hi,

I stumbled upon an interesting issue with default implementation of
`compute(K key, BiFunction<? super K, ? super V, ? extends V>
remappingFunction)` in JDK8 `ConcurrentMap`.
According to its contract the default method implementation assumes map
implementations do not support null values.

This is the begin of the default implementation:

default V compute(K key, BiFunction<? super K, ? super V, ? extends V>
remappingFunction) {
        Objects.requireNonNull(remappingFunction);
        V oldValue = get(key);
        for(;;) {
            V newValue = remappingFunction.apply(key, oldValue);
            if (newValue == null) {
                // delete mapping
                if (oldValue != null || containsKey(key)) {
                    // something to remove
                    if (remove(key, oldValue)) {
[...]


Let's say we have an empty map and 2 threads:
T1 is calling the `compute('foo', someFunction)`
T2 is concurrently calling calling `put('foo', 'bar');`

so the T1 will get `oldValue = null`, but `containsKey()` will return
`true` - because T2 already created the mapping `foo -> bar`. Hence T1 will
call `remove('foo', null)` !

Contract of `remove()` says: `throws NullPointerException if the specified
key or value is null, and this map does not permit null keys or values
optional.` -> the T1 will throw NPE.
Is it a bug in default method impl or do I understand it wrong?

Cheers,
Jaromir

-- 
“Perfection is achieved, not when there is nothing more to add, but when
there is nothing left to take away.”
Antoine de Saint Exupéry