RFR(S) 8166364: fatal error: acquiring lock DirtyCardQ_CBL_mon/16 out of order with lock Module_lock/6 -- possible deadlock

[harold seigel]

Hi David,

Thanks for the review.  I'll fix the comments before pushing the fix.

Harold


On 10/9/2016 4:59 PM, David Holmes wrote:
> Hi Harold,
>
> Change looks good. A couple of suggestions re comments below.
>
> On 8/10/2016 1:20 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this fix for JDK-8166364.
>>
>> This fix moves the setting of the module fields in the class mirrors of
>> the fixup_module_list outside of the Module_lock.  The determination of
>> whether a mirror should be added to the fixup_module_list is still done
>> under Module_lock as is the defining of module java.base.  This prevents
>> any synchronization issues with a mirror being erroneously added to the
>> fixup_module_list after module java.base is defined.  The other piece is
>> that the VM, in Modules::define_javabase_module(), guarantees under
>> Module_lock that only one thread will ever successfully define module
>> java.base.
>>
>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8166364/
>
> src/share/vm/classfile/modules.cpp
>
> Can you add a comment here:
>
>  246
> +      // Only the thread that actually defined the base module will 
> get here,
> +      // so no locking is needed.
> +
>  247   // Patch any previously loaded class's module field with 
> java.base's java.lang.reflect.Module.
>  248   ModuleEntryTable::patch_javabase_entries(module_handle);
>
> ---
>
> src/share/vm/classfile/javaClasses.cpp
>
> This comment is no longer quite reads right now it is not the else 
> clause:
>
>  801     // java.base was defined at some point between calling 
> create_mirror()
>  802     // and obtaining the Module_lock, patch this particular class 
> with java.base.
>
> suggest:
>
> // If java.base was already defined then patch this particular class 
> with java.base.
>
>
> Thanks,
> David
>
>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8166364
>>
>> The fix was tested with the JCK Lang and vm tests, and JTreg hotspot,
>> java/io, java/lang, and java/util tests using both fastdebug and
>> slowdebug builds.  The nsk cololocated and the non-colocated quick tests
>> were also run against a slowdebug build.
>>
>> Thanks, Harold
>>