Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JVMTI methods #16008

Merged

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Sep 28, 2022

  • Return JVMTI_ERROR_INVALID_THREAD if the input thread parameter is
    not a thread object.
  • Consolidate j.l.Thread and j.l.VirtualThread error-checking within
    getVMThread. This avoids repeated NULL checking and potentially
    fetching the current thread.
  • Convert getVMThread's boolean input parameters to a bit field.
  • More verbose naming used to avoid ambiguity: IS_JAVA_LANG_THREAD
    and IS_JAVA_LANG_VIRTUALTHREAD.

Fixes: #15986

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

@babsingh
Copy link
Contributor Author

babsingh commented Sep 28, 2022

See JVMTI spec, https://download.java.net/java/early_access/jdk19/docs/specs/jvmti.html, for confirmation.

@babsingh babsingh marked this pull request as draft September 28, 2022 18:47
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch from f1326cb to c62e49e Compare September 28, 2022 19:06
@babsingh babsingh changed the title Fix jvmtiGetCurrentContendedMonitor Fix JVMTI methods Sep 28, 2022
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch from c62e49e to 513a4d9 Compare September 28, 2022 19:25
@babsingh babsingh marked this pull request as ready for review September 28, 2022 19:35
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch from 513a4d9 to b0defb7 Compare September 29, 2022 14:07
@babsingh
Copy link
Contributor Author

Similar changes are being added to the JVMTI resume and suspend functions in #15903.

@babsingh
Copy link
Contributor Author

@gacholio Can you please review these changes?

@babsingh
Copy link
Contributor Author

getVMThread will throw JVMTI_ERROR_INVALID_THREAD if allowNull is FALSE. Moving ENSURE_JTHREAD check inside getVMThread will simplify the change. Let me give it a try.

@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch from b0defb7 to 2cf1d06 Compare September 29, 2022 19:52
@babsingh babsingh marked this pull request as draft September 29, 2022 19:53
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch 7 times, most recently from 1a0abc9 to 1ac8c88 Compare September 29, 2022 21:01
@babsingh babsingh marked this pull request as ready for review September 29, 2022 21:05
@babsingh
Copy link
Contributor Author

@gacholio Reworked the PR. All earlier comments have been addressed.

runtime/oti/j9.h Outdated Show resolved Hide resolved
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch 2 times, most recently from 7406037 to 4642a8d Compare September 30, 2022 17:39
@babsingh
Copy link
Contributor Author

Will have to retain ENSURE_JTHREAD macro since jvmtiRunAgentThread does not use getVMThread.

@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch from 4642a8d to f40fb61 Compare September 30, 2022 20:50
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch 2 times, most recently from b12fb29 to 003df7c Compare September 30, 2022 20:57
@babsingh babsingh marked this pull request as draft September 30, 2022 21:10
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch 4 times, most recently from e5682d0 to 5711401 Compare October 3, 2022 13:21
@babsingh babsingh marked this pull request as ready for review October 3, 2022 13:29
@babsingh
Copy link
Contributor Author

babsingh commented Oct 3, 2022

@gacholio The previous feedback has been addressed. This is ready for review.

@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch 4 times, most recently from b0f78b8 to ae0b96d Compare October 3, 2022 17:54
- Return JVMTI_ERROR_INVALID_THREAD if the input thread parameter is
not a thread object.
- Consolidate j.l.Thread and j.l.VirtualThread error-checking within
getVMThread. This avoids repeated NULL checking and potentially
fetching the current thread.
- Convert getVMThread's boolean input parameters to a bit field.
- More verbose naming used to avoid ambiguity: IS_JAVA_LANG_THREAD
and IS_JAVA_LANG_VIRTUALTHREAD.

Fixes: eclipse-openj9#15986

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh babsingh force-pushed the fix_GetCurrentContendedMonitor branch from ae0b96d to f0c5ffc Compare October 3, 2022 18:00
@gacholio
Copy link
Contributor

gacholio commented Oct 3, 2022

jenkins test sanity win jdk8

@gacholio
Copy link
Contributor

gacholio commented Oct 3, 2022

jenkins test sanity zlinux jdk19

@gacholio gacholio merged commit 2d9d132 into eclipse-openj9:master Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants