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

[Lois Foltan]

On 9/9/2016 5:06 PM, Zhengyu Gu wrote:
> Hi Lois,
>
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8163406/webrev/src/share/vm/classfile/javaClasses.cpp.sdiff.html
>   838     if (module.is_null()) {
>   839       // During startup, the module may be NULL only if java.base has not been defined yet.
>   840       // Put the class on the fixup_module_list to patch later when the java.lang.reflect.Module
>   841       // for java.base is known.
>   842       assert(!Universe::is_module_initialized(), "Incorrect java.lang.reflect.Module pre module system initialization");
>   843       MutexLocker m1(Module_lock, THREAD);
>   844       // Keep list of classes needing java.base module fixup
>
> Line #842, _module_initialized is set to true just before returning from call_initPhase2() from main thread, so this scenario
> can only happen before returning from call_initPhase2().

Hi Zhengyu,
Yes, that is correct.  More specifically this scenario should only 
happen prior to the module java.base being defined to the VM.

>   It seems to me that, at this point, this main thread is the only thread
> doing any class loading. Do I miss anything here?
Actually, there are multiple threads loading classes before the module 
system is initialized.

>
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8163406/webrev/src/share/vm/classfile/moduleEntry.hpp.sdiff.html
>   229   static bool javabase_defined() {
>   230     ModuleEntry* jb = javabase_moduleEntry();
>   231     return ((jb != NULL) && (jb->module_load_acquire() != NULL));
>   232   }I don't think load-acquire is needed here.

We discussed this internally and it was decided that we do need a 
load-acquire here.

Thanks for the review,
Lois

>
> Thanks,
>
> -Zhengyu
>
> On 09/07/2016 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