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

[Kim Barrett]

> On Mar 19, 2019, at 6:50 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
>> 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.

And it looks like this inconsistency of usage was introduced by me with JDK-8219613.