diff --git a/jdk/src/java.base/share/classes/java/util/AbstractMap.java b/jdk/src/java.base/share/classes/java/util/AbstractMap.java index 20d0cac928d..0b76ddd76b1 100644 --- a/jdk/src/java.base/share/classes/java/util/AbstractMap.java +++ b/jdk/src/java.base/share/classes/java/util/AbstractMap.java @@ -304,9 +304,28 @@ public abstract class AbstractMap implements Map { * 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. + * + *

Since there is no synchronization performed while accessing these fields, + * it is expected that java.util.Map view classes using these fields have + * no non-final fields (or any fields at all except for outer-this). Adhering + * to this rule would make the races on these fields benign. + * + *

It is also imperative that implementations read the field only once, + * as in: + * + *

 {@code
+     * public Set keySet() {
+     *   Set ks = keySet;  // single racy read
+     *   if (ks == null) {
+     *     ks = new KeySet();
+     *     keySet = ks;
+     *   }
+     *   return ks;
+     * }
+     *}
*/ - transient volatile Set keySet; - transient volatile Collection values; + transient Set keySet; + transient Collection values; /** * {@inheritDoc} @@ -325,8 +344,9 @@ public abstract class AbstractMap implements Map { * method will not all return the same set. */ public Set keySet() { - if (keySet == null) { - keySet = new AbstractSet() { + Set ks = keySet; + if (ks == null) { + ks = new AbstractSet() { public Iterator iterator() { return new Iterator() { private Iterator> i = entrySet().iterator(); @@ -361,8 +381,9 @@ public abstract class AbstractMap implements Map { return AbstractMap.this.containsKey(k); } }; + keySet = ks; } - return keySet; + return ks; } /** @@ -382,8 +403,9 @@ public abstract class AbstractMap implements Map { * method will not all return the same collection. */ public Collection values() { - if (values == null) { - values = new AbstractCollection() { + Collection vals = values; + if (vals == null) { + vals = new AbstractCollection() { public Iterator iterator() { return new Iterator() { private Iterator> i = entrySet().iterator(); @@ -418,8 +440,9 @@ public abstract class AbstractMap implements Map { return AbstractMap.this.containsValue(v); } }; + values = vals; } - return values; + return vals; } public abstract Set> entrySet(); diff --git a/jdk/src/java.base/share/classes/java/util/EnumMap.java b/jdk/src/java.base/share/classes/java/util/EnumMap.java index 141e10911a8..b665eaad856 100644 --- a/jdk/src/java.base/share/classes/java/util/EnumMap.java +++ b/jdk/src/java.base/share/classes/java/util/EnumMap.java @@ -380,10 +380,11 @@ public class EnumMap, V> extends AbstractMap */ public Set keySet() { Set ks = keySet; - if (ks != null) - return ks; - else - return keySet = new KeySet(); + if (ks == null) { + ks = new KeySet(); + keySet = ks; + } + return ks; } private class KeySet extends AbstractSet { @@ -418,10 +419,11 @@ public class EnumMap, V> extends AbstractMap */ public Collection values() { Collection vs = values; - if (vs != null) - return vs; - else - return values = new Values(); + if (vs == null) { + vs = new Values(); + values = vs; + } + return vs; } private class Values extends AbstractCollection { diff --git a/jdk/src/java.base/share/classes/java/util/HashMap.java b/jdk/src/java.base/share/classes/java/util/HashMap.java index 17b58b150d1..fd4d9b11c2d 100644 --- a/jdk/src/java.base/share/classes/java/util/HashMap.java +++ b/jdk/src/java.base/share/classes/java/util/HashMap.java @@ -902,8 +902,12 @@ public class HashMap extends AbstractMap * @return a set view of the keys contained in this map */ public Set keySet() { - Set ks; - return (ks = keySet) == null ? (keySet = new KeySet()) : ks; + Set ks = keySet; + if (ks == null) { + ks = new KeySet(); + keySet = ks; + } + return ks; } final class KeySet extends AbstractSet { @@ -949,8 +953,12 @@ public class HashMap extends AbstractMap * @return a view of the values contained in this map */ public Collection values() { - Collection vs; - return (vs = values) == null ? (values = new Values()) : vs; + Collection vs = values; + if (vs == null) { + vs = new Values(); + values = vs; + } + return vs; } final class Values extends AbstractCollection { diff --git a/jdk/src/java.base/share/classes/java/util/IdentityHashMap.java b/jdk/src/java.base/share/classes/java/util/IdentityHashMap.java index efd759f59a5..bd1e217e7ff 100644 --- a/jdk/src/java.base/share/classes/java/util/IdentityHashMap.java +++ b/jdk/src/java.base/share/classes/java/util/IdentityHashMap.java @@ -964,10 +964,11 @@ public class IdentityHashMap */ public Set keySet() { Set ks = keySet; - if (ks != null) - return ks; - else - return keySet = new KeySet(); + if (ks == null) { + ks = new KeySet(); + keySet = ks; + } + return ks; } private class KeySet extends AbstractSet { @@ -1069,10 +1070,11 @@ public class IdentityHashMap */ public Collection values() { Collection vs = values; - if (vs != null) - return vs; - else - return values = new Values(); + if (vs == null) { + vs = new Values(); + values = vs; + } + return vs; } private class Values extends AbstractCollection { diff --git a/jdk/src/java.base/share/classes/java/util/LinkedHashMap.java b/jdk/src/java.base/share/classes/java/util/LinkedHashMap.java index 15ade87276e..0253cd51bfd 100644 --- a/jdk/src/java.base/share/classes/java/util/LinkedHashMap.java +++ b/jdk/src/java.base/share/classes/java/util/LinkedHashMap.java @@ -528,8 +528,12 @@ public class LinkedHashMap * @return a set view of the keys contained in this map */ public Set keySet() { - Set ks; - return (ks = keySet) == null ? (keySet = new LinkedKeySet()) : ks; + Set ks = keySet; + if (ks == null) { + ks = new LinkedKeySet(); + keySet = ks; + } + return ks; } final class LinkedKeySet extends AbstractSet { @@ -577,8 +581,12 @@ public class LinkedHashMap * @return a view of the values contained in this map */ public Collection values() { - Collection vs; - return (vs = values) == null ? (values = new LinkedValues()) : vs; + Collection vs = values; + if (vs == null) { + vs = new LinkedValues(); + values = vs; + } + return vs; } final class LinkedValues extends AbstractCollection { diff --git a/jdk/src/java.base/share/classes/java/util/TreeMap.java b/jdk/src/java.base/share/classes/java/util/TreeMap.java index 248ef934369..2da0a5e8b28 100644 --- a/jdk/src/java.base/share/classes/java/util/TreeMap.java +++ b/jdk/src/java.base/share/classes/java/util/TreeMap.java @@ -852,7 +852,11 @@ public class TreeMap */ public Collection values() { Collection vs = values; - return (vs != null) ? vs : (values = new Values()); + if (vs == null) { + vs = new Values(); + values = vs; + } + return vs; } /** diff --git a/jdk/src/java.base/share/classes/java/util/WeakHashMap.java b/jdk/src/java.base/share/classes/java/util/WeakHashMap.java index c89567a320a..1aa8ec4396d 100644 --- a/jdk/src/java.base/share/classes/java/util/WeakHashMap.java +++ b/jdk/src/java.base/share/classes/java/util/WeakHashMap.java @@ -866,7 +866,11 @@ public class WeakHashMap */ public Set keySet() { Set ks = keySet; - return (ks != null ? ks : (keySet = new KeySet())); + if (ks == null) { + ks = new KeySet(); + keySet = ks; + } + return ks; } private class KeySet extends AbstractSet { @@ -915,7 +919,11 @@ public class WeakHashMap */ public Collection values() { Collection vs = values; - return (vs != null) ? vs : (values = new Values()); + if (vs == null) { + vs = new Values(); + values = vs; + } + return vs; } private class Values extends AbstractCollection {