Skip to content

Update monitor count assertions - #21541

Merged
babsingh merged 1 commit into
eclipse-openj9:masterfrom
tajila:vt6
Apr 1, 2025
Merged

Update monitor count assertions#21541
babsingh merged 1 commit into
eclipse-openj9:masterfrom
tajila:vt6

Conversation

@tajila

@tajila tajila commented Apr 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

The ownedMonitorCount includes the number of re-entries to the same monitor whereas monitorCount does not.

@tajila
tajila requested review from babsingh and fengxue-IS April 1, 2025 16:35
@tajila

tajila commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

jenkins test sanity.functional amac jdk24

fengxue-IS
fengxue-IS suggested changes Apr 1, 2025
View reviewed changes

@fengxue-IS fengxue-IS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove the monitorCount += 1 on line 900-905 and line 1056-1060, this will break the osThread->lockedmonitorcount value since object.wait will update those separately via the omrthread_monitor_exit on line 1111 (and in enterContinuationImpl` when it reacquires the monitor)

@tajila

tajila commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

@fengxue-IS updated changes

@tajila tajila changed the title Update monitor count asserations Update monitor count assertions Apr 1, 2025
@tajila
Update monitor count assertions
7639862 
The ownedMonitorCount includes the number of re-entries to the same
monitor whereas monitorCount does not.

Signed-off-by: Tobi Ajila <atobia@ca.ibm.com>
@babsingh

babsingh commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

We should also run sanity.openjdk in the PR builds.

fengxue-IS
fengxue-IS approved these changes Apr 1, 2025
View reviewed changes

@fengxue-IS fengxue-IS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@tajila

tajila commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

jenkins test sanity amac jdk24

@fengxue-IS fengxue-IS mentioned this pull request Apr 1, 2025
Merged
@tajila

tajila commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

Only failure was java/lang/Thread/virtual/RetryMonitorEnterWhenPinned.java and its not hitting the assertion.

@tajila

tajila commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

it looks similar to #20955 (comment)

@tajila
tajila marked this pull request as ready for review April 1, 2025 21:01
@tajila

tajila commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

@babsingh please take a look

@JasonFengJ9

JasonFengJ9 commented Apr 1, 2025

Copy link
Copy Markdown
Member

Only failure was java/lang/Thread/virtual/RetryMonitorEnterWhenPinned.java and its not hitting the assertion.

The test is re-excluded.

@babsingh
babsingh merged commit 7431737 into eclipse-openj9:master Apr 1, 2025
@babsingh

babsingh commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

#21533 along with this PR need to be ported to the 0.51 branch.

@tajila tajila mentioned this pull request Apr 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@babsingh babsingh babsingh approved these changes
+1 more reviewer
@fengxue-IS fengxue-IS fengxue-IS approved these changes
Reviewers whose approvals may not affect merge requirements

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

@tajila @babsingh @JasonFengJ9 @fengxue-IS