RFR: JDK-8220671: Initialization race for non-JavaThread PtrQueues

Roman Kennke rkennke at redhat.com
Tue Mar 26 11:30:02 UTC 2019


>>>> This thread went a bit off. May I propose this for review:
>>>>
>>>> http://cr.openjdk.java.net/~rkennke/JDK-8220671/webrev.07/
>>>>
>>>> it passes tier1 tests locally, and I submitted it to jdk/submit but 
>>>> that seems to have other hiccups.
>>>>
>>>> WDYT?
>>>
>>> I'm still not convinced the activation predicates for the STS joiners
>>> are correct here.  And I'm not what the right predicate might be or if
>>> it even exists.
>>>
>>> Also, STS is presently a "GC" thing, and this would insinuate it into
>>> non-GC code, and affect non-GC threads (such as the WatcherThread and
>>> the JfrThreadSampler).
>>>
>>> I still prefer using the NonJavaThreadsList_lock in NJT::pre/post_run
>>> and around the the relevant state changes, such as setting the SATB
>>> qset's _all_active member.
>>
>> Ok, let's go with your patch:
>>
>> http://cr.openjdk.java.net/~kbarrett/8220671/use_njtlock.00/
> 
> Looks good to me. I also think this is the better approach, as I would 
> also feel uncomfortable with exposing the STS outside of GC.
> 
>>
>> I've tested it against my stress test and Shenandoah test suite, and 
>> all seems fine.
>>
>> Is the extra locking around assignment of global state needed? I think 
>> the locking in threads_do() is sufficient?
> 
> The locking over "_all_active = active;" looks sane to me. This patch 
> has no locking in threads_do(), or am I missing something?

Oh, somehow I assumed that non_java_threads_do() would be under the 
lock. I think it probably should...

Roman



More information about the hotspot-gc-dev mailing list