RFR (M) JDK-8163406: The fixup_module_list must be protected by Module_lock when inserting new entries

[Coleen Phillimore]

http://cr.openjdk.java.net/~lfoltan/bug_jdk8163406/webrev/src/share/vm/classfile/javaClasses.cpp.udiff.html

+ assert(!Universe::is_module_initialized(), "Incorrect 
java.lang.reflect.Module pre module system initialization");


Can you assert this here?  Can another thread initialize the module 
before this line?  It's not locked.

Also, could you make the module fixup list logic a function called from 
create_mirror()?  It's too long with too many different things 
happening.  We'd gotten it nice, short and concise at one point!

http://cr.openjdk.java.net/~lfoltan/bug_jdk8163406/webrev/src/share/vm/classfile/moduleEntry.cpp.html

Can you add a comment before

ModuleEntryTable::patch_javabase_entries

to make it clear that there's a module field in java.lang.Class instances?

Also, this line should be pulled out of the loop:

399     Thread* THREAD = Thread::current();
400     KlassHandle kh(THREAD, k);


Or better yet, remove both and use the implicit constructor for 
KlassHandles in the call to fixup_module_field (KlassHandle doesn't use 
THREAD for anything).

I think everything except these small comments looks good.

Coleen

On 9/7/16 11:31 AM, Lois Foltan wrote:
> Hello,
>
> Please review the following fix:
>
> Webrev:
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8163406/webrev/
>
> Bug: The fixup_module_list must be protected by Module_lock when 
> inserting new entries
> https://bugs.openjdk.java.net/browse/JDK-8163406
>
> Summary:
> During bootstrapping, after the load of java.lang.Class, the method 
> java_lang_Class::create_mirror() is multi-threaded.  Before java.base 
> is defined to the VM, the fixup_module_list stores all classes that 
> once java.base is defined, must have their module field patched with 
> java.base's java.lang.reflect.Module. Insertion of a class into the 
> fixup_module_list must be protected via Module_lock. Included in this 
> fix is correct order access store and load of the 
> java.lang.reflect.Module field for java.base's ModuleEntry.  Thank you 
> to David Holmes for guidance on this piece.
>
> Testing:
> Full hotspot nightly testing, JCK vm & lang, several iterations of 
> jaxp tests to test JDK-8164721