RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java - A Revival

[Peter Levart]

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.christian at 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
>>