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

[David Holmes]

Hi Harold,

On 3/08/2017 11:03 PM, 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 

The code that calls patch_javabase_entries has this:

  // Only the thread that actually defined the base module will get here,
  // so no locking is needed.

   // Patch any previously loaded class's module field with java.base's 
java.lang.Module.
   ModuleEntryTable::patch_javabase_entries(module_handle);

so it seems that comment is wrong and that locking is indeed needed 
somewhere! At a minimum your setting of the mirror needs a following 
storestore barrier, or (better) the set/get of the mirror uses 
load-acquire/store-release.

Thanks,
David
-----

> 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
>