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

Simplify Class and Object JNI wrappers #1105

Open
CoPrez opened this issue Jul 20, 2022 · 0 comments
Open

Simplify Class and Object JNI wrappers #1105

CoPrez opened this issue Jul 20, 2022 · 0 comments
Milestone

Comments

@CoPrez
Copy link
Contributor

CoPrez commented Jul 20, 2022

In this PR:
#1101

The Class and Object helpers in JavaWrappers were updated to restrict external access to the m_class and m_object internal members. The goal is to ensure when setting the member variables to a new jobject it using a global JNI ref (through calling env->NewGlobalRef). Doing so protects external classes and child classes from instantiating local JNI references that may become stale before the lifetime of the wrapper object. It does not however protect the Class and Object wrappers from misusing the jobject references.

Instead of making Class and Object aware of the need to create a global JNI reference a new templated class should be created to store the jobject reference and ensure we're creating global references. The Class and Object wrappers could then use that new container for their jclass and jobject member variables and remove the duplicated responsibility of maintaining global references.

Alternatively it might warrant looking into using the fbjni wrapper library instead of the custom JavaWrappers implementation.

@CoPrez CoPrez changed the title Simplify JClass and JObject wrappers Simplify Class and Object JNI wrappers Jul 20, 2022
@thomlucc thomlucc added the 6.0 label Sep 23, 2022
@thomlucc thomlucc modified the milestones: Future, 6.0 Sep 23, 2022
@thomlucc thomlucc removed the 6.0 label Sep 23, 2022
@bghgary bghgary modified the milestones: 6.0, Future Oct 13, 2022
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

No branches or pull requests

3 participants