RFR: 8319935: Ensure only one JvmtiThreadState is created for one JavaThread associated with attached native thread [v3]

[Serguei Spitsyn]

On Tue, 28 Nov 2023 02:51:54 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>>> Thank you for filing and fixing this issue! I'm kind of late here. Sorry for that. Is it hard to create a JTreg test for an attaching native thread? I can help if you have a standalone prototype. You can look for some examples in the folder: `test/hotspot/jtreg/serviceability/jvmti/vthread`.
>> 
>> Hi @sspitsyn we don't have an extracted standalone test case (yet) to demonstrate the crashes. The crashes could not reproduce consistently. Outside the debugger (lldb), I ran the test (one of the affected ones) 10 times/per-iteration in order to reproduce. I found the crashes could be affected by both timing and memory layout. During the investigation, I noticed the problem became hidden when I increased allocation size for ThreadsList::_threads (as one of the experiments that I did, I wanted to mprotect the memory to be read-only in order to find who trashed the memory, so was trying to allocate memory up to page boundary). That's the reason why I added noreg-hard tag earlier.
>> 
>> I gave some more thoughts today. Perhaps, we could write a whitebox test to check the JvmtiThreadState, without being able to consistently trigger crashes. We could add a WhiteBox api to iterate the JvmtiThreadState list and validate if all the JavaThread pointers were valid after detaching. The test would need to create native threads to attach and detach before the check. That could more reliably test the 1-1 mapping of JvmtiThreadState and JavaThread. What do you think? 
>> 
>> Thanks for volunteering to help with the test. I created https://bugs.openjdk.org/browse/JDK-8320614 today. Should I assign it to you?
>
> @jianglizhou Thank you for filing the sub-task. You have already seen some crashes. Even though you do not have a standalone test case, it is still valuable if you describe a test scenario (at least, surfacely) which helped to observe the problem. Could you, add it to the sub-task report, please?

> Thanks for the careful review, @sspitsyn! The fix is for 22. We probably should also consider back-porting to JDK 11 to prevent any potential changes in the area accidentally reintroducing the bug. The https://bugs.openjdk.org/browse/JDK-8312174 change has been back-ported to 11, which resolved the crashes by luck.
> I'll request backport after this fix is integrated.

Nice. I've targeted it to 22. I agree it is better to have it back-ported. Its back-port is not going to be clean though.

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

PR Comment: https://git.openjdk.org/jdk/pull/16642#issuecomment-1830872157