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

Clear vmthread from thread object at shutdown #18314

Closed
wants to merge 1 commit into from

Conversation

tajila
Copy link
Contributor

@tajila tajila commented Oct 19, 2023

Related to: #16659

@tajila
Copy link
Contributor Author

tajila commented Oct 19, 2023

jenkins test sanity alinux64 jdk21

@tajila
Copy link
Contributor Author

tajila commented Oct 20, 2023

will investigate the jitserver failures

@gacholio
Copy link
Contributor

This is already done in the java code that runs when a thread ends.

@tajila
Copy link
Contributor Author

tajila commented Oct 20, 2023

This is already done in the java code that runs when a thread ends.

If there is an exception in that code then the threadref is not guaranteed to be cleared. I thought about updating that code and putting eetop = Thread.NO_REF; in a finally block, however, with Java code its always possible that someone can modify it such that the threadref is not cleared. So I figure the safest thing is to do it natively.

@gacholio
Copy link
Contributor

gacholio commented Oct 20, 2023

The java code holds a lock while setting it (not sure if that actually does anything), so the native code is not quite the same.

I would say anyone modifying Thread internals in a way that avoids the finally block gets what they deserve, so I think the java solution would be better (especially considering the native code has introduced failures).

@tajila
Copy link
Contributor Author

tajila commented Oct 23, 2023

Replaced with ibmruntimes/openj9-openjdk-jdk#679

@tajila tajila closed this Oct 23, 2023
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.

2 participants