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

[Per Liden]

Hi,

On 3/26/19 11:43 AM, Roman Kennke wrote:
>>> 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?

cheers,
Per

> 
> Roman