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

[Kim Barrett]

> On Mar 19, 2019, at 5:51 PM, Roman Kennke <rkennke at redhat.com> wrote:
> 
> See:
> https://bugs.openjdk.java.net/browse/JDK-8221102
> 
> I believe G1 is also affected by this problem, as it seems to flush SATB queues in pretty much the same way as Shenandoah:
> 
> http://hg.openjdk.java.net/jdk/jdk/file/ddfb658c8ce3/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp#l1776
> 
> Please check it, and update as appropriate. I'm working on a fix.
> 
> Roman

I did some looking at the claiming mechanism, and I agree there's an
inconsistency of usage that can mess things up.

We have possibly_parallel_threads_do, which only looks at Java threads
and the VM thread.  Other threads don't get examined, and so don't
have their claim state updated.

We also have callers of Threads::threads_do where the closure's
do_thread conditionalizes its operation on a call to claim_oops_do.
So all threads are examined and their claim state is updated.

It seems like the right (or should I say wrong) sequence of operations
could cause an NJT being visited by Threads::threads_do to appear to
have already been claimed from the get go, and so will be skipped by
all of the parallel claiming threads.

Sprinkling more assert_all_threads_claimed won't currently help detect
this, since it too only examines Java threads and the VM thread.

It looks like an even number of uses of possibly_parallel_threads_do
between a pair of Threads::threads_do with explicit claims will
confuse the second Threads::threads_do.

And of course, there's the problem that Shenandoah is using
possibly_parallel_threads_do in a place where skipping the
StringDedupThread is bad.

Yikes!  How is this ever working at all?

I'm not sure why possibly_parallel_threads_do applies to a subset of
the threads.  One solution would be to change that.  But there are
users of it that maybe don't expect other kinds of threads and might
not want to pay extra to examine and ignore them.  Another would be
to have two different claim fields for the two different iterators.
They could be packed into the same thread member at some small cost,
though looking at where the current member is located, adding a second
member there should not cost any space in 64bit platforms.