> 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
