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

We need to use NewGloblRef when caching jclass instances #9595

Merged
merged 2 commits into from Sep 23, 2019

Conversation

normanmaurer
Copy link
Member

Motivation:

It is not safe to cache a jclass without obtaining a global reference via NewGlobalRef.

Modifications:

Correctly use NewGlobalRef(...) before caching

Result:

Correctly cache jclass instance

Motivation:

It is not safe to cache a jclass without obtaining a global reference via NewGlobalRef.

Modifications:

Correctly use NewGlobalRef(...) before caching

Result:

Correctly cache jclass instance
@franz1981
Copy link
Contributor

I haven't noticed that...need to take a look to the JNI doc :O
Well done!

Copy link
Contributor

@johnou johnou left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@franz1981 franz1981 left a comment

Choose a reason for hiding this comment

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

lgtm!

@normanmaurer normanmaurer added this to the 4.1.42.Final milestone Sep 23, 2019
@normanmaurer normanmaurer merged commit dc4de7f into 4.1 Sep 23, 2019
@normanmaurer normanmaurer deleted the cache_jclass_correctly branch September 23, 2019 10:48
@normanmaurer normanmaurer self-assigned this Sep 23, 2019
normanmaurer added a commit that referenced this pull request Sep 23, 2019
Motivation:

It is not safe to cache a jclass without obtaining a global reference via NewGlobalRef.

Modifications:

Correctly use NewGlobalRef(...) before caching

Result:

Correctly cache jclass instance
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.

None yet

3 participants