RFR: 8240774: [REDO] G1DirtyCardQueue destructor has useless flush

Kim Barrett kbarrett at openjdk.org
Wed May 24 10:32:01 UTC 2023


Please review this change to VM initialization, to eliminate a workaround for
a bootstrapping problem.

A JavaThread must not touch oops and invoke barriers unless it is on the
threads list.  Barrier set operations on_thread_attach and on_thread_detach
bracket the time when a thread can do stuff with oops.

However, this restriction is currently violated by the main thread during VM
initialization.  It may (and indeed does) touch oops before it has been added
to the thread list, including executing post-barriers and enqueuing entries
in its DCQ.

That has led to two separate bootstrapping workarounds:

- ~G1DirtyCardQueue flushes the queue before proceeding to the base class
(PtrQueue) destructor. This is not normally needed, and is even
counter-productive, since it may hide a problem. But if an error is detected
during VM initialization that main thread gets deleted. Since it's not on the
threads list, there's no on_thread_detach to flush the queue. Without the
flush in the DCQ destructor that can lead to an assert in ~PtrQueue because
the queue isn't empty.

- on_thread_attach can't assert the queue is empty, even though normally it
must be.  But for the main thread, being added to the threads list (so calling
on_thread_attach) happens after it has enqueued entries in its DCQ.  So we
currently don't have those asserts.

To remove the need for these workarounds, this change moves the addition of
the main thread to the threads list earlier in the initialization sequence. It
needs to be before code that might execute barriers. After init_globals is too
late. But it can't be too early, before some relevant data structures are
initialized, so it can't be before init_globals. So this change splits
init_globals at a strategic point, and adds the main thread to the threads
list there. That point is just before the call to universe2_init, which calls
Universe::genesis.

With that change we can then remove the workarounds in ~G1DirtyCardQueue and
G1BarrierSet::on_thread_attach.

Testing:
mach5 tier1-5

Manually tested error handling for failures reported by init_globals and
init_globals2. This involved patching to force error reporting when
-XX:+UseNewCode. Handling errors from init_globals works as before, if a
failure happened in that part of the sequence. Handling errors from
init_globals2 runs afoul of pre-existing JDK-8308764.

-------------

Commit messages:
 - remove workarounds
 - add main thread to threads list earlier

Changes: https://git.openjdk.org/jdk/pull/14119/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14119&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8240774
  Stats: 30 lines in 5 files changed: 20 ins; 5 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/14119.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14119/head:pull/14119

PR: https://git.openjdk.org/jdk/pull/14119


More information about the hotspot-dev mailing list