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 jvmtiGetThreadInfo to return correct thread group #16376

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

dipak-bagadiya
Copy link
Contributor

The jvmtiGetThreadInfo returns the invalid thread group in case of unstarted threads. A NULL targetThread indicates that the thread is either unstarted or terminated so this check is not sufficient to validate terminated threads.

The boolean check is updated in jvmtiGetThreadInfo to return a valid thread group for unstarted threads.

Fixes: #16226

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com

@dipak-bagadiya dipak-bagadiya marked this pull request as ready for review November 29, 2022 15:04
@dipak-bagadiya
Copy link
Contributor Author

@babsingh and @fengxue-IS Please review PR

Copy link
Contributor

@fengxue-IS fengxue-IS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

runtime/jvmti/jvmtiThread.c Outdated Show resolved Hide resolved
@babsingh
Copy link
Contributor

https://openj9-jenkins.osuosl.org/job/PullRequest-LineEndingsCheck-OpenJ9/7285/console

@dipak-bagadiya The line endings check is failing.

13:36:06  runtime/jvmti/jvmtiThread.c:529: space before tab in indent.
13:36:06  + 					 * As per the JVMTI spec, the thread group should be NULL IFF the thread is terminated.
13:36:06  runtime/jvmti/jvmtiThread.c:530: space before tab in indent.
13:36:06  + 					 */

The jvmtiGetThreadInfo returns the invalid thread group in case of
unstarted threads. A NULL targetThread indicates that the thread is
either unstarted or terminated so this check is not sufficient to
validate terminated threads.

The boolean check is updated in jvmtiGetThreadInfo to return a valid
thread group for unstarted threads.

Fixes: eclipse-openj9#16226

Signed-off-by: Dipak Bagadiya <dipak.bagadiya@ibm.com>
Copy link
Contributor

@fengxue-IS fengxue-IS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@babsingh
Copy link
Contributor

jenkins test sanity zlinux jdk19

@babsingh
Copy link
Contributor

jenkins compile win jdk8

@babsingh
Copy link
Contributor

babsingh commented Nov 29, 2022

@dipak-bagadiya After this PR is merged, we can re-enable the test. The following exclude lines need to be removed:

Also, the test excluded label will need to be removed in #16226.

@fengxue-IS I will be on vacation for the remainder of the week. Please assist Dipak if there are any doubts.

@babsingh
Copy link
Contributor

jenkins test sanity amac jdk19

@babsingh
Copy link
Contributor

https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.functional_s390x_linux_Personal/47/tapResults/

@pshipton @JasonFengJ9 @AdamBrousseau Are the below file system failures related to an infra issue?

FAILED: test_isAnnotationPresent
java.lang.AssertionError: Following exception occured : Cannot invoke "java.io.File.deleteOnExit()" because "folder" is null
	at org.testng.Assert.fail(Assert.java:96) from jdk.internal.loader.ClassLoaders$AppClassLoader@43abce9b(file:/home/jenkins/workspace/Test_openjdk19_j9_sanity.functional_s390x_linux_Personal_testList_1/aqa-tests/TKG/lib/testng.jar)
	at org.openj9.test.annotationPackage.Test_PackageAnnotation2.test_isAnnotationPresent(Test_PackageAnnotation2.java:52) from jdk.internal.loader.ClassLoaders$AppClassLoader@43abce9b(file:/home/jenkins/workspace/Test_openjdk19_j9_sanity.functional_s390x_linux_Personal_testList_1/jvmtest/functional/Java8andUp/GeneralTest.jar)
...

FAILED: test_fileHasOtherAccess
java.lang.AssertionError: null
	at org.testng.Assert.fail(Assert.java:96)
	at org.testng.Assert.fail(Assert.java:103)
	at org.openj9.test.jdk.internal.agent.Test_FileSystem_Base.testSetup(Test_FileSystem_Base.java:47)
...

@babsingh
Copy link
Contributor

jenkins test sanity xlinux jdk19

@AdamBrousseau
Copy link
Contributor

Unless that test cares about cleaning up the files, I would assume an infra issue. Folder is cleanup up on disk now.

@JasonFengJ9
Copy link
Member

[TargetManager] [ERROR] could not load or use openj9.internal.tools.attach.target.AttachHandler

This looks like a machine/setup issue.

@JasonFengJ9
Copy link
Member

FAILED: test_getResources
java.lang.NullPointerException: Cannot invoke "java.io.File.deleteOnExit()" because "folder" is null
at org.openj9.test.support.resource.Support_Resources.createTempFolder(Support_Resources.java:85) from

java.io.IOException: Not a directory
at java.base/java.io.UnixFileSystem.createFileExclusively(UnixFileSystem.java:356) from jrt:/java.base
at java.base/java.io.File.createTempFile(File.java:2179) from jrt:/java.base
at java.base/java.io.File.createTempFile(File.java:2225) from jrt:/java.base
at org.testng.reporters.FileStringBuffer.flushToFile(FileStringBuffer.java:105) from

There are quite a few errors in sanity.funtional, wondering if it is related to adoptium/TKG#378 which has been reverted.
@babsingh I suggest another PR build.

@babsingh
Copy link
Contributor

jenkins test sanity zlinux jdk19

@babsingh babsingh merged commit afb2309 into eclipse-openj9:master Nov 30, 2022
dipak-bagadiya added a commit to dipak-bagadiya/aqa-tests that referenced this pull request Nov 30, 2022
dipak-bagadiya added a commit to dipak-bagadiya/aqa-tests that referenced this pull request Dec 1, 2022
Test had been fixed by-
eclipse-openj9/openj9#16376

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
llxia pushed a commit to adoptium/aqa-tests that referenced this pull request Dec 1, 2022
Re-enable JTReg test thrinfo01
Test had been fixed by eclipse-openj9/openj9#16376

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JVMTI GetThreadInfo/thrinfo01 Fails
5 participants