JDK-8145164 : Default implementation of ConcurrentMap::compute can throw NPE
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.concurrent
  • Affected Version: 8u66,9
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2015-12-09
  • Updated: 2019-05-21
  • Resolved: 2016-01-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b104Fixed
Related Reports
Duplicate :  
Description
FULL PRODUCT VERSION :
jara@hazelpad:~/devel/oss/maven-plugins$ java -version
java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)


ADDITIONAL OS VERSION INFORMATION :
irrelevant

EXTRA RELEVANT SYSTEM CONFIGURATION :
Paul Sandoz has confirmed it's a bug. See http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037432.html

A DESCRIPTION OF THE PROBLEM :
Default implementation of `compute(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction)` in JDK8 `ConcurrentMap` 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.  


REPRODUCIBILITY :
This bug can be reproduced always.


Comments
ConcurrentMap doc bugs: No @throws IllegalArgumentException, even though methods are called that document that exception. computeIfAbsent states: * The default implementation may retry these steps when multiple threads * attempt updates including potentially calling the remapping function * multiple times. but in fact all operations are attempted at most once.
14-12-2015

Yes, this is a bug. It is unlikely to be observed because you need a ConcurrentMap that does not override the default methods but _does_ throw NPE in remove(x, null) which JDK implementations do not, not even when null values are not permitted. These methods could use better testing and documentation as well.
14-12-2015

Tried to build a test case around the scenario suggested by the submitter, attached is the same. JDK 8u66 - Fail JDK 8u72 - Fail JDK 9eaB93 - Fail
10-12-2015