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

2023-12-04 Thread David Holmes
On Mon, 4 Dec 2023 18:16:52 GMT, Daniel D. Daugherty  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.
>
> Daniel D. Daugherty has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   dholmes CR - adjust a comment.

We would need to examine all uses of 
JvmtiVMObjectAllocEventCollector::JvmtiVMObjectAllocEventCollector() to see if 
they can occur in the context of an attaching thread prior to allocation of the 
thread object. I suspect not because we allocate immediately after creating the 
JavaThread. The issue we hit here was events triggered by the execution of the 
Java code for the Thread constructor. It may be that we are just lucky that the 
same Java code doesn't lead to the VMObjectAlloc event though. ???

-

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


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

2023-12-04 Thread Jiangli Zhou
On Mon, 4 Dec 2023 20:52:46 GMT, Daniel D. Daugherty  wrote:

> 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.

Thumbs up, thanks!

-

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


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

2023-12-04 Thread Jiangli Zhou
On Mon, 4 Dec 2023 20:52:46 GMT, Daniel D. Daugherty  wrote:

>>> @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.

> > @dcubed-ojdk and @dholmes-ora, is there a case
> > JvmtiVMObjectAllocEventCollector::JvmtiVMObjectAllocEventCollector()
> > might also see an attaching thread without the thread obj allocated?
> 
> JvmtiVMObjectAllocEventCollector is the code path that we ran into with the 
> internal stress test, but in the case that tripped, we had a thread obj 
> allocated. If we ran the same code path without the thread obj allocated, 
> then we would return `nullptr` which was the goal of your original fix.
> 
> I don't know if that specific test case is actually reached by any of our 
> tests, but if such a case occurred, it did not result in the guarantee() 
> firing or any other failure mode that I saw. The `guarantee(state != nullptr) 
> failed: exiting thread called setup_jvmti_thread_state` has not been seen in 
> Mach5 Tier[1-8] testing with this fix in place and I didn't see any other 
> test failures that could be tracked back to problems with this code.
> 
> A JvmtiVMObjectAllocEventCollector object is created in 34 places in jvm.cpp 
> alone and I haven't checked all of those. I only checked out the one use in 
> JVM_GetStackAccessControlContext.

Thanks. I was wondering if we would need further work to handle 
`JvmtiVMObjectAllocEventCollector::JvmtiVMObjectAllocEventCollector()` with 
something similar to the JvmtiSampledObjectAllocEventCollector change, if it's 
possible to trigger the `guarantee(state != nullptr)` for attaching thread with 
no allocated thread obj.

-

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


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

2023-12-04 Thread Daniel D . Daugherty
On Mon, 4 Dec 2023 19:01:05 GMT, Jiangli Zhou  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


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

2023-12-04 Thread Daniel D . Daugherty
On Mon, 4 Dec 2023 18:13:50 GMT, Daniel D. Daugherty  wrote:

>> Initially I thought this was not the right fix as we should not be exposing 
>> an attaching thread that may still have a partially constructed `threadObj`. 
>> But this issue shows that we must expose such a thread because the 
>> constructor of the `Thread` object can trigger these events on the current 
>> thread so it must have a valid JVMTI state!
>> 
>> Thanks.
>
> @dholmes-ora - Thanks for the review.

> @dcubed-ojdk and @dholmes-ora, is there a case
> JvmtiVMObjectAllocEventCollector::JvmtiVMObjectAllocEventCollector()
> might also see an attaching thread without the thread obj allocated?

JvmtiVMObjectAllocEventCollector is the code path that we ran into with
the internal stress test, but in the case that tripped, we had a thread obj
allocated. If we ran the same code path without the thread obj allocated,
then we would return `nullptr` which was the goal of your original fix.

I don't know if that specific test case is actually reached by any of our tests,
but if such a case occurred, it did not result in the guarantee() firing or any
other failure mode that I saw. The
`guarantee(state != nullptr) failed: exiting thread called 
setup_jvmti_thread_state`
has not been seen in Mach5 Tier[1-8] testing with this fix in place and I didn't
see any other test failures that could be tracked back to problems with this
code.

A JvmtiVMObjectAllocEventCollector object is created in 34 places in jvm.cpp
alone and I haven't checked all of those. I only checked out the one use in
JVM_GetStackAccessControlContext.

-

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


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

2023-12-04 Thread Serguei Spitsyn
On Mon, 4 Dec 2023 18:16:52 GMT, Daniel D. Daugherty  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.
>
> Daniel D. Daugherty has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   dholmes CR - adjust a comment.

Looks good. Thank you for fixing this!

-

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16934#pullrequestreview-1763277330


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

2023-12-04 Thread Jiangli Zhou
On Mon, 4 Dec 2023 18:16:52 GMT, Daniel D. Daugherty  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.
>
> Daniel D. Daugherty has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   dholmes CR - adjust a comment.

Marked as reviewed by jiangli (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16934#pullrequestreview-1763177973


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

2023-12-04 Thread Jiangli Zhou
On Mon, 4 Dec 2023 05:16:59 GMT, Jiangli Zhou  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


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

2023-12-04 Thread Daniel D . Daugherty
On Mon, 4 Dec 2023 06:31:43 GMT, David Holmes  wrote:

>> Daniel D. Daugherty has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   dholmes CR - adjust a comment.
>
> Initially I thought this was not the right fix as we should not be exposing 
> an attaching thread that may still have a partially constructed `threadObj`. 
> But this issue shows that we must expose such a thread because the 
> constructor of the `Thread` object can trigger these events on the current 
> thread so it must have a valid JVMTI state!
> 
> Thanks.

@dholmes-ora - Thanks for the review.

> src/hotspot/share/prims/jvmtiThreadState.inline.hpp line 90:
> 
>> 88: // Don't add a JvmtiThreadState to a thread that is exiting or is 
>> attaching.
>> 89: // When a thread is attaching, it may not have a Java level thread 
>> object
>> 90: // created yet.
> 
> The comment needs adjusting now - suggestion:
> 
> // Don't add a JvmtiThreadState to a thread that is exiting, or is attaching
> // and does not yet have a Java level thread object allocated.

Thanks for the suggested change. I agree and I've applied it.

-

PR Comment: https://git.openjdk.org/jdk/pull/16934#issuecomment-1839205636
PR Review Comment: https://git.openjdk.org/jdk/pull/16934#discussion_r1414306493


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

2023-12-04 Thread Daniel D . Daugherty
On Mon, 4 Dec 2023 05:16:59 GMT, Jiangli Zhou  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.

-

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


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

2023-12-04 Thread Daniel D . Daugherty
> 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.

Daniel D. Daugherty has updated the pull request incrementally with one 
additional commit since the last revision:

  dholmes CR - adjust a comment.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16934/files
  - new: https://git.openjdk.org/jdk/pull/16934/files/5c9eb2ec..0de1484f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16934=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=16934=00-01

  Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/16934.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16934/head:pull/16934

PR: https://git.openjdk.org/jdk/pull/16934


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

2023-12-03 Thread David Holmes
On Sat, 2 Dec 2023 17:15:57 GMT, Daniel D. Daugherty  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.

Initially I thought this was not the right fix as we should not be exposing an 
attaching thread that may still have a partially constructed `threadObj`. But 
this issue shows that we must expose such a thread because the constructor of 
the `Thread` object can trigger these events on the current thread so it must 
have a valid JVMTI state!

Thanks.

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

> 88: // Don't add a JvmtiThreadState to a thread that is exiting or is 
> attaching.
> 89: // When a thread is attaching, it may not have a Java level thread 
> object
> 90: // created yet.

The comment needs adjusting now - suggestion:

// Don't add a JvmtiThreadState to a thread that is exiting, or is attaching
// and does not yet have a Java level thread object allocated.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16934#pullrequestreview-1761678171
PR Review Comment: https://git.openjdk.org/jdk/pull/16934#discussion_r1413421607


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

2023-12-03 Thread Jiangli Zhou
On Sat, 2 Dec 2023 17:15:57 GMT, Daniel D. Daugherty  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.

-

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


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

2023-12-02 Thread Daniel D . Daugherty
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.

-

Commit messages:
 - 8321069: Test failure after JDK-8319935: exiting thread called 
setup_jvmti_thread_state

Changes: https://git.openjdk.org/jdk/pull/16934/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=16934=00
  Issue: https://bugs.openjdk.org/browse/JDK-8321069
  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16934.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16934/head:pull/16934

PR: https://git.openjdk.org/jdk/pull/16934