RFR (M) JDK-8163406: The fixup_module_list must be protected by Module_lock when inserting new entries
coleen.phillimore at oracle.com
Sat Sep 10 13:13:45 UTC 2016
On 9/9/16 6:29 PM, Lois Foltan wrote:
> On 9/8/2016 7:15 PM, Coleen Phillimore wrote:
>> + 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.
> Hi Coleen,
> The only time within java_lang_Class::create_mirror() a module should
> be NULL is during pre module system initialization, so it is important
> to make this assertion. How is this any different than checking for
>> 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!
> Will do.
>> Can you add a comment before
>> to make it clear that there's a module field in java.lang.Class
> Will do.
>> 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).
> Ok, this is very similar to the code that is used in
> Universe::fixup_mirrors which I suspect this code was copied from.
> Would you like me to change fixup_mirrors as well?
No need. It'll get cleaned up soon. I just wanted new code to not use
>> I think everything except these small comments looks good.
> Thanks for the review!
>> On 9/7/16 11:31 AM, Lois Foltan wrote:
>>> Please review the following fix:
>>> Bug: The fixup_module_list must be protected by Module_lock when
>>> inserting new entries
>>> 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.
>>> Full hotspot nightly testing, JCK vm & lang, several iterations of
>>> jaxp tests to test JDK-8164721
More information about the hotspot-runtime-dev