[jdk19] RFR: 8289091: move oop safety check from SharedRuntime::get_java_tid() to JavaThread::threadObj() [v3]

[Daniel D. Daugherty]

On Tue, 28 Jun 2022 16:55:29 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> A trivial move of the oop safety check from SharedRuntime::get_java_tid() to
>> JavaThread::threadObj(). Also made adjustments to the threadObj() calls in
>> JavaThread::print_on_error() and JavaThread::get_thread_name_string() so
>> that we don't get secondary crashes when a JavaThread crashes after it has
>> detached the GC barrier.
>> 
>> Tested with Mach5 Tier[1-7]. A Mach5 Tier8 will be started this weekend.
>
> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
> 
>   dholmes CR - use Thread::current() instead of Thread::current_or_null().

Just to be clear:
@dholmes-ora wrote above:
> So to me trying to make secondary crash handling more robust in the current case
> is not worth the cost of the extra checks. If the checks were only in the crash
> reporting path then that would be okay, but not when they impact normal code
> execution. Sorry.

The code we're arguing about is in `JavaThread::get_thread_name_string()` and
when you look at the PR with white space changes disabled, you'll see this new
code which is executed in the "normal code" case:

  Thread* current = Thread::current();
  if (!current->is_Java_thread() || JavaThread::cast(current)->is_oop_safe()) {
    // Only access threadObj() if current thread is not a JavaThread
    // or if it is a JavaThread that can safely access oops.


which is one Thread::current() call and one if-statement.

There is also this new code block that's only executed in the error case:

  } else {
    // Current JavaThread has exited...
    if (current == this) {
      // ... and is asking about itself:
      name_str = "<no-name - current JavaThread has exited>";
    } else {
      // ... and it can't safely determine this JavaThread's name so
      // use the default thread name.
      name_str = Thread::name();
    }
  }

so none of the else-block matters in the "normal code" case.

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

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