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

[Lois Foltan]

On 4/18/2016 3:06 AM, Stefan Karlsson wrote:
> On 2016-04-15 21:45, Alan Bateman wrote:
>>
>> On 15/04/2016 18:02, Lois Foltan wrote:
>>>
>>> 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?
>> There will be a number of threads running before the base module is 
>> defined to the VM. As things stand the the java threads at this point 
>> will be the Common-Cleaner, Finalizer, Reference Handler and Signal 
>> Handler.
>
> So, are you saying that we need the atomics?
>
> The java_lang_Class::create_mirror function isn't multi-thread safe, 
> and must already be guarded by a lock (SystemDictionary_lock AFAICT). 
> The increment in Unsafe_DefineAnonymousClass0, will only be done once, 
> for the single InstanceKlass instance in the CLD. And all reads of 
> _keep_alive from the GC are done during safepoints.
The anonymous class is inserted in the fixup mirror and fixup module 
lists during java_lang_Class::create_mirror() before it is made public 
or "published" as loaded.  So the two instances where the reference 
count is incremented, Unsafe_DefineAnonymousClass0 and in 
java_lang_Class::create_mirror(), are guarded by a lock as well as the 
decrement in Unsafe_DefineAnonymousClass0.  No other thread has access 
to the class during this time, as it is being loaded.
>
> How does ModuleEntryTable::patch_javabase_entries guard against 
> concurrent inserts into the _fixup_module_field_list list?
That leaves the decrement in ModuleEntryTable::patch_javabase_entries() 
as possibly unguarded. This only occurs when the VM is called to define 
the module java.base.  I believe this should be okay but will double check.

Thanks,
Lois

>
> thanks,
> StefanK
>
>
>>
>> -Alan
>