On Mon, 4 Dec 2023 19:01:05 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:
>> @dcubed-ojdk Thanks for the notification. I just ran one of our affected >> test 100 times with JDK-8312174 change rolled back and with both following >> applied: >> >> - https://git.openjdk.org/jdk/pull/16642 >> - https://github.com/openjdk/jdk/pull/16934 >> >> All 100 runs passed without failure. I'm going to run all tests tonight, >> will report back later tomorrow if I see any issue. > >> @jianglizhou - Thanks for doing the testing. Can you also do a review? We >> need two reviewers for this change. > > Complete test run finished. Checking the results, looks like there's no issue > related to JVMTIThreadState change. > > @dcubed-ojdk Reducing the check to `(thread->threadObj() == nullptr && > thread->is_attaching_via_jni())` looks ok. > > I just rechecked all usages of setup_jvmti_thread_state(). Currently it's > used in three cases: > - JvmtiDynamicCodeEventCollector::JvmtiDynamicCodeEventCollector() > - JvmtiVMObjectAllocEventCollector::JvmtiVMObjectAllocEventCollector() > - JvmtiSampledObjectAllocEventCollector::start() > > JDK-8319935 ran into issue with > JvmtiSampledObjectAllocEventCollector::start() call path. We changed > JvmtiSampledObjectAllocEventCollector::object_alloc_is_safe_to_sample() to > avoid sampling if there is no thread obj allocated for the attaching thread. > We also changed JvmtiThreadState::state_for_while_locked to handle the > attaching case and return null. @dcubed-ojdk and @dholmes-ora, is there a > case JvmtiVMObjectAllocEventCollector::JvmtiVMObjectAllocEventCollector() > might also see an attaching thread without the thread obj allocated? > > JvmtiDynamicCodeEventCollector::JvmtiDynamicCodeEventCollector() call path > probably is not affected by this case. @jianglizhou and @sspitsyn - Thanks for the reviews. In the interest of reducing the noise in the Mach5 CI, I'm going ahead with integrating this fix without waiting for a reply to my comment above. If there are remaining issues, then we'll deal with them in a follow-up bug/RFE. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16934#issuecomment-1839457963
