RFR 8185103: TestThreadDumpMonitorContention.java crashed due to SIGSEGV in G1SATBCardTableModRefBS::write_ref_field_pre_work

[harold seigel]

Thanks Coleen for the review!

Harold


On 8/3/2017 1:40 PM, coleen.phillimore at oracle.com wrote:
>
> This change looks good.  Very minor nit:
>
> + if(k->java_mirror() == NULL) k->set_java_mirror(mirror());
>
>
> Needs a space between the if and (.   Probably should also be:
>
>    if (k->java_mirror() == NULL) {
>       // Only set the mirror if not set above while setting the module
>       k->set_java_mirror();
>   }
>
> ie. with a comment.
>
> I do not need to see a new version.
>
> Thanks,
> Coleen
>
> On 8/3/17 9:03 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this JDK-10 fix for JDK-8185103.  The problem occurred  
>> because classes were being put on the fixup_module_field_list before 
>> their mirror field was set.  If a (different) thread called method 
>> patch_javabase_entries() before the class's mirror field was set then 
>> this would cause a SIGSEGV because patch_javabase_entries() 
>> eventually calls obj_field_put() which tries to dereference the 
>> class's mirror field.
>>
>> This change fixes the problem by setting the class's mirror field 
>> before putting the class on the fixup_module_field_list.
>>
>> Open Webrev: 
>> http://cr.openjdk.java.net/~hseigel/bug_8185103/webrev/index.html
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8185103
>>
>> The fix was tested with the JCK Lang and VM tests, the JTreg hotspot, 
>> java/io, java/lang, java/util and other tests, the co-located NSK 
>> tests, and with JPRT.
>>
>> Additionally, the fix was tested by temporarily adding a 
>> naked_short_sleep(50) to method initialize_mirror_fields() shortly 
>> after it put a class on the fixup_module_field_list. The sleep was 
>> added in order to enhance the likelihood of patch_javabase_entries() 
>> being called before the class's mirror field got set.  Without the 
>> fix, the TestThreadDumpMonitorContent.java test and the test reported 
>> in JDK-8183309 <https://bugs.openjdk.java.net/browse/JDK-8183309> 
>> reliably got the reported SIGSEGVs.  With the fix, the tests passed.
>>
>> Thanks, Harold
>>
>