RFR (S): JDK-8152949: Jigsaw crash when Klass in _fixup_module_field_list is unloaded

[Lois Foltan]

On 4/15/2016 9:11 AM, Stefan Karlsson wrote:
> Hi Lois,
>
> On 2016-04-15 14:50, Lois Foltan wrote:
>>
>> On 4/14/2016 4:23 PM, Dean Long wrote:
>>> Do the inc_keep_alive()  and dec_keep_alive() updates need to be 
>>> atomic by any chance?
>>
>> Thanks Dean for the review and good point.  I will make that change 
>> and send out an updated webrev.
>
> When would we ever race on the _keep_alive variable? Or is this more 
> of a defensive change to safeguard against future changes?

Hi Stefan,

In start up before module system initialization in complete I believe 
the VM is single threaded, so the increment/decrement reference counts 
do not need to be atomic.  Adding it is a defensive move in case the 
reference count is ever used passed start up in the future.  It kind of 
does seem a bit excessive, sounds like you agree?

Thanks,
Lois

>
> Thanks,
> StefanK
>
>> Lois
>>
>>>
>>> dl
>>>
>>> On 4/14/2016 12:29 PM, Lois Foltan wrote:
>>>> Hello,
>>>>
>>>> Please review the following fix:
>>>>
>>>> Webrev:
>>>>       http://cr.openjdk.java.net/~lfoltan/bug_jdk8152949/
>>>>
>>>> Bug: Jigsaw crash when Klass in _fixup_module_field_list is unloaded
>>>>       https://bugs.openjdk.java.net/browse/JDK-8152949
>>>>
>>>> Summary:
>>>> Prior to java.base being defined to the VM by the module system 
>>>> initialization, classes loaded must be saved on a fixup list in 
>>>> order to later have their java.lang.Class' module field patched 
>>>> with java.base's java.lang.reflect.Module object once java.base is 
>>>> defined. Before module system initialization is complete, all 
>>>> classes loaded must have java.base as their defining module and be 
>>>> loaded by the boot loader.   It was erroneously assumed that all 
>>>> classes placed on the module fixup list therefore would not die 
>>>> before java.base was defined.  This assumption did not hold for 
>>>> anonymous classes which have a shorter lifetime than the boot 
>>>> loader.  Test cases run with a small heap, -Xmx2m, would cause GC 
>>>> to unload the anonymous classes on the fixup list, later causing 
>>>> issues when an attempt was made to patch these classes with 
>>>> java.base's java.lang.reflect.Module object. Thank you to Per Liden 
>>>> and Stefan Karlsson for contributing this fix for the runtime team.
>>>>
>>>> Test:
>>>> - java/lang, java/util, java/io, all Hotspot jtreg tests, Hotspot 
>>>> colocated tests & noncolo.quick.testlist
>>>> - several iterations of ConcurrentLinkedQueue/RemoveLeak.java which 
>>>> exhibited the problem
>>>
>>
>