RFR: 8307068: store a JavaThread* in the java.lang.Thread object after the JavaThread* is added to the main ThreadsList [v2]

[Daniel D. Daugherty]

On Tue, 2 May 2023 08:54:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Daniel D. Daugherty has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase.
>
> src/hotspot/share/runtime/javaThread.cpp line 1675:
> 
>> 1673:   // Publish the JavaThread* in java.lang.Thread after the JavaThread* is
>> 1674:   // on a ThreadsList. There will be a release when the Theads_lock is
>> 1675:   // dropped somewhere in the caller, but let's be more proactive.
> 
> Any release associated with dropping the Threads_lock may come too late given this thread is now visible via JVMTI, so this is more than just being "proactive" IMO.

Okay. I've updated the comment to:

  // Publish the JavaThread* in java.lang.Thread after the JavaThread* is
  // on a ThreadsList. We don't want to wait for the release when the
  // Theads_lock is dropped somewhere in the caller since the JavaThread*
  // is already visible to JVM/TI via the ThreadsList.

> src/hotspot/share/runtime/javaThread.cpp line 2113:
> 
>> 2111:   // Publish the JavaThread* in java.lang.Thread after the JavaThread* is
>> 2112:   // on a ThreadsList.
>> 2113:   java_lang_Thread::set_thread(thread_oop(), target); // isAlive == true now
> 
> Why did you decide this doesn't have to be a release?

Because the `Threads_lock` is grabbed in this function and released again
when the `mu` destructor runs, but I'm going to change my mind.  I've changed
the comment and code to:

  // Publish the JavaThread* in java.lang.Thread after the JavaThread* is
  // on a ThreadsList. We don't want to wait for the release when the
  // Theads_lock is dropped when the 'mu' destructor is run since the
  // JavaThread* is already visible to JVM/TI via the ThreadsList.
  java_lang_Thread::release_set_thread(thread_oop(), target); // isAlive == true now

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13723#discussion_r1183030407
PR Review Comment: https://git.openjdk.org/jdk/pull/13723#discussion_r1183034248