Bug ID: JDK-8035284 AbstractMap unnecessarily initializes two volatiles to null

Versions (Unresolved/Resolved/Fixed)

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 8	JDK 9
8u20Fixed	9 b08Fixed

In AbstractMap there are two volatile fields that are explicitly set to null as part of initialization.
This should be unnecessary since all fields are guaranteed to be null.

public abstract class AbstractMap<K,V> implements Map<K,V> {
...
    /**
     * Each of these fields are initialized to contain an instance of the
     * appropriate view the first time this view is requested.  The views are
     * stateless, so there's no reason to create more than one of each.
     */
    transient volatile Set<K>        keySet = null;
    transient volatile Collection<V> values = null;
...
}


There might be other similar instance in the JDK that could be fixed at the same time.

@Paul, having a class that rely on an implementation detail of another class is wrong in my opinion. So I think it's Ok to fix that issue.
19-03-2014
See IntelliJ's code inspection "Redundant field initilization" (not enabled by default, run it explicitly by name). Also, it questionable why these fields are volatile, atomic access is sufficient, which we get by default, racy is OK e.g. see redefined use in ConcurrentHashMap: private transient KeySetView<K,V> keySet; private transient ValuesView<K,V> values; private transient EntrySetView<K,V> entrySet; public KeySetView<K,V> keySet() { KeySetView<K,V> ks; return (ks = keySet) != null ? ks : (keySet = new KeySetView<K,V>(this, null)); } However, goodness knows what code out there is inheriting from AbstractMap and might be relying on a happens-before relationship, so conservatively we probably cannot change it :-(
19-02-2014
I agree with the comment about finding other instances of this as it has come up a number of times in other areas too. It would be nice if javac didn't generate the initialization but that would require proving that there aren't any side effects. Minimally an inspector to find the cases would be useful as it wouldn't be too hard to see if there are any in potentially performance sensitive code.
19-02-2014
The fixVersion should never have more than one version so I've changed it to 9. A backport for 8u20 can be created in advance or it will be created automatically when the change is pushed.
19-02-2014
In Java ME we used to have a commenting convention to indicate that the program was taking advantage of default initialization, and that an initializer wasn't forgotten. Something like: transient volatile Set<K> keySet; // = null On the other hand, I question whether fixing this up all over the JDK is the right thing to do. Can't there be some optimization somewhere that deals with this? It seems silly that an apparently innocuous change to the code makes such a big difference in performance.
19-02-2014
I will try to find other similar instances to fix at the same time.
19-02-2014

Relates :	JDK-8145539 - (coll) AbstractMap.keySet and .values should not be volatile
Relates :	JDK-6736490 - Remove unnecessary explicit initialization of volatile variables