RFR: 8321069: JvmtiThreadState::state_for_while_locked() returns nullptr for an attached JNI thread with a java.lang.Thread object after JDK-8319935

[Jiangli Zhou]

On Mon, 4 Dec 2023 05:16:59 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>> In the fix for the following bug:
>> 
>> [JDK-8319935](https://bugs.openjdk.org/browse/JDK-8319935) Ensure only one JvmtiThreadState is created for one JavaThread associated with attached native thread
>> 
>> JvmtiThreadState::state_for_while_locked() was changed to
>> return nullptr for attaching JNI threads regardless of whether
>> that JNI thread/JavaThread had a java.lang.Thread object.
>> 
>> We should only filter out a JavaThread that's attaching via JNI
>> if it has no java.lang.Thread object.
>> 
>> This fix has been tested with Mach5 Tier[1-7] and there are no related test failures.
>> Mach5 Tier8 is in process.
>> 
>> I'm going to need @jianglizhou to rerun her testing for:
>> 
>> [JDK-8319935](https://bugs.openjdk.org/browse/JDK-8319935) Ensure only one JvmtiThreadState is created for one JavaThread associated with attached native thread
>> 
>> since the test(s) for that fix are not yet integrated in the jdk/jdk repo.
>
> @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.

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

PR Comment: https://git.openjdk.org/jdk/pull/16934#issuecomment-1839282331