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

Roman Kennke rkennke at redhat.com
Tue Mar 19 20:24:44 UTC 2019

>>>>>> The problem we're trying to solve is that (some implementations of)
>>>>>> on_thread_attach needs to be atomic with the associated list addition
>>>>>> wrto safepoints, or more precisely, with an operation performed within
>>>>>> some safepoints.  on_thread_attach is caching global state in the
>>>>>> thread local data, while the operation is updating the global state
>>>>>> and needs to also update the thread local caches.  For G1 and
>>>>>> Shenandoah, it's the SATB activation state; for ZGC it's the bad bit
>>>>>> mask.
>>> Yes, it is not strictly the safepoint that is the issue. But as you say, there's state that we're reading concurrently that is otherwise written at safepoints (the SATB active flag), and not protected otherwise (e.g. by some lock that we could take when reading). There's also global flags and state updated at safepoints, and I am not sure we're good with that either if we don't let the non-Java-thread synchronize with safepoints on attach.
>>> Therefore I think that the way to go is to let such threads synchronize with safepoints on on_thread_attach(), and via STS. A Shenandoah-only solution that seems to do it:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8220671/webrev.06/
>>> Compared to my other proposals it has the advantage that it's only changing GC specific code, and not with any global/shared code. It will not introduce STS into paths that don't synchronize this anyway (e.g. CMS).
>>> Unless we come up with something even better, this would be the way to go for me. If you agree, I can do the G1 change in the same fashion.
>>> Roman
>> As I said over in the JDK-8221086 thread, I don’t think that works.
>> It is making on_thread_attach atomic wrto safepoints, but doesn’t
>> include the NJT list modifications.
> The problem race, in pseudo-code:
> // thread creation
> STS.join
>   attach(this_thread)
>     this_thread.SATB.active = old_state
> STS.leave
> // Asynchronous (wrto this_thread) safepoint might not update
> // this_thread's SATB.active state because this_thread is not
> // yet visible to the threads_do() performing the update.
> lock NJTList 
>   this_thread._next = NJTList._head
>   release_store(NJTList._head, this_thread)
> unlock NJTList 

yes, thanks. That is helpful.

What seems strange is that this patch:

is failing with my testcase. Am I doing something wrong there? (When I
tried with webrev.06 was not failing, but this was probably just lucky?
The test is not 100% reliable)

Or maybe I'm thinking in wrong direction altogether.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20190319/6dcf51ef/signature.asc>

More information about the hotspot-gc-dev mailing list