EsotericSoftware/kryo

Java 9 illegal reflective access warning

guidomedina opened this issue ยท 22 comments

I'm quite sure it can be ignore for some time, but reporting it anyway:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.esotericsoftware.kryo.util.UnsafeUtil (file:/home/signals/green-engine-runner/scripts/admin/repo/kryo-4.0.1.jar) to constructor java.nio.DirectByteBuffer(long,int,java.lang.Object)
WARNING: Please consider reporting this to the maintainers of com.esotericsoftware.kryo.util.UnsafeUtil

news on this ?

I am also interested in an update. I think we can ignore, but it is a bit messy in the logs when i use java 9.

Warning aside, I'm more interested in understanding what this means for Kryo once this breaks. Is this something that has a clear upgrade path?

Hi, i am the dev of fast-serialization, another serialization replacement. I currently cannot see a practical path to get rid of the reflection-module-mess. Any idea anybody ?

Just a head's up, by running all code (in my case two network endpoints) in a JVM (9.0.1) with the command line "--illegal-access=deny" (which denies access to the aforementioned features), everything works as normal, and ofc no warning is issued.

I'm either not understanding that command line correctly or those reflections are not necessary for standard operations?

@RuedigerMoeller you developed something similar, what is it used for? Why do you need reflection on java.nio.DirectByteBuffer's constructor? Why does it work if that access is forbidden?

It might be the case that I have yet to come across a specific serialization type that requires it, but knowing what it is would be helpful!

Generic serialization/deserialization without reflection cannot not work except by relying completely on Unsafe ..

  • FST reuses the default hooks of JDK's built in serialization to avoid the need to write custom serializers for 1000's of classes. This requires reflective access by design.
  • upon deserialization its required to set private fields
  • deserialization needs an 'no-op' constructor, else class initialization (e.g. default values / constructor code) will lead to wrong deserialization results.

DirectByteBuffer

There is no reference to DirectByteBuffer in FST, probably its just happening when trying to figure out a no-op constructor for re-instantiation. What example are you refering to ?

There is no upgrade path except longish '--add-open ' command lines or moving to Unsafe completely.

@RuedigerMoeller I wasn't referring to generic reflection, but specifically that on the ByteBuffer's constructor (which is what is triggering this warning I would assume).

By completely denying access to reflective access on those internal APIs I still can serialize and deserialize just about anything, and I expected something to fail without the ability to access java.nio.DirectByteBuffer's constructor by reflection.

This is what I don't understand, why serialization libraries need reflective access to the ByteBuffer, and why if I completely deny it it still works.

What are you running exactly for testing ? As said there is no reference to DirectByteBuffer in the source, so its an artefact of the generic algorithm (probably no-op constructor lookup) ?

It probably works as it uses Unsafe if present .. however forcing libs to dependent on Unsafe is not exactly the expected outcome of modules :)

Probably denying does not work or there is something wrong with your testing. Turn off use of Unsafe in FST and it will definitiely require reflective access to internal fields in many places. Its just a matter of logic, so I'd rather doubt the testing ;)

Ah .. i see, you are talking of this specific Kryo issue .. i am adressing a wider issue as this won't be the only place where reflection restrictions hit. JDK needs a better way to declare '--add open' than commandline

@RuedigerMoeller I'm running a client/server Kryonet instance with Kryo 4.0.1 as the serializer for it. I move lots of large data structures, although I must admit I try to stick to plain arrays of primitives and primitives only. I think I have just a couple of classes that use lots of POJOs and I serialize them for local persistence.

Either, as you said, that "deny" option is not working or silently defaults to using Unsafe. BTW, I'm not using J9 modules, just 1.8 language level and bytecode running on the JVM9. I wanted to move over to it because of the various improvements (compact strings among others).

If this is as bad as it gets I can count my blessings, at least until java 10 hits. Crossing fingers and hoping somehow both Kryonet and Kryo will survive that!

Hi, I found a way:

  • use unsupported.ReflectionFactory to obtain constructors for externalizable/serializable and readObject/writeObject/readReplace...
  • use unsupported.Unsafe in order to set/get private fields

what's bad:

  • we are now forced to use Unsafe .. (fst could run without "Unsafe" in jdk < 9)
  • the code is not compatible with Android anymore, split of codebase required

regarding DirectByteBuffer: i just stumbled upon (commented code) in fst: the bytebuffer public api does not allow to reset underlying memory buffer, to reduce allocation one could set these private fields by reflection (maybe this is same in kryo)

Hi,
Is anyone actively looking at this? This has been open now for 6 months and JDK 10 has come out during this time. The warning seems to be specifically around DirectByteBuffer; does it need to be accessed reflectively?

magro commented

@ekoutanov I saw the discussion here but didn't start to work on this.

I started to get the build ready for Travis to be able to build/test with jdk 9/10 (in #574).

If you have a suggestion WHAT exactly should be changed for java 9 I'd be happy to hear this, even better would be a PR :-)

Is there an actual problem that needs addressing? Kryo tries unsafe, then falls back to ReflectASM, then falls back to reflection.

Just a head's up, by running all code (in my case two network endpoints) in a JVM (9.0.1) with the command line "--illegal-access=deny" (which denies access to the aforementioned features), everything works as normal, and ofc no warning is issued.

Great, it sounds like everything is fine. If you don't want to see the warning, just turn it off. If the access is denied, things still work. If your serialization relies on access that is denied, you need to do your serialization differently (or don't use a JVM where access is denied).

@NathanSweet , @voidburn
The switch --illegal-access=deny is JVM-wide, affecting all libraries, not just Kryo. And while it's a good thing that Kryo will fall back to an alternate source of reflection, just about any other library that performs illegal access (there are still tons of them out there) will fail.

If you just need the warning suppressed, add the following to your JVM args --add-opens=java.base/java.nio=ALL-UNNAMED. It's always better to be specific when suppressing a warning.

Any latest news on this issue with Java 11?

@HabeebCycle: You currently have two options:

Open up the modules as described above or disable unsafe using -Dkryo.unsafe=false.

@theigl adding the flag is not working for me..any chance this is related to using kryo as a sub dependency of spring state machine? Do you see something wrong on my args?

Screen Shot 2021-07-12 at 16 05 35

@Daanielvb: SSM depends on Kryo 4.0.2 and the kryo.unsafe flag was added in a release candidate for 5.0.0. I can't see from your screenshot which invocation causes the warning, but you probably need to add additional add-opens flags to work around this.

@theigl this is my full stack trace

WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by com.esotericsoftware.kryo.util.UnsafeUtil (file:/$HOME/.gradle/caches/modules-2/files-2.1/com.esotericsoftware/kryo-shaded/4.0.2/e8c89779f93091aa9cb895093402b5d15065bf88/kryo-shaded-4.0.2.jar) to constructor java.nio.DirectByteBuffer(long,int,java.lang.Object) WARNING: Please consider reporting this to the maintainers of com.esotericsoftware.kryo.util.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

I thought I was already using the add-open flags for java.nio properly (as the image from the previous message). Do you know what I may be missing?

@Daanielvb: add-opens has to be a JVM argument and not an environment variable. Try adding it to the beginning of your VM args.