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

[Dean Long]

On 4/15/2016 10:02 AM, Lois Foltan wrote:
>
> 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?
>

It does seems excessive if we are single threaded.  So calling 
Universe::is_module_initialized() should always return false in those 
functions?  If so, then you could add an assert.

dl

> 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
>>>>
>>>
>>
>