Replace Unsafe and illegal reflection operations

Gave a quick try to lmdbjava with JDK 9 and wanted to record the findings here.

Due the the class access restrictions imposed by Jigsaw, the following JVM arguments have to be specified:

--add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/sun.nio.ch=ALL-UNNAMED

@phraktle

Thanks @phraktle. Are there any changes you'd suggest to improve Java 9 readiness?

If there was a way to avoid using Unsafe access to ByteBuffers (pushing the address manipulation to the JNI native layer), it would make it easier to adopt this library in the future. However, with the above VM args, it seems to work fine. (Also, if there was proper Jigsaw module meta-data, the use of the generic "ALL-UNNAMED" could be avoided).

Is there going to be any replacement for the sort of Unsafe ByteBuffer manipulations done here? VarHandles perhaps?

Any PRs that improve our Java 9 support are most welcome. Unfortunately I have limited time to work on this at the moment.

I've now had a chance to test LmdbJava with Java 9.

First, the good news:

Some of our Unsafe use can be replaced with com.kenai.jffi.MemoryIO operations
Netty Unpooled now allows allows wrapping a pointer-derived address
Maven running under Java 9 can compile Java 8 while including a Java 9 module-info.java

Next, the bad news:

Violating field encapsulation in ByteBuffer and ByteBuf is no longer permitted
Some Maven plugins (eg dependency analysis) fail (eg ASM class errors)
AppVeyor and Travis macOS do not yet support Java 9

The last two issues aren't major and will probably be resolved once Java 9 is GA.

The former issue is important though, as complying with ByteBuffer and ByteBuf field visibility modifiers will result in allocation and deallocation overhead on each use. On the bright side, we'd be properly using those classes and this would mean easier eventual support for Android. Plus users wouldn't be forced to use --add-opens (which apparently disappears in Java 10 anyway).

Buffers using Agrona and byte[] will continue to operate at the same speed as they presently do. This is because Agrona allows an existing buffer to inexpensively wrap a new address, and byte[] already incurs copying, allocation and deallocation costs. Two of the buffers are therefore unaffected.

How would users feel about ByteBuffer and ByteBuf being used in accordance with their API contracts? This will make them slower, but encapsulation compliance is the only major Java 9 blocker.

This is all in flux IMHO. We don't know how Panama will turn out yet and moving to VarHandles seems complicated from my point of view. Its unfortunate that users will need to add extra options to run lmdbjava in Java 9, but I think patience is the right approach at this moment.

I tweeted Brian Goetz and his advice is to use VarHandles to move away from Unsafe.

https://twitter.com/BrianGoetz/status/843650963370135553

Some notes from the lwjgl guys.

First, lets address the elephant in the room. Yes, we do use Unsafe for this. We're not happy about it either, but there is no alternative. VarHandles in Java 9 will not help, because we're accessing private implementation details. Unless a pure Java alternative is provided in Java 9, this is the best we can do until Java 10 and Project Panama (looks like we'll stop using NIO buffers then anyway).

https://blog.lwjgl.org/memory-management-in-lwjgl-3/

As LWJGL notes, VarHandles do not allow code to arbitrarily access another module's private fields.

I suggest we:

Remove all Unsafe use and replace with MemoryIO. That means no need to requires jdk.unsupported in the module-info.java and no more compiler warnings.
Make ByteBufferProxy.PROXY_SAFE allocate new instances and copy. Copying has precedent with PROXY_BA already. This means ByteBuffer is being used as its API permits.
Either remove ByteBufferProxy.PROXY_OPTIMAL or retain it with the current pattern of illegal reflective setAccessible(true) field access (Java 9 users must --add-opens).
Update the LmdbJava benchmark to quantify the current buffer alternatives.

I don't believe PROXY_OPTIMAL is worth retaining given it would occupy such a narrow niche:

Proxy	Portable (1)	Valid Use (2)	Dependencies	Flags (3)	Fast
BA	Yes	Yes	No	No	No (4, 5, 6)
SAFE	Yes	Yes	No	No	No (4, 5, 6)
OPTIMAL	No	No	No	Yes	No (6, 7)
DB	No	Yes	Yes	No	Yes
NETTY	No	Yes	Yes	No	Yes

(1) Portable across all Java-certified JVMs
(2) LmdbJava correctly using the relevant buffer API
(3) LmdbJava module referenced in Java 9 --add-opens flag
(4) Allocation
(5) Copying
(6) Bounds checking
(7) Reflective field access (address and length)

Just noting here the Java 9 module-info.java found to work for future reference (I won't commit this given it would disrupt the build to exclude this Java file etc):

module org.lmdbjava {
   requires jdk.unsupported;
   requires agrona; 
   requires jffi;
   requires jnr.constants;
   requires jnr.ffi;
   exports org.lmdbjava;
}

There's also another obvious option: implement ByteBuffer creation in JNI native code. Of course this would complicate the binding code, but it does not rely on the hacks that are being phased out in the JDK...

On a related note, JDK9 introduced the --permit-illegal-access global flag, which makes it easier to trace offending code.

I'm fine with the suggestion of removing OPTIMAL (unsafe and reflection) and having ByteBuffers wrap byte arrays instead.

However, i'm not sure what this solves since lmdbjava would still be transitively dependent on Unsafe through jnrffi and Agrona, which means we'll still be stuck on Java 9 unless Project Panama or something else comes along.

I'm not sure what this solves since lmdbjava would still be transitively dependent on Unsafe through jnrffi and Agrona

The main thing it would solve is removing all illegal code from LmdbJava, in turn simplifying our maintenance effort, Java 9 compliance status and Android porting compatibility. Those using Agrona, Netty, safe ByteBuffer and byte[] will see no material performance difference.

I concede JNR-FFI, Agrona and Netty still have to solve these issues, but they should be better positioned to do so given they are popular, low-level libraries.

My broader (non-LmdbJava) testing with Java 9 found countless issues with other applications and libraries as well. As such I have put Java 9 on ice for now and suggest we sit back and see what happens over the next few months. This seems pragmatic anyway given the build and tooling ecosystem isn't there yet (eg Java 9 meaning broken Maven plugins, lack of working CI etc).

Java 9 has been released couple of days ago, so what's the deal?

We're seeing the issue reported in #167 with jdk11
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.lmdbjava.ByteBufferProxy$AbstractByteBufferProxy (file:/var/cache/rchain-build/home-cache/coursier/v1/https/repo1.maven.org/maven2/org/lmdbjava/lmdbjava/0.8.1/lmdbjava-0.8.1.jar) to field java.nio.Buffer.address
WARNING: Please consider reporting this to the maintainers of org.lmdbjava.ByteBufferProxy$AbstractByteBufferProxy
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

Since JDK 16 strong JDK internals encapsulation started and will continue with JDK17.
This also affects upstream libraries like datalevin/datalevin#47 .

The good news might be that some improvements to interact with native code are going to be adopted: https://openjdk.java.net/jeps/412 .

I can confirm that using

--add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/sun.nio.ch=ALL-UNNAMED

is required when you want to use lmdbjava with Java 16. I hope this workaround still works with Java 17 (but I did not try it).

@jubax

@jubax According to https://openjdk.java.net/jeps/403 it should still work:

It will still be possible to use the --add-opens command-line option, or the Add-Opens JAR-file manifest attribute, to open specific packages.

However, migrating off of these packages seems like a good direction.

With each new java version, the migration is looking more and more necessary.

Commit 5687bd6 shows how to add the required JVM parameters for running tests in Maven builds etc. It's implemented as a profile as we presently perform CI builds with Java 8.

https://openjdk.java.net/jeps/412 looks a promising pathway for Java 17 and above, particularly given Java 17 is LTS.

How would people feel about LmdbJava 1.x targeting Java 16 and below, with LmdbJava 2.x requiring Java 17 and above? This would draw a line in the sand and allow use of newer APIs.

I think making the split like that is a very good idea.
It allows the project to move forward while dropping support for old things and should keep users happy.
My +1 for this.

This issue may be 5 years old, but we've been waiting for Project Panama to release long-term, stable APIs. It was only with Java 17 (14 September 2021) did this occur.

My previous comment proposed that LmdbJava 1.x target up to Java 16 and LmdbJava 2.x target Java 17+. I still think that's the most reasonable way forward here, but I have not tried to use the new APIs myself so we may encounter unexpected issues when attempting to.

The practical issue is LmdbJava 2.x is in effect a rewrite to rely on native Java 17+ APIs. This will require a significant investment of time and unfortunately I am unable to commit to doing so in the foreseeable future. On a more positive note, we do have extensive test coverage and CI that will verify the success of any rewrite across the major operating systems with minimal work. If anyone had some time to start on this work I would welcome them to reach out, and I'll help out as I can (eg I'll prioritise native builds on some of the other platforms that have been requested, releasing an LdmbJava 1.0.0 etc).

@benalexau : I think this is a sensible approach.
We would all love for it to be here now but things are as they are.

I believe it would be more sensible to target JDK19 +
https://openjdk.java.net/projects/jdk/19/

JDK19 has JEP 424: Foreign Function & Memory API (Preview) . https://openjdk.java.net/jeps/424 .

I believe it could be made to work with JDK17+ but it might be difficult / not worth it.

I believe it would be more sensible to target JDK19 + ...... has JEP 424: FFM

I believe it could be made to work with JDK17+ but it might be difficult / not worth it.

I agree.

Quoting JEP 424:

We do not propose here to change sun.misc.Unsafe in any way. The FFM API's support for off-heap memory is an excellent alternative to the wrappers around malloc and free in sun.misc.Unsafe, namely allocateMemory, setMemory, copyMemory, and freeMemory. We hope that libraries and applications that require off-heap storage adopt the FFM API so that, in time, we can deprecate and then eventually remove these sun.misc.Unsafe methods.

Based on the above, Unsafe will not be going away any time soon. The present Unsafe opting-in and associated warnings will remain, but even hypothetically rewriting LmdbJava for Java 19+ will still at minimum require command line flags (eg to enable the FFM preview).

Have you considered using JNA? You can access the underlying native memory using their Memory and Pointer classes from native code and handle them as a direct ByteBuffer on the Java side. You could also do something like I did here: https://github.com/KognitionAI/pilecv4j/blob/master/lib-util/src/main/java/ai/kognition/pilecv4j/util/NativePointerWrap.java which will allow users to clean up direct byte buffers rather than relying on the garbage collector/finalize.

JDK24+ will show additional warning for JNI access and JNI will be removed from JDK25+
https://openjdk.org/jeps/498

Closed in response to #259 which provides a concrete pathway forward.

Navigation Menu

Repository navigation

Replace Unsafe and illegal reflection operations #42

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Replace Unsafe and illegal reflection operations #42

Description

Activity

bp-alex commented on Jan 11, 2017

phraktle commented on Jan 12, 2017

scottcarey commented on Feb 6, 2017

bp-alex commented on Feb 6, 2017

bp-alex commented on Mar 17, 2017

krisskross commented on Mar 19, 2017

krisskross commented on Mar 20, 2017

krisskross commented on Mar 20, 2017

bp-alex commented on Mar 21, 2017

bp-alex commented on Mar 21, 2017

phraktle commented on Mar 22, 2017

phraktle commented on Mar 22, 2017

krisskross commented on Mar 22, 2017

bp-alex commented on Mar 25, 2017

DigitalSmile commented on Sep 27, 2017

8 remaining items

9rb commented on Oct 29, 2020

ieugen commented on Jun 3, 2021

jubax commented on Jul 5, 2021

tkorach commented on Jul 15, 2021

maxim5 commented on Aug 15, 2021

bp-alex commented on Aug 17, 2021

ieugen commented on Aug 17, 2021

bp-alex commented on Jun 11, 2022

ieugen commented on Jun 11, 2022

bp-alex commented on Jun 15, 2022

jimfcarroll commented on Dec 6, 2024

tran4o commented on Mar 19, 2025

bp-alex commented on Oct 24, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions