Move all JI setAccessible to trySetAccessible.#5843
Merged
headius merged 2 commits intojruby:masterfrom Aug 21, 2019
Merged
Conversation
The logic previously was depending on the ji.setAccessible property, which by default would simply refuse to set accessible *at all* when the Java version was >= "1.9" (also showing this logic was introduced very early in the Java 9 development cycle.) That default is removed here, along with a the CAN_SET_ACCESSIBLE static field in RubyInstanceConfig deprecated and replaced with the original meaning of this property, simply whether to attempt setAccessible. The direct calls to setAccessible have been replaced by the trySetAccessible in backport9 library that checks if the target member's module is open to JRuby, allowing users to opt-in to that reflective access if their application needs it. Previously these cases still skipped setAccessible just due to being on Java 9+. Most of the changes here were masked by the property, but I have also fixed several others that were not guarded by the property or that were otherwise used in booting JRuby or extending Java integration. This fixes jruby#5841 by allowing `java_method` Method objects to go ahead and trySetAccessible when the module is open. This relates to jruby#5821, which fixed in 9.2.8 a similar issue where even an open module would not set fields and methods accessible.
kares
approved these changes
Aug 21, 2019
Member
kares
left a comment
There was a problem hiding this comment.
great, we're finally behave 99.9+% same on 8 as on Java 9+
minor suggestion: have a JRuby trySetAccessible helper
... so we do not have to keep in mind to pass Ruby.class
Member
Author
Yeah that might be a good idea. I hate to add something more to Ruby.java, but maybe to our Java class? It's already pretty fat and not really public API. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The logic previously was depending on the ji.setAccessible
property, which by default would simply refuse to set accessible
at all when the Java version was >= "1.9" (also showing this
logic was introduced very early in the Java 9 development cycle.)
That default is removed here, along with a the CAN_SET_ACCESSIBLE
static field in RubyInstanceConfig deprecated and replaced with
the original meaning of this property, simply whether to attempt
setAccessible.
The direct calls to setAccessible have been replaced by the
trySetAccessible in backport9 library that checks if the target
member's module is open to JRuby, allowing users to opt-in to that
reflective access if their application needs it. Previously these
cases still skipped setAccessible just due to being on Java 9+.
Most of the changes here were masked by the property, but I have
also fixed several others that were not guarded by the property
or that were otherwise used in booting JRuby or extending Java
integration.
This fixes #5841 by allowing
java_methodMethod objects togo ahead and trySetAccessible when the module is open.
This relates to #5821, which fixed in 9.2.8 a similar issue where
even an open module would not set fields and methods accessible.