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

Close VM and thread libraries on successful DestroyJavaVM #17336

Merged
merged 1 commit into from
May 9, 2023

Conversation

gacholio
Copy link
Contributor

@gacholio gacholio commented May 4, 2023

Close VM and thread libraries if DestroyJavaVM is successful.

Related: #17267

Signed-off-by: Graham Chapman graham_chapman@ca.ibm.com

Close VM and thread libraries if DestroyJavaVM is successful.

Related: eclipse-openj9#17267

Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
@gacholio
Copy link
Contributor Author

gacholio commented May 4, 2023

This has passed an all platforms internal build.

@gacholio gacholio changed the title close libraries Close VM and thread libraries on successful DestroyJavaVM May 4, 2023
@gacholio gacholio marked this pull request as ready for review May 5, 2023 00:54
@gacholio gacholio requested a review from pshipton May 5, 2023 00:55
@pshipton
Copy link
Member

pshipton commented May 5, 2023

There are other libraries preloaded as well, should we save the handles so they can also be closed?
On Windows: hookable, port, j9zip
Others: omrsig, port
and z/OS also opens java

@gacholio
Copy link
Contributor Author

gacholio commented May 5, 2023

Perhaps - if they have globals they could be a problem.

@gacholio
Copy link
Contributor Author

gacholio commented May 9, 2023

Can I suggest we worry about other libraries in a future PR? This is really meant to address the foolish thread control API that fills in the memory categories (no doubt a result of the J9/OMR disconnect).

@pshipton
Copy link
Member

pshipton commented May 9, 2023

Ok.

@pshipton pshipton merged commit 7815549 into eclipse-openj9:master May 9, 2023
2 checks passed
@gacholio gacholio deleted the multivm branch May 9, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants