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

[Daniel D. Daugherty]

On 7/31/17 8:35 AM, Aleksey Shipilev wrote:
> On 07/31/2017 04:24 PM, Daniel D. Daugherty wrote:
>> 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/
> Those changes make sense, thanks.

Thanks for the fast review!


> It is probably worth mentioning that Threads::parallel_java_threads_do should be in sync with
> Threads::possibly_parallel_oops_do? It gets easier to point out the symmetry: possibly_parallel_...
> claims all Java threads and the VMThread, so this should also claim the VMThread.

We would have to be careful about how we phrase that.
Threads::possibly_parallel_oops_do() claims and applies
the closure to all the threads it claims.

Threads::parallel_java_threads_do() is missing the claim
for the VMThread (this bug), but does not apply the
closure to the VMThread.

I think we'll be in good shape once
Threads::assert_all_threads_claimed() is updated to make
sure that the VMThread is claimed. Once that happens, anyone
that uses StrongRootsScope to manage the "claim" protocol
will have a sanity check in place.

Dan



>
> -Aleksey
>