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

Eliminate or configure invasive JDK class accesses for Java 9 #4834

Open
headius opened this Issue Oct 31, 2017 · 13 comments

Comments

Projects
None yet
7 participants
@headius
Member

headius commented Oct 31, 2017

In #4111 we tracked general Java 9 support, for running JRuby and associated apps and libraries.

This bug tracks the additional flags and cleanup we should do relating to accessibility warnings.

At a minimum, we need to audit all uses of reflection to "crack open" classes and find alternatives or consider --add-opens flags.

Here is the list of illegal accesses we do while running gem install rails:

WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/Users/headius/projects/jruby/lib/ruby/stdlib/jopenssl.jar) to constructor java.security.cert.CertificateFactory(java.security.cert.CertificateFactorySpi,java.security.Provider,java.lang.String)
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/Users/headius/projects/jruby/lib/ruby/stdlib/jopenssl.jar) to field java.security.MessageDigest.provider
WARNING: Illegal reflective access by org.jruby.ext.zlib.RubyZlib to field java.util.zip.CRC32.crc
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/Users/headius/projects/jruby/lib/ruby/stdlib/jopenssl.jar) to constructor java.security.cert.CertificateFactory(java.security.cert.CertificateFactorySpi,java.security.Provider,java.lang.String)
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/Users/headius/projects/jruby/lib/ruby/stdlib/jopenssl.jar) to constructor java.security.cert.CertificateFactory(java.security.cert.CertificateFactorySpi,java.security.Provider,java.lang.String)
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/Users/headius/projects/jruby/lib/ruby/stdlib/jopenssl.jar) to constructor java.security.cert.CertificateFactory(java.security.cert.CertificateFactorySpi,java.security.Provider,java.lang.String)

More of these warnings will occur on Windows or when running without native IO, since in those cases we crack open core JDK IO classes.

@headius headius added this to the JRuby 9.2.0.0 milestone Oct 31, 2017

@clonezone

This comment has been minimized.

Show comment
Hide comment
@clonezone

clonezone Nov 9, 2017

+1

Had to hack a bunch of build code to ignore this output.

clonezone commented Nov 9, 2017

+1

Had to hack a bunch of build code to ignore this output.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 10, 2017

Member

To help others dealing with the warnings, here's an example of eliminating them (a.k.a. a workaround!).

The --add-opens flag in Java 9 is needed for most of these, to fully open those packages up like they used to be in Java 8 and below. It requires a little hunting to figure out the module names, but for the three accesses during gem install rails I came up with the following three flags:

--add-opens java.base/java.security.cert=ALL-UNNAMED
--add-opens java.base/java.security=ALL-UNNAMED
--add-opens java.base/java.util.zip=ALL-UNNAMED

I put these into JAVA_OPTS env and that eliminates the warnings. Follow this pattern and you should be able to ease your build hacking.

We may need to install some of these flags into our launcher, for accesses that we can't yet replace through other means. In order for this to be really kosher with the module system, however, we'll want to export our own module so that the ALL-UNNAMED nuclear option is not necessary. We are tracking the module discussions (just starting) at #4835.

Member

headius commented Nov 10, 2017

To help others dealing with the warnings, here's an example of eliminating them (a.k.a. a workaround!).

The --add-opens flag in Java 9 is needed for most of these, to fully open those packages up like they used to be in Java 8 and below. It requires a little hunting to figure out the module names, but for the three accesses during gem install rails I came up with the following three flags:

--add-opens java.base/java.security.cert=ALL-UNNAMED
--add-opens java.base/java.security=ALL-UNNAMED
--add-opens java.base/java.util.zip=ALL-UNNAMED

I put these into JAVA_OPTS env and that eliminates the warnings. Follow this pattern and you should be able to ease your build hacking.

We may need to install some of these flags into our launcher, for accesses that we can't yet replace through other means. In order for this to be really kosher with the module system, however, we'll want to export our own module so that the ALL-UNNAMED nuclear option is not necessary. We are tracking the module discussions (just starting) at #4835.

@rallenh

This comment has been minimized.

Show comment
Hide comment
@rallenh

rallenh Nov 14, 2017

> jruby --version:

jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) 64-Bit Server VM 9.0.1+11 on 9.0.1+11 +jit [mswin32-x86_64]

> gem list:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jruby.runtime.encoding.EncodingService to field java.io.Console.cs
WARNING: Please consider reporting this to the maintainers of org.jruby.runtime.encoding.EncodingService
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

adding --add-opens java.base/java.io=ALL-UNNAMED:
> gem list:

*** LOCAL GEMS ***

did_you_mean (default: 1.0.1)
jar-dependencies (default: 0.3.10)
jruby-openssl (default: 0.9.21 java)
jruby-readline (default: 1.1.1 java)
json (default: 1.8.3 java)
minitest (default: 5.4.1)
net-telnet (default: 0.1.1)
power_assert (default: 0.2.3)
psych (default: 2.2.4 java)
rake (default: 10.4.2)
rdoc (default: 4.2.0)
test-unit (default: 3.1.1)

rallenh commented Nov 14, 2017

> jruby --version:

jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) 64-Bit Server VM 9.0.1+11 on 9.0.1+11 +jit [mswin32-x86_64]

> gem list:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jruby.runtime.encoding.EncodingService to field java.io.Console.cs
WARNING: Please consider reporting this to the maintainers of org.jruby.runtime.encoding.EncodingService
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

adding --add-opens java.base/java.io=ALL-UNNAMED:
> gem list:

*** LOCAL GEMS ***

did_you_mean (default: 1.0.1)
jar-dependencies (default: 0.3.10)
jruby-openssl (default: 0.9.21 java)
jruby-readline (default: 1.1.1 java)
json (default: 1.8.3 java)
minitest (default: 5.4.1)
net-telnet (default: 0.1.1)
power_assert (default: 0.2.3)
psych (default: 2.2.4 java)
rake (default: 10.4.2)
rdoc (default: 4.2.0)
test-unit (default: 3.1.1)
@zoras

This comment has been minimized.

Show comment
Hide comment
@zoras

zoras Nov 14, 2017

> jruby --version:
jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) 64-Bit Server VM 9.0.1+11 on 9.0.1+11 [darwin-x86_64]

> rails c
WARNING: Illegal reflective access by org.jruby.java.invokers.FieldMethodZero to field sun.nio.ch.FileChannelImpl.fd

I couldn't figure out the module name for this referenced from jruby/java/invokers/FieldMethodZero.java#L25.

Here're for others I found:

export JAVA_OPTS="$(echo --add-opens=java.base/{java.lang,java.security,java.util,java.security.cert,java.util.zip,java.lang.reflect,java.util.regex,java.net,java.io,java.lang}=ALL-UNNAMED) --illegal-access=warn"

zoras commented Nov 14, 2017

> jruby --version:
jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) 64-Bit Server VM 9.0.1+11 on 9.0.1+11 [darwin-x86_64]

> rails c
WARNING: Illegal reflective access by org.jruby.java.invokers.FieldMethodZero to field sun.nio.ch.FileChannelImpl.fd

I couldn't figure out the module name for this referenced from jruby/java/invokers/FieldMethodZero.java#L25.

Here're for others I found:

export JAVA_OPTS="$(echo --add-opens=java.base/{java.lang,java.security,java.util,java.security.cert,java.util.zip,java.lang.reflect,java.util.regex,java.net,java.io,java.lang}=ALL-UNNAMED) --illegal-access=warn"

@zoras

This comment has been minimized.

Show comment
Hide comment
@zoras

zoras Nov 16, 2017

I'm getting this error while running rake db:migrate on app running rails 4.2.10

❯ java --version                                                                                                                                 ✭ ◼
java 9.0.1
Java(TM) SE Runtime Environment (build 9.0.1+11)
Java HotSpot(TM) 64-Bit Server VM (build 9.0.1+11, mixed mode)

❯ jruby -v                                                                                                                                       ✭ ◼
jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) 64-Bit Server VM 9.0.1+11 on 9.0.1+11 [darwin-x86_64]

❯ rake db:migrate                                                                                                                                ✭ ◼
WARNING: Illegal reflective access by org.jruby.java.invokers.FieldMethodZero to field sun.nio.ch.FileChannelImpl.fd
dd7dcecacd7bc0a6c1aae7983e65bad55afff444
rake aborted!
TypeError: illegal access on 'clone': class org.jruby.javasupport.JavaMethod cannot access a member of class java.lang.Object (in module java.base) with modifiers "protected native"
arjdbc/jdbc/RubyJdbcConnection.java:460:in `init_connection'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

zoras commented Nov 16, 2017

I'm getting this error while running rake db:migrate on app running rails 4.2.10

❯ java --version                                                                                                                                 ✭ ◼
java 9.0.1
Java(TM) SE Runtime Environment (build 9.0.1+11)
Java HotSpot(TM) 64-Bit Server VM (build 9.0.1+11, mixed mode)

❯ jruby -v                                                                                                                                       ✭ ◼
jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) 64-Bit Server VM 9.0.1+11 on 9.0.1+11 [darwin-x86_64]

❯ rake db:migrate                                                                                                                                ✭ ◼
WARNING: Illegal reflective access by org.jruby.java.invokers.FieldMethodZero to field sun.nio.ch.FileChannelImpl.fd
dd7dcecacd7bc0a6c1aae7983e65bad55afff444
rake aborted!
TypeError: illegal access on 'clone': class org.jruby.javasupport.JavaMethod cannot access a member of class java.lang.Object (in module java.base) with modifiers "protected native"
arjdbc/jdbc/RubyJdbcConnection.java:460:in `init_connection'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

perlun pushed a commit to perlun/dotfiles that referenced this issue Nov 29, 2017

@greghuc

This comment has been minimized.

Show comment
Hide comment
@greghuc

greghuc Mar 7, 2018

Just tried upgrading a postgres-backed API to jruby 9.1.16.0 and java 9, which failed. Was busted running some database migrations via Sequel, which hit an illegal access: https://github.com/jeremyevans/sequel/blob/4cad051de9e63042c8950be69d6289686e19c0ac/lib/sequel/adapters/jdbc/postgresql.rb#L228

java -version
openjdk version "9-internal"
OpenJDK Runtime Environment (build 9-internal+0-2016-04-14-195246.buildd.src)
OpenJDK 64-Bit Server VM (build 9-internal+0-2016-04-14-195246.buildd.src, mixed mode)

jruby -version
jruby 9.1.16.0 (2.3.3) 2018-02-21 8f3f95a OpenJDK 64-Bit Server VM 9-internal+0-2016-04-14-195246.buildd.src on 9-internal+0-2016-04-14-195246.buildd.src +jit [linux-x86_64]

Error on running database migrations:

Error: TypeError: illegal access on 'getField': class org.jruby.javasupport.JavaMethod cannot access a member of class org.postgresql.jdbc.PgResultSetMetaData with modifiers "protected"
/home/vagrant/.rbenv/versions/jruby-9.1.16.0/lib/ruby/gems/shared/gems/sequel-5.6.0/lib/sequel/adapters/jdbc/postgresql.rb:228:in `type_convertor'

greghuc commented Mar 7, 2018

Just tried upgrading a postgres-backed API to jruby 9.1.16.0 and java 9, which failed. Was busted running some database migrations via Sequel, which hit an illegal access: https://github.com/jeremyevans/sequel/blob/4cad051de9e63042c8950be69d6289686e19c0ac/lib/sequel/adapters/jdbc/postgresql.rb#L228

java -version
openjdk version "9-internal"
OpenJDK Runtime Environment (build 9-internal+0-2016-04-14-195246.buildd.src)
OpenJDK 64-Bit Server VM (build 9-internal+0-2016-04-14-195246.buildd.src, mixed mode)

jruby -version
jruby 9.1.16.0 (2.3.3) 2018-02-21 8f3f95a OpenJDK 64-Bit Server VM 9-internal+0-2016-04-14-195246.buildd.src on 9-internal+0-2016-04-14-195246.buildd.src +jit [linux-x86_64]

Error on running database migrations:

Error: TypeError: illegal access on 'getField': class org.jruby.javasupport.JavaMethod cannot access a member of class org.postgresql.jdbc.PgResultSetMetaData with modifiers "protected"
/home/vagrant/.rbenv/versions/jruby-9.1.16.0/lib/ruby/gems/shared/gems/sequel-5.6.0/lib/sequel/adapters/jdbc/postgresql.rb:228:in `type_convertor'
@greghuc

This comment has been minimized.

Show comment
Hide comment
@greghuc

greghuc Mar 7, 2018

Actually, scratch my previous comment. I was running an older version of Java 9 ("9-internal"), rather than the latest "9.0.4". Running on latest 9.0.4, my database migrations and test suite run - but with 'Illegal reflective access' warnings. However, the puma web server fails to start: #5082

Lesson learned: install a specific version of java 9 directly, rather than depending on 'apt-get install openjdk-9-jdk'.

greghuc commented Mar 7, 2018

Actually, scratch my previous comment. I was running an older version of Java 9 ("9-internal"), rather than the latest "9.0.4". Running on latest 9.0.4, my database migrations and test suite run - but with 'Illegal reflective access' warnings. However, the puma web server fails to start: #5082

Lesson learned: install a specific version of java 9 directly, rather than depending on 'apt-get install openjdk-9-jdk'.

@jmiettinen

This comment has been minimized.

Show comment
Hide comment
@jmiettinen

jmiettinen Mar 14, 2018

Contributor

Is there a good reason for tinkering with CRC32 internals?

It seems to be just to support use of potentially faster implementation for call to Zlib.crc(string, non_zero_value). Is this a very common occurrence which we want to optimize for instead of using JZlib.crc32_combine that can handle non-zero start values as is the current slow path?

Contributor

jmiettinen commented Mar 14, 2018

Is there a good reason for tinkering with CRC32 internals?

It seems to be just to support use of potentially faster implementation for call to Zlib.crc(string, non_zero_value). Is this a very common occurrence which we want to optimize for instead of using JZlib.crc32_combine that can handle non-zero start values as is the current slow path?

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 15, 2018

Member

@jmiettinen I traced it back to this commit: e52313e

I did this so long ago I'm not entirely certain of my justification, but I believe it's to work around Java's CRC32 not being restartable at a particular initial value, which we need to support for Ruby's API.

This may no longer be a limitation. In any case, you're right, this is one of those items that should be fixed or eliminated, either by using better Java CRC32 APIs that allow an initial value or by doing a clean-room implementation of CRC32 that works the way we want.

Member

headius commented Mar 15, 2018

@jmiettinen I traced it back to this commit: e52313e

I did this so long ago I'm not entirely certain of my justification, but I believe it's to work around Java's CRC32 not being restartable at a particular initial value, which we need to support for Ruby's API.

This may no longer be a limitation. In any case, you're right, this is one of those items that should be fixed or eliminated, either by using better Java CRC32 APIs that allow an initial value or by doing a clean-room implementation of CRC32 that works the way we want.

@jmiettinen

This comment has been minimized.

Show comment
Hide comment
@jmiettinen

jmiettinen Mar 15, 2018

Contributor

Yeah, I can see that this is to support Ruby Zlib.crc(string, non_zero_value). But as there's now that JZLib included, could this invasive approach be removed?

It would probably be slightly slower as the OpenJDK CRC32 uses intrinsics, but it does not seem to make the code more complex at least:

final boolean slowPath = start != 0;
final int bytesLength = bytes == null ? 0 : bytes.length();
long result = 0;
if (bytes != null) {
    CRC32 checksum = new CRC32();
    checksum.update(bytes.getUnsafeBytes(), bytes.begin(), bytesLength);
    result = checksum.getValue();
}
if (slowPath) {
    result = JZlib.crc32_combine(start, result, bytesLength);
}
Contributor

jmiettinen commented Mar 15, 2018

Yeah, I can see that this is to support Ruby Zlib.crc(string, non_zero_value). But as there's now that JZLib included, could this invasive approach be removed?

It would probably be slightly slower as the OpenJDK CRC32 uses intrinsics, but it does not seem to make the code more complex at least:

final boolean slowPath = start != 0;
final int bytesLength = bytes == null ? 0 : bytes.length();
long result = 0;
if (bytes != null) {
    CRC32 checksum = new CRC32();
    checksum.update(bytes.getUnsafeBytes(), bytes.begin(), bytesLength);
    result = checksum.getValue();
}
if (slowPath) {
    result = JZlib.crc32_combine(start, result, bytesLength);
}
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 15, 2018

Member

@jmiettinen Hey, that's a great idea! Can you make a PR?

Member

headius commented Mar 15, 2018

@jmiettinen Hey, that's a great idea! Can you make a PR?

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 19, 2018

Member

Here's a set of opens that gets our build to not warn (it uses 9.1.8.0 during parts). @enebo and I talked today about creating a tool that can analyze the warning output and produce a command line.

--add-opens java.base/sun.nio.ch=ALL-UNNAMED
--add-opens java.base/java.io=ALL-UNNAMED
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.lang.reflect=ALL-UNNAMED
--add-opens java.base/java.util.regex=ALL-UNNAMED
--add-opens java.base/java.net=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/javax.crypto=ALL-UNNAMED
Member

headius commented Apr 19, 2018

Here's a set of opens that gets our build to not warn (it uses 9.1.8.0 during parts). @enebo and I talked today about creating a tool that can analyze the warning output and produce a command line.

--add-opens java.base/sun.nio.ch=ALL-UNNAMED
--add-opens java.base/java.io=ALL-UNNAMED
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.lang.reflect=ALL-UNNAMED
--add-opens java.base/java.util.regex=ALL-UNNAMED
--add-opens java.base/java.net=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/javax.crypto=ALL-UNNAMED

@enebo enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 24, 2018

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Oct 11, 2018

Member

This depends on #4835 which had some work for 9.2.1 and will have more for 9.2.2.

Member

headius commented Oct 11, 2018

This depends on #4835 which had some work for 9.2.1 and will have more for 9.2.2.

@headius headius modified the milestones: JRuby 9.2.1.0, JRuby 9.2.2.0 Oct 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment