On Mon, 20 Nov 2023 19:28:13 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

>> Please review JvmtiThreadState::state_for_while_locked change to handle the 
>> state->get_thread_oop() null case. Please see 
>> https://bugs.openjdk.org/browse/JDK-8319935 for details.
>
> Jiangli Zhou has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add a check for a thread is_attaching_via_jni, based on David Holmes' 
> comment.

src/hotspot/share/prims/jvmtiThreadState.inline.hpp line 100:

> 98:     assert(state->get_thread_oop() != nullptr, "incomplete state");
> 99:   }
> 100: #endif

Nit: I would suggest to write this assert in the form:

  // Make sure we don't see an incomplete state. An incomplete state can cause
  // a duplicate JvmtiThreadState being created below and bound to the 'thread'
  // incorrectly, which leads to stale JavaThread* from the JvmtiThreadState
  // after the thread exits.
   assert(state == nullptr || state->get_thread_oop() != nullptr, "incomplete 
state");

The `#ifdef ASSERT` and `#endif` are not needed then.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16642#discussion_r1401332452

Reply via email to