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

Move all JI setAccessible to trySetAccessible. #5843

Merged
merged 2 commits into from Aug 21, 2019

Conversation

@headius
Copy link
Member

headius commented Aug 21, 2019

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_method Method objects to
go 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.

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_method` Method objects to
go 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.
@headius headius added this to the JRuby 9.2.9.0 milestone Aug 21, 2019
@headius headius requested a review from kares Aug 21, 2019
@kares
kares approved these changes Aug 21, 2019
Copy link
Member

kares left a 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

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Aug 21, 2019

minor suggestion: have a JRuby trySetAccessible helper
... so we do not have to keep in mind to pass Ruby.class

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. Java.trySetAccessible would just do Modules.trySetAccessible(..., Java.class) and have the same effect.

@headius headius merged commit fe0411a into jruby:master Aug 21, 2019
4 of 5 checks passed
4 of 5 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
jruby.jruby Build #20190821.4 succeeded
Details
jruby.jruby (Job linux) Job linux succeeded
Details
jruby.jruby (Job mac) Job mac succeeded
Details
jruby.jruby (Job windows) Job windows succeeded
Details
@headius headius deleted the headius:ji_try_set_accessible branch Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.