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.

Sign up for GitHub

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

Jump to bottom

Adjust ThreadListStackTracesTest for checkReentrantLock #63

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Adjust ThreadListStackTracesTest for checkReentrantLock #63

merged 1 commit into from
Nov 17, 2023

Conversation

singh264
Copy link
Contributor

@singh264 singh264 commented Nov 16, 2023 •
edited
Loading

Adjust ThreadListStackTracesTest for checkReentrantLock
to be ready when the virtual thread state is WAITING after
the reentrant lock parks the virtual thread. When the
virtual thread is mounted, the virtual thread state, which
is obtained from the carrier thread, can transition
between WAITING and RUNNABLE before the virtual thread is
parked on the lock.

Issue: eclipse-openj9/openj9#18089
Signed-off-by: Amarpreet Singh [email protected]

@singh264 singh264 marked this pull request as ready for review November 16, 2023 17:03
@singh264
Copy link
Contributor Author

@babsingh requesting review

@singh264 singh264 mentioned this pull request Nov 16, 2023
Closed
@babsingh
Copy link
Member

In the commit message, the flow is better if Thread.State. is removed i.e. Thread.State.WAITING -> WAITING.

@babsingh
Copy link
Member

The test is flawed due to the timing gap. This change will reduce the frequency of the failure. In the worst case, the checkReentrantLock sub-test can be disabled via commenting if the failure still occurs due to the timing gap.

How less frequent is the failure with this PR? Does it never occur?

@babsingh babsingh mentioned this pull request Nov 16, 2023
Merged
@singh264
Copy link
Contributor Author

How less frequent is the failure with this PR? Does it never occur?

The test does not fail with this PR when it is run 2500 times.

The test fails approximately once every 100 times without this PR.

@singh264 singh264 force-pushed the openj9_issues_18089 branch from 4b4f9cf to c2578b0 Compare November 16, 2023 18:36
@singh264 singh264 requested a review from babsingh November 16, 2023 18:41
babsingh
babsingh reviewed Nov 16, 2023
View reviewed changes
Copy link
Member

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

Also, squash the commits.

.../jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java Show resolved Hide resolved
.../jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java Outdated Show resolved Hide resolved
.../jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java Outdated Show resolved Hide resolved
@singh264
Adjust ThreadListStackTracesTest for checkReentrantLock
aa716a1 
Adjust ThreadListStackTracesTest for checkReentrantLock
to be ready when the virtual thread state is WAITING after
the reentrant lock parks the virtual thread. When the
virtual thread is mounted, the virtual thread state, which
is obtained from the carrier thread, can transition
between WAITING and RUNNABLE before the virtual thread is
parked on the lock.

Issue: eclipse-openj9/openj9#18089
Signed-off-by: Amarpreet Singh <[email protected]>
@singh264 singh264 force-pushed the openj9_issues_18089 branch from be222e4 to aa716a1 Compare November 16, 2023 19:17
@singh264
Copy link
Contributor Author

Also, squash the commits.

@babsingh I have squashed the commits, updated the commit message, and addressed the new comments.

Re-requesting your review.

@singh264 singh264 requested a review from babsingh November 16, 2023 19:43
babsingh
babsingh approved these changes Nov 17, 2023
View reviewed changes
Copy link
Member

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

300x grinder passed: https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/36164.

@babsingh babsingh merged commit 56e65df into ibmruntimes:openj9 Nov 17, 2023
2 checks passed
@tajila
Copy link
Member

tajila commented Nov 20, 2023

@singh264 Need to double deliver to 0.42

@singh264
Copy link
Contributor Author

@singh264 Need to double deliver to 0.42

@tajila I have created a PR to merge the changes to branch v0.42.0-release #70.

@babsingh
Copy link
Member

babsingh commented Nov 20, 2023 •
edited
Loading

@singh264
Copy link
Contributor Author

These changes are also needed for 0.43 since 0.43 is a Java 21 refresh.

@babsingh I have created a PR to merge the changes to branch v0.43.0-release #71.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keithc-ca keithc-ca keithc-ca left review comments

@babsingh babsingh babsingh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@singh264 @babsingh @tajila @keithc-ca