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

Invalidate JNI IDs for unloaded classes #18519

Merged
merged 1 commit into from
Nov 24, 2023
Merged

Conversation

gacholio
Copy link
Contributor

If the -XX:+KeepJNIIDs option is specified, invalidate IDs for unloaded classes rather than freeing them.

Detect invalidated IDs in JVMTI calls and return the appropriate error.

JNI calls may be updated in a future PR (we will need to decide how to handle invalid IDs in those cases as there is no standard error handling mechanism in JNI).

Related: #17466

@gacholio gacholio requested a review from tajila November 23, 2023 18:13
If the -XX:+KeepJNIIDs option is specified, invalidate IDs for unloaded
classes rather than freeing them.

Detect invalidated IDs in JVMTI calls and return the appropriate error.

JNI calls may be updated in a future PR (we will need to decide how to
handle invalid IDs in those cases as there is no standard error handling
mechanism in JNI).

Related: eclipse-openj9#17466

Signed-off-by: Graham Chapman <[email protected]>
@gacholio
Copy link
Contributor Author

Tested internally with the new option off and forced on.

@tajila
Copy link
Contributor

tajila commented Nov 23, 2023

jenkins test sanity alinux64 jdk21

@pshipton
Copy link
Member

Is the plan to document the option?

@gacholio
Copy link
Contributor Author

If we go to production with this, probably (though requiring new options seems to be problematic in general for people). A probably worse alternative is to auto-enable this when we detect ClassLoadHook has been enabled (the hack required to enabled ASGCT).

@tajila
Copy link
Contributor

tajila commented Nov 23, 2023

Is the plan to document the option?

Im not in favour of this. AGCT is an undocumented best effort solution. Documenting an option to better support it legitimize the API in a manner I feel is mis-leading.

A probably worse alternative is to auto-enable this when we detect ClassLoadHook has been enabled

If it comes to it, this is probably the least worst outcome IMO. It puts us at parity with the RI.

@tajila tajila merged commit a2c9085 into eclipse-openj9:master Nov 24, 2023
@gacholio gacholio deleted the jniid branch November 24, 2023 12:48
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.

3 participants