Protobuf: <Java> JDK 9 warning: Illegal reflective access by com.google.protobuf.UnsafeUtil to field java.nio.Buffer.address

[xenji]

G'day!

While testing one of our projects against JDK 9, the message below was printed to stderr. As it the message suggests, I'm now going on your nerves ;).

I've collected a few system details below and also a link to a file in your repo where I suspect the candidate that it complains about. If you can point me in a direction of how to solve it, I'm happy to contribute a PR.

As far as I can judge, the error message refers to this line here:
java/core/src/main/java/com/google/protobuf/UnsafeUtil.java#L378

---

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.protobuf.UnsafeUtil (file:/Users/mmueller/.gradle/caches/modules-2/files-2.1/com.google.protobuf/protobuf-java/3.4.0/b32aba0cbe737a4ca953f71688725972e3ee927c/protobuf-java-3.4.0.jar) to field java.nio.Buffer.address
WARNING: Please consider reporting this to the maintainers of com.google.protobuf.UnsafeUtil
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

System specs:

System:
Darwin Brick 17.0.0 Darwin Kernel Version 17.0.0: Thu Aug 24 21:48:19 PDT 2017; root:xnu-4570.1.46~2/RELEASE_X86_64 x86_6

Java:
java version "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)

Gradle:
------------------------------------------------------------
Gradle 4.2.1
------------------------------------------------------------

Build time:   2017-10-02 15:36:21 UTC
Revision:     a88ebd6be7840c2e59ae4782eb0f27fbe3405ddf

Groovy:       2.4.12
Ant:          Apache Ant(TM) version 1.9.6 compiled on June 29 2015
JVM:          9.0.1 (Oracle Corporation 9.0.1+11)
OS:           Mac OS X 10.13 x86_64

[liujisi]

This is from the unsafe utility that we use in Google to optimize for performance without generating code. We are aware that the unsafe APIs are planned to be deprecated/removed since Java9 - thus this warning. The UnsafeUtil code is opensourced to show the usages of those APIs, and we are still working with the Java9 community to resolve the issue.

That said, your project are not relying on the unsafe access, unless you explicitly call the unsafe APIs. The log is printed only because of the class initialization.

[bartlomiej-dawidow]

Most major libraries have fixed this, or are working on it, protobuf should do the same:
https://github.com/netty/netty/pull/7650
https://github.com/google/guice/issues/1085

Tolerating warnings in logs is universally considered a bad practice. Java 9 has been GA for four months.

[xfxyjwf]

@bartlomiej-dawidow Do you have a suggestion about how to fix the UnsafeUtil code? It's not clear to me whether there is an alternative that works on Java 7+ and doesn't trigger the Java 9 warning.

[bartlomiej-dawidow]

Based on @pherl 's response the unsafe code is only used for optional optimizations and it is not needed for the library to work:
https://github.com/google/protobuf/issues/3781#issuecomment-339794368

So a natural fix would be to move that functionality to a separate Maven module, for example protobuf-java-unsafe. You could probably achieve the same effect with some class loader magic that would delay the problematic class's initialization until it is actually called.

[xfxyjwf]

@bartlomiej-dawidow It's not a requirement at the moment, but will likely become one in the future. Separating the code or delay the initialization won't fix the issue.

[shemnon]

@xfxyjwf Do a multi-release jar: http://openjdk.java.net/jeps/238 - The main issue is how to wrangle bazel an acceptable fashion to make one of those, since the Java 9 version of the class will need to have the same name of com.google.protobuf.UnsafeUtils in a dir bazel doesn't natively support.

[xfxyjwf]

@shemnon Thanks! That multi-release jar seems exactly what we need to provide support for both Java 9 and pre Java 9. I'll investigate how we can make use of it in protobuf.

[outkaj]

Hello! What's the status of work on the multi-release jar mentioned above? Thanks!

[almothafar]

It is Java 10 here and I'm still getting warnings!
Any news?

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.protobuf.UnsafeUtil (file:.......) to field java.nio.Buffer.address
WARNING: Please consider reporting this to the maintainers of com.google.protobuf.UnsafeUtil
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

[saturnism]

trying to use Graal and definitely hit issues w/ unsafe usage.

[saturnism]

quick ping; java 11 is released.

[schlachtzeuger]

It's nearly two years since this issue popped up and there are only a few months left (Feb '19) until Java8 runs out of support. Please provide a new version that makes protobuf compatible with current Java versions.

[eed3si9n]

What's the current blocker? Is this waiting on Bazel's ability to create Multi-Release JAR? - https://github.com/bazelbuild/bazel/issues/5947

In the meantime could we move on using any other build tool like Maven, Gradle, or sbt that can build Multi-Release JAR, please? Also if anyone has created a fork, please let me know.
https://github.com/protocolbuffers/protobuf/pull/4557 got closed in April, but since the use of Unsafe is guarded by if condition, putting null in there based on System.getProperty("java.version") should be slow, but an okay workaround right?

[shemnon]

Once Google moves off of Java 8 internally I suspect they will just fix it and say Java 8 (or performant Java 8) is out of scope of this library.

[almothafar]

They can be under different versions! it would be simple like that, like v4 for JDK 9/10/11+ and v3 for JDK 7/8, We have 3 versions, and soon 4th version after JDK 8, I understand that Java 8 is mostly used now (since Oct. 2013) and Java 9 from 2017, but still last 3 versions added a lot of things for Java.

[perlun]

@xfxyjwf Any updates on this?

[ejona86]

FYI: I tested with --illegal-access=fail and protobuf operated correctly and got no warning. Seems it may be a false positive.

I'd be very interested to see a failure caused by the access.

[cushon]

@ejona86 the flag is --illegal-access=deny (not warn). One of the warnings is coming from this part of UnsafeUtil, which accesses java.nio.Buffer#address: https://github.com/protocolbuffers/protobuf/blob/29f27bfd0bd474bc3272fead14917a689c1825db/java/core/src/main/java/com/google/protobuf/UnsafeUtil.java#L370-L374

That's roughly equivalent to the following:

import java.lang.reflect.Field;
import java.nio.Buffer;

class T {
  public static void main(String[] args) throws Exception {
    Field f = Buffer.class.getDeclaredField("address");
    f.setAccessible(true);
  }
}

$ java -fullversion
openjdk full version "11.0.1+13"
$ java --illegal-access=deny T
Exception in thread "main" java.lang.reflect.InaccessibleObjectException: Unable to make field long java.nio.Buffer.address accessible: module java.base does not "opens java.nio" to unnamed module @606d8acf
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:340)
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:280)
        at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:176)
        at java.base/java.lang.reflect.Field.setAccessible(Field.java:170)
        at T.main(T.java:7)

[kevinashaw]

I am continuing to get this error message with protobuf-java-3.6.1 with Java 9:
WARNING: Illegal reflective access by com.google.protobuf.UnsafeUtil

[ejona86]

@cushon, I ran it with deny as well :). I ran it and then later mentioned I had done it, and so got mixed up with with flag actually worked.

[ejona86]

@cushon, also your code snipped isn't quite equivalent. The actual code will catch the exception and move on. Now there may be a bug, but the code at least attempts to behave well.

It looks like IterableByteBufferInputStream, for example, fails to check hasUnsafeByteBufferOperations(). But I see lots of other places that do check for availability first.

[ghost]

I started digging into this issue. The warning stems from here. The Field returns by this method is simply used to pass to Unsafe.objectFieldOffset(), which seems to work even without setAccessible(true).

I might be missing something, but we may be able to simply remove that call, which will get rid of the warning.

[mkurz]

(@haon4 Your second link does not jump to the line)

[ghost]

(@haon4 Your second link does not jump to the line)

Oops. Missing an L in the URL. Fixed.

[ejona86]

The setAccessible is necessary to have later reflective calls of the field work, because the fields are private.

[ejona86]

It was pointed out to me that the field() method is only used once. So it does actually look like the setAccessible() may be unnecessary.

[ghost]

This issue should be fixed at master with https://github.com/protocolbuffers/protobuf/commit/624a40a387e1b2da4c6bbde44ad3d0a5efb9879b#diff-6c9e043b78f78efc096211703bb7fa4a, which should be included in our next release.