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

Java 9 illegal reflective access warning #543

Closed
guidomedina opened this issue Sep 29, 2017 · 22 comments
Closed

Java 9 illegal reflective access warning #543

guidomedina opened this issue Sep 29, 2017 · 22 comments

Comments

@guidomedina
Copy link

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
@guidomedina guidomedina changed the title Java 9 warning about illegal reflective operation Java 9 illegal reflective access warning Sep 29, 2017
@eolivelli
Copy link

news on this ?

@mrkb80
Copy link

mrkb80 commented Dec 5, 2017

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.

@voidburn
Copy link

voidburn commented Dec 5, 2017

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?

@RuedigerMoeller
Copy link

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 ?

@voidburn
Copy link

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!

@RuedigerMoeller
Copy link

RuedigerMoeller commented Dec 31, 2017

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 ?

@RuedigerMoeller
Copy link

RuedigerMoeller commented Dec 31, 2017

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

@voidburn
Copy link

@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.

@RuedigerMoeller
Copy link

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 ;)

@RuedigerMoeller
Copy link

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

@voidburn
Copy link

voidburn commented Dec 31, 2017

@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!

@RuedigerMoeller
Copy link

RuedigerMoeller commented Jan 3, 2018

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)

@ekoutanov
Copy link

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
Copy link
Collaborator

magro commented Apr 4, 2018

@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 :-)

@NathanSweet
Copy link
Member

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).

@ekoutanov
Copy link

@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.

@HabeebCycle
Copy link

Any latest news on this issue with Java 11?

@theigl
Copy link
Collaborator

theigl commented Nov 9, 2020

@HabeebCycle: You currently have two options:

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

@Daanielvb
Copy link

@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

@theigl
Copy link
Collaborator

theigl commented Jul 12, 2021

@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.

@Daanielvb
Copy link

Daanielvb commented Jul 12, 2021

@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?

@theigl
Copy link
Collaborator

theigl commented Jul 13, 2021

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

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

No branches or pull requests