URGENT RFR (S): fix for Test8004741.java crashes with SIGSEGV in JDK10-hs nightly (8185273)

Daniel D. Daugherty daniel.daugherty at oracle.com
Mon Jul 31 16:42:53 UTC 2017


On 7/31/17 9:14 AM, Daniel D. Daugherty wrote:
> On 7/31/17 8:42 AM, Roman Kennke wrote:
>> Hi Dan,
>>
>> You could also do_thread() on the VMThread, and let the ThreadClosurer
>> filter it. I believe the ThreadClosure in safepoint.cpp (currently only
>> consumer) already filters it. This would make it consistent with
>> Threads::possibly_parallel_oops_do() (and infact, that latter method
>> could just use the new Threads::parallel_java_threads_do() but this is
>> beyond the scope). I leave that to you to decide though.
>
> I'm good with just adding the missing part of the "claims" protocol.
> I'm not comfortable with applying the closure to the VMThread since
> I'm just visiting the GC sandbox as it were... :-)
>
>> I'd also include the fix for assert_all_threads_claimed() because it's
>> related (and the cause for me not noticing this slip). But that is up to
>> you too. ;-)
>
> Yes, I plan to kick off another JPRT run with the additional fix for
> assert_all_threads_claimed()... If that goes well, then I'll include
> it...

Here's the addition of the assert:

$ diff -C 6 src/share/vm/runtime/thread.cpp.cr0 
src/share/vm/runtime/thread.cpp
*** src/share/vm/runtime/thread.cpp.cr0 Sun Jul 30 18:49:06 2017
--- src/share/vm/runtime/thread.cpp     Mon Jul 31 08:22:47 2017
***************
*** 4360,4371 ****
--- 4360,4375 ----
   void Threads::assert_all_threads_claimed() {
     ALL_JAVA_THREADS(p) {
       const int thread_parity = p->oops_do_parity();
       assert((thread_parity == _thread_claim_parity),
              "Thread " PTR_FORMAT " has incorrect parity %d != %d", 
p2i(p), thread_parity, _thread_claim_parity);
     }
+   VMThread* vmt = VMThread::vm_thread();
+   const int thread_parity = vmt->oops_do_parity();
+   assert((thread_parity == _thread_claim_parity),
+          "VMThread " PTR_FORMAT " has incorrect parity %d != %d", 
p2i(vmt), thread_parity, _thread_claim_parity);
   }
   #endif // ASSERT

   void Threads::possibly_parallel_oops_do(bool is_par, OopClosure* f, 
CodeBlobClosure* cf) {
     int cp = Threads::thread_claim_parity();
     ALL_JAVA_THREADS(p) {


I ran a test JPRT job and there were no problems.

Aleksey and Roman, are you two good with the assert?


Dan


>
>>
>> In other words, thumbs up, unless you want to add the above points.
>
> Thanks for the review!
>
>
>> And sorry for making such a mess!
>
> No worries. We have it covered.
>
> Dan
>
> P.S.
> Reminder: you're supposed to be on vacation! (But I do appreciate
> you taking the time to chime in here...)
>
>
>>
>> Roman
>>
>>> Greetings,
>>>
>>> I have a fix for the following P1 JDK10-hs integration_blocker bug:
>>>
>>>      8185273 Test8004741.java crashes with SIGSEGV in JDK10-hs nightly
>>>      https://bugs.openjdk.java.net/browse/JDK-8185273
>>>
>>> The fix is 2 lines and the comment describing the fix is 4 lines:
>>>
>>> src/share/vm/runtime/thread.cpp:
>>>
>>> L3388: void Threads::parallel_java_threads_do(ThreadClosure* tc) {
>>> <snip>
>>> L3395:   // This function is used by ParallelSPCleanupTask in
>>> safepoint.cpp
>>> L3396:   // for cleaning up JavaThreads, but we have to keep the
>>> VMThread's
>>> L3397:   // _oops_do_parity field in sync so we don't miss a parallel
>>> GC on
>>> L3398:   // the VMThread.
>>> L3399:   VMThread* vmt = VMThread::vm_thread();
>>> L3400:   (void)vmt->claim_oops_do(true, cp);
>>>
>>> I'm also including some new logging for the VMThread (tag == 
>>> 'vmthread')
>>> that came in useful during this bug hunt. Lastly, I've fixed a few 
>>> minor
>>> typos that I ran across in the areas where I was hunting.
>>>
>>> Webrev URL: http://cr.openjdk.java.net/~dcubed/8185273-webrev/0/
>>>
>>> There's lots of discussion in the bug. The evaluation comment that I
>>> added
>>> on Sunday, July 30 is probably the most complete and hopefully the
>>> most clear.
>>>
>>> For context, here's the webrev for 8180932 and another bug fix:
>>>
>>> http://cr.openjdk.java.net/~rkennke/8180932/webrev.18/
>>> http://cr.openjdk.java.net/~rkennke/8185102/webrev.01/
>>>
>>>
>>> Testing:
>>>    - JPRT
>>>    - Test8004741.java has been running in a forever loop with 
>>> 'fastdebug'
>>>      bits (17200+ iterations) and 'slowdebug' bits (13400+ iterations)
>>>
>>> Comments, questions and feedback are welcome.
>>>
>>> Dan
>>>
>>> P.S.
>>> Roman and I were also thinking about updating
>>> Threads::assert_all_threads_claimed() to verify
>>> that the VMThread is also claimed... Obviously
>>> that's not part of the current patch...
>>
>
>




More information about the hotspot-gc-dev mailing list