Hi Brent,
If 'map' field could be observed as null (which I think is only
possible, if Properties object is unsafely published), then so can
'defaults' field. You could make it final as well if it was not protected.
So I'm thinking what kind of memory fences would make those fields
behave so that they would not be observed uninitialized even when the
instance is published via data race. I think that the following would be
enough, for example:
private Properties(Properties defaults, int initialCapacity) {
// use package-private constructor to
// initialize unused fields with dummy values
super((Void) null);
map = new ConcurrentHashMap<>(initialCapacity);
this.defaults = defaults;
VarHandle.storeStoreFence();
}
...
public String getProperty(String key) {
VarHandle.loadLoadFence();
Object oval = map.get(key);
String sval = (oval instanceof String) ? (String)oval : null;
return ((sval == null) && (defaults != null)) ?
defaults.getProperty(key) : sval;
}
...of course, you would have to put loadLoadFence() at the beggining of
each method that uses any of those two fields. Accesses to 'defaults' in
subclasses wouldn't be included, but at least accesses in Properties
class would be covered.
Regards, Peter
On 06/15/18 17:44, Brent Christian wrote:
Hi,
In JDK 9, 8029891[1] refactored java.util.Properties to store its
values in an internal ConcurrentHashMap, and removed synchronization
from "reader" methods in order to avoid potential hangs/deadlocks
during classloading.
Claes has noticed that there is the possibility of the new 'map' field
being observed with its default value (null), before being set.
After looking at the JSR 133 FAQ[2], I agree with Claes that we should
make 'map' a field final.
Please review my change to do this:
Webrev:
http://cr.openjdk.java.net/~bchristi/8199435/webrev/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8199435
Thanks,
-Brent
1. https://bugs.openjdk.java.net/browse/JDK-8029891
2.
https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#finalRight
