Note that there is a possible initialization circularity introduced by
this patch, which I think is harmless because:
- ThreadLocalRandom has just recently been enhanced to cope with such
situations - CHM needs ThreadLocalRandom in put() for example, when new
entry is being inserted, TLR invokes
Boolean.getBoolean("java.util.secureRandomSeed") as the last thing in
its static initializer but still works correctly even before this last
part of initializer is finished).
- When an invocation of Boolean.getBoolean ends up in the same
Properties instance (the System.getProperties()) it might be asking for
an entry in the same CHM instance (CHM.get()) which is just being
modified by put(). This amounts to re-entrance of CHM instance, but is
harmless as Boolean.getBoolean is only possibly invoked as the last
thing of various CHM modifications methods when the CHM state is already
consistent (see addCount() invocations in CHM).
Regards, Peter
On 05/13/2016 01:55 AM, Brent Christian wrote:
Update to the test, and additional comments:
http://cr.openjdk.java.net/~bchristi/8029891/webrev.5/webrev/
Thanks,
-Brent
On 5/12/16 4:15 PM, Mandy Chung wrote:
On May 12, 2016, at 2:44 PM, Brent Christian
<brent.christ...@oracle.com> wrote:
On 5/12/16 11:44 AM, Mandy Chung wrote:
This patch looks good.
To help future readers to understand this, it may be better to move:
1152 private transient ConcurrentHashMap<Object, Object> map =
1153 new ConcurrentHashMap<>(8);
to the beginning and add a comment describing what are lock-free
and what are synchronized (basically some part of your summary below).
Also a comment in Hashtable::defaultWriteHashtable and
readHashtable methods to mention that they are overridden by
Properties.
Good ideas.
In the GetResource.java test, what is the reason taking out:
73 URL u2 =
Thread.currentThread().getContextClassLoader().getResource("sun/util/resources/CalendarData.class”);
It was coming back null and failing the test, I think because
Jigsaw moved CalendarData into a different module (jdk.localedata, I
believe?).
src/java.base/share/classes/sun/util/resources/CalendarData.properties
is still in java.base. This is due to resource encapsulation.
Unnamed module calling ClassLoader::getResource of a resource in a
named module returns null.
I fiddled with @modules a bit, but stopped when I discovered that
when the test deadlocks, it hangs on the first call to getResource()
and doesn't get to this line.
I can look into getting it to load CalendarData again, if you like.
The launcher and builtin class loader implementation has changed so
much that the code path exercised JDK-6977738 no longer exists. I
think dropping line 73 is okay.
Mandy
