RFR: 8288139: JavaThread touches oop after GC barrier is detached

[Daniel D.Daugherty]

On Wed, 15 Jun 2022 22:12:45 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Update SharedRuntime::get_java_tid() to verify that the calling thread is safely
>> accessing its own threadObj(). This check uses the new is_gc_barrier_detached()
>> function added by [JDK-8288497](https://bugs.openjdk.org/browse/JDK-8288497) add support for JavaThread::is_gc_barrier_detached().
>> 
>> The above check was used to reproduce the failure mode without Shenandoah
>> and the remainder of the fix relocates the offending code from
>> ThreadsSMRSupport::remove_thread() to Threads::remove(). The work of
>> removed the 'tid' entry from the ThreadIdTable is still done under the
>> protection of the Threads_lock.
>> 
>> This fix along with the fix for JDK-8288497 has been tested in Mach5 Tier[1-8].
>> There are no related failures in Mach5 Tier[1-7]; Mach5 Tier8 is still running.
>
> src/hotspot/share/runtime/sharedRuntime.cpp line 998:
> 
>> 996: JRT_END
>> 997: 
>> 998: jlong SharedRuntime::get_java_tid(Thread* thread) {
> 
> Additional RFE: can we not declare this to take JavaThread and so avoid the checks and casts below?

I can research that and file a follow up RFE if necessary.

> src/hotspot/share/runtime/thread.cpp line 3603:
> 
>> 3601:       ThreadIdTable::remove_thread(tid);
>> 3602:     }
>> 3603: 
> 
> I don't like exposing `ThreadSMR` internal details like this. Can we at least make this a little `ThreadSMRSupport` method that we call from here.
> 
> Or ... can we just move `ThreadsSMRSupport::remove_thread(p);` to before `on_thread_detach`?
> 
> Or can we grab the pid before `on_thread_detach` and pass it to `ThreadsSMRSupport::remove_thread`?

Ummmm... The code that I moved is not a TheadSMR internal detail. That code
is part of the M&M support that was added by Daniil in:

JDK-8185005 Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)
https://bugs.openjdk.org/browse/JDK-8185005

I don't remember why Daniil put that code in ThreadsSMRSupport::remove_thread(p),
but it is just as appropriate to put it in Threads::remove(). That M&M cleanup code
is part of the cleanup necessary when a JavaThread is removed. The important part
is to call that code after the Threads_lock is grabbed.

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

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