On Tue, 21 Nov 2023 23:32:13 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> 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. Changed as suggested. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16642#discussion_r1402771379
