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

JVMTI NotSuspended test fails with incorrect virtual thread stacktrace #16688

Closed
dipak-bagadiya opened this issue Feb 8, 2023 · 3 comments
Closed

Comments

@dipak-bagadiya
Copy link
Contributor

dipak-bagadiya commented Feb 8, 2023

Issue

The stacktrace of virtual thread is incorrect since it doesn't start/match the CONTINUATION_CLASS_NAME
and CONTINUATION_METHOD_NAME.

Test CMD

make test TEST="jtreg:test/hotspot/jtreg/serviceability/jvmti/stress/StackTrace/NotSuspended" JTREG="JAVA_OPTIONS=--enable-preview -Dvm.continuations=true;VERBOSE=all"

Test Output

STDOUT:
Agent_OnLoad started
Agent_OnLoad finished
Synchronization point checkStatus(0) called.
Data 0x7f2f740a4bd8 0x7f2f54002b18
Agent: wait for thread to start
Agent: started
Agent: Stacktrace of virtual thread is incorrect: doesn't start from enter(...):
JVMTI Stack Trace: frame count: 6
 0: java/lang/Object: waitImpl(JI)V
 1: java/lang/Object: wait(JI)V
 2: java/lang/Object: wait(J)V
 3: java/lang/Thread: join(J)V
 4: java/lang/Thread: join()V
 5: com/sun/javatest/regtest/agent/MainWrapper: main([Ljava/lang/String;)V
@babsingh babsingh changed the title JVMTI NotSuspended Test Failed with Incorrect Stacktrace of Virtual Thread  JVMTI NotSuspended test fails with incorrect virtual thread stacktrace Feb 8, 2023
@babsingh babsingh added this to the Java 20 milestone Feb 8, 2023
@babsingh
Copy link
Contributor

babsingh commented Feb 8, 2023

@fengxue-IS I believe that this test fails because our implementation of Continuation does not match the RI. Do you agree? If so, do you want to match the RI or permanently exclude the test?

@fengxue-IS
Copy link
Contributor

fengxue-IS commented Feb 9, 2023

@fengxue-IS I believe that this test fails because our implementation of Continuation does not match the RI. Do you agree? If so, do you want to match the RI or permanently exclude the test?

This is not due to mismatch of the impl, looking at the test code, seem like we have returned a carrier/platform thread as result of GetVirtualThread call, this should be fixed in jvmtiGetVirtualThread code.

The test code seem to suggest that a JVMTI_ERROR_THREAD_NOT_ALIVE error should be returned when the platform thread passed into GetVirtualThread doesn't have a virtual thread mounted.

Once that have been fixed, we should retest to see if it is still looking at implementation specific details. (I suspect that will be the case as OpenJ9's continuation entry point is Continuation.execute(...) instead of the enter(...) expected by the test case.

babsingh added a commit to babsingh/openj9 that referenced this issue Feb 27, 2023
If no virtual thread is mounted on the targetThread i.e.
carrierThreadObject == threadObject, then return null
for the output virtual thread value.

Virtual thread is not pinned in this function. So, read
threadObject and carrierThreadObject once and store their
values to avoid inconsistency.

Related: eclipse-openj9#16688
Related: eclipse-openj9#16751

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/openj9 that referenced this issue Feb 27, 2023
Continuation.execute is renamed to Continuation.enter.

Matching the RI will allow us to run more OpenJDK tests without
tweaking them for our implementation and be better prepared to
support unimplemented/new features in the future.

Related: eclipse-openj9#16688
Related: eclipse-openj9#16751

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/aqa-tests that referenced this issue May 2, 2023
- NotSuspended/GetStackTraceNotSuspendedStressTest has been fixed.
- SuspendThread/suspendthrd03 has been removed in JDK20.
- Now, VThreadTest fails because of eclipse-openj9/openj9#15920.

Related:
- eclipse-openj9/openj9#16688
- eclipse-openj9/openj9#16242
- eclipse-openj9/openj9#17307

Depends on eclipse-openj9/openj9#17318

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/aqa-tests that referenced this issue May 2, 2023
- NotSuspended/GetStackTraceNotSuspendedStressTest has been fixed.
- SuspendThread/suspendthrd03 has been removed in JDK20.
- Now, VThreadTest fails because of eclipse-openj9/openj9#15920.
- eclipse-openj9/openj9#16185 and eclipse-openj9/openj9#16279 are
permanently excluded. Changed their reason to adoptium#1297 to support
the test tool, which automatically enables tests after the related
issues are closed.

Related:
- eclipse-openj9/openj9#16688
- eclipse-openj9/openj9#16242
- eclipse-openj9/openj9#17307

Depends on eclipse-openj9/openj9#17318

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
llxia added a commit to adoptium/aqa-tests that referenced this issue May 2, 2023
- NotSuspended/GetStackTraceNotSuspendedStressTest has been fixed.
- SuspendThread/suspendthrd03 has been removed in JDK20.
- Now, VThreadTest fails because of eclipse-openj9/openj9#15920.
- eclipse-openj9/openj9#16185 and eclipse-openj9/openj9#16279 are
permanently excluded. Changed their reason to #1297 to support
the test tool, which automatically enables tests after the related
issues are closed.

Related:
- eclipse-openj9/openj9#16688
- eclipse-openj9/openj9#16242
- eclipse-openj9/openj9#17307

Depends on eclipse-openj9/openj9#17318

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>
Co-authored-by: Lan Xia <19273206+llxia@users.noreply.github.com>
@babsingh
Copy link
Contributor

babsingh commented May 2, 2023

NotSuspended/GetStackTraceNotSuspendedStressTest has been enabled in adoptium/aqa-tests#4554.

The test has been fixed through the following PRs:

@babsingh babsingh closed this as completed May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants