RFR: 8288497: add support for JavaThread::is_gc_barrier_detached()

[David Holmes]

On Wed, 15 Jun 2022 15:44:21 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

> A trivial fix to add support for JavaThread::is_gc_barrier_detached() which allows
> us to add checks to detect failures like:
> 
>     JDK-8288139 JavaThread touches oop after GC barrier is detached
>     https://bugs.openjdk.org/browse/JDK-8288139
> 
> This fix along with the fix for JDK-8288139 has been tested in Mach5 Tier[1-8].
> There are no related failures in Mach5 Tier[1-7]; Mach5 Tier8 is still running.

Hi Dan,

After looking at the barrier code I'm warming up to the proposal you have made, but with a couple of suggestions below.

Thanks.

src/hotspot/share/runtime/thread.cpp line 3601:

> 3599:     // requiring the GC state to be alive.
> 3600:     BarrierSet::barrier_set()->on_thread_detach(p);
> 3601:     if (p->is_exiting()) {

I don't understand why this needs to be conditional?

src/hotspot/share/runtime/thread.hpp line 1180:

> 1178:   bool is_exiting() const;
> 1179:   // thread's GC barrier is detached or thread is terminated
> 1180:   bool is_gc_barrier_detached() const;

Can we name this based on what it allows (access to oops) rather than what it relates to deep down? e.g. `can_access_oops_safely` or `is_oop_safe` ?

src/hotspot/share/runtime/thread.hpp line 1186:

> 1184:   bool check_is_terminated(TerminatedTypes l_terminated) const {
> 1185:     return l_terminated != _not_terminated && l_terminated != _thread_exiting &&
> 1186:            l_terminated != _thread_gc_barrier_detached;

Existing: I don't think I buy this argument that we need to check all the states we don't want to see rather than the one we do. If the JavaThread has been freed we could see anything - and we should never be executing this code in the first place.

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

PR: https://git.openjdk.org/jdk19/pull/20