jmonkeyengine: ReflectionAllocator is broken on JDK 16

SEVERE: Buffer cannot be destroyed: java.nio.DirectFloatBufferU

Even by adding this JVM arg (which used to solve that on jdk 11+)

“–add-opens=java.base/jdk.internal.ref=ALL-UNNAMED”

you can test it with

https://github.com/jMonkeyEngine/jmonkeyengine/blob/d76d4dc8c67bc99c5fd197d41a102639194a4e3b/jme3-examples/src/main/java/jme3test/app/TestReleaseDirectMemory.java#L43

Forum post: https://hub.jmonkeyengine.org/t/jme-on-jdk-16/44411/6?u=ali_rs

About this issue

  • Original URL
  • State: open
  • Created 3 years ago
  • Reactions: 1
  • Comments: 76 (76 by maintainers)

Most upvoted comments

Before we build our own native dependency to only call malloc we should really just use one of the widely available allocators already.

One that is available to jwgl3 is jemalloc: https://javadoc.lwjgl.org/org/lwjgl/system/jemalloc/JEmalloc.html There’s also https://netty.io/wiki/using-as-a-generic-library.html Also considering that the only reason we have the ReflectionAllocator is that we don’t want to wait for the direct buffer to be freed by the Garbage Collector (which is because an access to an already freed byte buffer may be catastrophic). This is still a very active topic in the java world (https://stackoverflow.com/questions/3496508/deallocating-direct-buffer-native-memory-in-java-for-jogl)

Especially the hint to project panama is interesting, if you have a completely new API also supporting try-with-resources. Quoting https://openjdk.org/jeps/370:

Moreover, working with direct buffers can be cumbersome since deallocation of the memory associated with direct buffers is left to the garbage collector; that is, only after a direct buffer is deemed unreachable by the garbage collector can the associated memory be released. […] While using JNI to access memory is also a possibility, the inherent costs associated with this solution make it seldom applicable in practice. The overall development pipeline is complex, since JNI requires the developer to write and maintain snippets of C code. JNI is also inherently slow, since a Java-to-native transition is required for each access.

And most importantly:

Alternatives Keep using existing APIs such as ByteBuffer and Unsafe or, worse, JNI.

-> So the Java Devs consider JNI (us wrapping malloc) worse than unsafe/byte buffer. Keep in mind a java allocator could allocate bigger byte buffers manually and then only giving out chunks. But then, we only use it for Megabyte Sized Textures (well and small vert/index buffers… Could probably also pool them)

So in general, I can’t recommend anything right now, but if our buffers are mainly to communicate with openGL, memory mapping with a new API may be interesting. And a specialized allocator may make a difference, too. With malloc I don’t know how thread safety looks like etc and you’d still need to get the native address of the byte buffer and try to wrap it etc.

Furthermore, I wouldn’t recommend cmake for something so simple, since you need visual studio and the cmake workload on windows, on mac os probably XCode (my mac can’t even use any recent XCode anymore because it’s “old”) etc. If it only targets CI, may even cross compile on linux (or have very simple manual gcc calls), or try out the Gradle Support for C++ projects, as seems common on Android anyway.

Note, I am talking about this issue:

Sometimes the order of these static executions is different, e.g the one in the BufferUtils runs before the LwjglContext, which will cause BufferAllocator allocator to get initialized with Reflectionallocator and bypass the LWJGL3 allocator. open_mouth So best to specify it via JVM args.

riccardobl sugested to use a different approach to solve the issue of staticly setting allocator in code :

It seems we should find a different way to set the allocator, maybe the renderers should have the buffer allocator under the same class name and namespace so that when the modules are switched it gets replaced without setting it “manually”.

maybe an enhancement to this would be to config the allocator name via properties file (for each backend) instead of using a fixed class name for all allocators.

So that BufferAllocatorFactory will read the property PROPERTY_BUFFER_ALLOCATOR_IMPLEMENTATION from the properties file.

I agree with Pavl, that’s just workarounds and ultimately this may stop working, the JRE already moved from a warning to an error. So we’d need to find different allocators. Without knowing what it’s exact use is, there are probably a lot of different allocators to choose from, so we should do the research work here

but this solution can be an alternative for natives on desktop ?

Sorry not sure if I understand what you mean. LwjglBufferAllocator in jme3-lwjgl3 is already using the native method (via JNI through lwjgl3) and we are not going to replace it.

But if you are talking about lwjgl2, it does not use a native allocator, it uses RefectionAllocator, so yes we can write a native allocator for lwjgl2 to replace the RefectionAllocator, just like what you did for android.

The idea of a NativeAllocator seems good to me too, i will try to implement it for android.

Does anyone know if it is used outside of the lwjgl2 renderer?

Yes, AndroidBufferAllocator uses it. It is error-prone on android as well. See https://hub.jmonkeyengine.org/t/androidbufferallocator-and-nonsdkapiusedviolation/45124/2?u=ali_rs

For Android, a workaround may be to write a JNI-based allocator that uses c++ to crete and destroy DirectByteBuffer. Something like:

JNIEXPORT jobject JNICALL allocate(JNIEnv* env, jclass clazz, jint numBytes) {

   char* ptr = (char*)malloc(numBytes);
   return env->NewDirectByteBuffer(ptr, numBytes);
}

JNIEXPORT void JNICALL destroyDirectBuffer(JNIEnv* env, jobject thiz, jobject bufferRef)
{
    void* buffer = env->GetDirectBufferAddress(bufferRef);
    free(buffer);
}

another workaround is to use PrimitiveAllocator which does nothing and lets GC handle the buffer cleanup.

A note for anyone who wants to try this with LWJGL3:

LWJGL3 will statically register its own buffer allocator at LwjglContext:

https://github.com/jMonkeyEngine/jmonkeyengine/blob/ceb356462aeedf7769dced52e1d5572b01deca31/jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglContext.java#L93-L104

which will be statically initialized at this line into the final field allocator:

https://github.com/jMonkeyEngine/jmonkeyengine/blob/ceb356462aeedf7769dced52e1d5572b01deca31/jme3-core/src/main/java/com/jme3/util/BufferUtils.java#L65

So you must make sure to register ReflectionAllocator before the static block run, for example, you can specify it as JVM argument in Gradle build:

'-Dcom.jme3.BufferAllocatorImplementation=com.jme3.util.ReflectionAllocator'
  • Off-topic: Sometimes the order of these static executions is different, e.g the one in the BufferUtils runs before the LwjglContext, which will cause BufferAllocator allocator to get initialized with Reflectionallocator and bypass the LWJGL3 allocator. 😮
    So best to specify it via JVM args.

Note that the jme3-alloc library should only contain the code and natives required for buffer allocation and deallocation. It should not contains other stuff related to physics, image loading,…!

and I’m unclear how much time remains before the JME 3.6 code freeze.

It can be a separate project independent of JME releases.

  • Libbulletjme is a huge amount of code (253K lines of source, 370KB class JAR + a 3 MB native for each platform). I sincerely doubt JMonkeyEngine wants it as a dependency.
  • Minie and Libbulletjme define many of the same Java classes. Adding Libbulletjme as a dependency of JMonkeyEngine would break Minie in ways that would be difficult to fix.
  • While Libbulletjme implements automatic deallocation of (native) physics objects, it doesn’t do so for direct buffers. For buffers, it uses a copy of JMonkeyEngine’s PrimitiveAllocator—it never deallocates direct buffers!

I’m willing to create a new library for buffer allocation and deallocation, using a weak-reference tracking mechanism similar to what Libbulletjme uses for physics objects. However, I don’t have a clear sense of how long it would take, and I’m unclear how much time remains before the JME 3.6 code freeze.

Just my two cents!

If you think it is possible to rip only the required java code + natives from lwjgl3 then go that way but if you think creating our own thin API around malloc/calloc would be easier, then do that way, or if you think it would be easy to use GetDirectBufferAddress from lwjgl2 code to get java ByteBufer memory address and destroy it with stdlib then use that.

You can use whatever build tool you are familiar with it does not matter. What matters is that we want a jme3-alloc.jar with natives inside and a java class that implements BufferAllocator:

https://github.com/jMonkeyEngine/jmonkeyengine/blob/4db02fae9dd8ebb213b3a06316a53019ef9b620f/jme3-core/src/main/java/com/jme3/util/BufferAllocator.java#L6-L24

The jar should be hosted on maven central (either under org.jmonkeyengine or @Scrappers-glitch own organization) and then we will add a dependency to it in jme3-lwjgl.

Simillar to what @stephengold did with https://github.com/stephengold/j-ogg-all/ and https://github.com/stephengold/stack-alloc

Edit:

You can also take a look at how LibGdx does this in there buffer allocator: https://github.com/libgdx/libgdx/blob/28223b6169a99ab063b2b253fb0b1ebf264d0eca/gdx/src/com/badlogic/gdx/utils/BufferUtils.java#L559-L567

if using jemalloc will be enough then let me know.

I am okay using lwjgl3 jemalloc lib. Can we use it without depending on lwjgl3-core?

I think it cannot, this is a script generated by lwjgl.org/customize.

import org.gradle.internal.os.OperatingSystem

project.ext.lwjglVersion = "3.3.1"

switch (OperatingSystem.current()) {
	case OperatingSystem.LINUX:
		def osArch = System.getProperty("os.arch")
		project.ext.lwjglNatives = osArch.startsWith("arm") || osArch.startsWith("aarch64")
			? "natives-linux-${osArch.contains("64") || osArch.startsWith("armv8") ? "arm64" : "arm32"}"
			: "natives-linux"
		break
	case OperatingSystem.MAC_OS:
		project.ext.lwjglNatives = System.getProperty("os.arch").startsWith("aarch64") ? "natives-macos-arm64" : "natives-macos"
		break
	case OperatingSystem.WINDOWS:
		def osArch = System.getProperty("os.arch")
		project.ext.lwjglNatives = osArch.contains("64")
			? "natives-windows${osArch.startsWith("aarch64") ? "-arm64" : ""}"
			: "natives-windows-x86"
		break
}

repositories {
	mavenCentral()
}

dependencies {
	implementation platform("org.lwjgl:lwjgl-bom:$lwjglVersion")

	implementation "org.lwjgl:lwjgl"
	implementation "org.lwjgl:lwjgl-jemalloc"
	runtimeOnly "org.lwjgl:lwjgl::$lwjglNatives"
	runtimeOnly "org.lwjgl:lwjgl-jemalloc::$lwjglNatives"
}

No problem, take your time and ask it on the forum if you need help.

Once all architectures are built into jme-alloc.jar, other tasks like uploading to sonatype will be simple…

CMake is just a simple build script that you can use to generate native system build files and it’s callable from gradle task with Runtime.getRuntime().exec() could be used to call the cmake script to build a shared object file before including it in the output jme-alloc.jar, I am not yet sure about how other architectures should be handled, you may give me some time to model this.

@Scrappers-glitch I think we better create it separate from the lwjgl2 project. (e.g. in a jme3-calloc project hosted on JME Github organization) You can copy whatever code you need from lwjgl2. I guess you might have a hard time tackling with lwjgl2 native builds for different OS. Our lwjgl2 build workflow uses precompiled natives.

I would like to know @stephengold opinion about this as well.

Looks clean, i don’t mind to create a new API, however i am not so far good at building natives for cross-platform (i only do linux arches and android), so this might take a lot of time, i am currently learning cmake if it would bring something new to the engine, then let me know…

EDIT: For cmake, you will just have to run the build script and it will generate a native system build file (makefile or visual studio solution) that will use the native compiler for the build process, I think there will be no special configuration, we can create a dummy project to test it with github actions before doing anything.

@Scrappers-glitch I think we better create it in a stand alone project separate from the lwjgl2 project . (e.g. in a jme3-calloc project hosted on JME Github organization) so it can be used outside of lwjgl2 too. You can copy whatever native code you need from lwjgl2. I guess you might have a hard time tackling with lwjgl2 native builds for different OS. Our lwjgl2 build workflow uses precompiled natives.

Not sure though, I would like to know @stephengold opinion about this as well.

So we’d need to find different allocators. Without knowing what it’s exact use is, there are probably a lot of different allocators to choose from, so we should do the research work here

I think for android, there was a discussion around that, and “calloc” is suggested by the core team.

#1821 (comment)

Yes, because calloc = malloc + evaluating the buffer to NULL or zero (memset(0)).

We should track lwjgl2 native header files to decide whether there is already a destroy criteria there (it must be there), however I cannot find any header file inside lwjgl2-jme ? I hope they are not inside the shared object files (which is a bad practice)…I will try to track them down…

Yeah, I will be happy to work on this, but I think if Lwjgl already (which I think it really does) provides a java implementation for a native allocator, we might use it.

After diving more and adding some println to the suppressed exceptions I gathered some more info about error:

java.lang.reflect.InaccessibleObjectException: Unable to make public abstract jdk.internal.ref.Cleaner sun.nio.ch.DirectBuffer.cleaner() accessible: module java.base does not "exports sun.nio.ch" to unnamed module @4bca8b50
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
	at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
	at com.jme3.util.ReflectionAllocator.loadMethod(ReflectionAllocator.java:84)
	at com.jme3.util.ReflectionAllocator.<clinit>(ReflectionAllocator.java:63)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)
	at com.jme3.util.BufferAllocatorFactory.create(BufferAllocatorFactory.java:30)
	at com.jme3.util.BufferUtils.<clinit>(BufferUtils.java:65)
	at com.jme3.renderer.lwjgl.LwjglGL.<init>(LwjglGL.java:15)
	at com.jme3.system.lwjgl.LwjglContext.initContext(LwjglContext.java:283)
	at com.jme3.system.lwjgl.LwjglContext.initContextFirstTime(LwjglContext.java:265)
	at com.jme3.system.lwjgl.LwjglContext.internalCreate(LwjglContext.java:454)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:124)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:221)
	at java.base/java.lang.Thread.run(Thread.java:833)
java.lang.ClassNotFoundException: sun.misc.Cleaner
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)
	at com.jme3.util.ReflectionAllocator.loadMethod(ReflectionAllocator.java:83)
	at com.jme3.util.ReflectionAllocator.<clinit>(ReflectionAllocator.java:64)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)
	at com.jme3.util.BufferAllocatorFactory.create(BufferAllocatorFactory.java:30)
	at com.jme3.util.BufferUtils.<clinit>(BufferUtils.java:65)
	at com.jme3.renderer.lwjgl.LwjglGL.<init>(LwjglGL.java:15)
	at com.jme3.system.lwjgl.LwjglContext.initContext(LwjglContext.java:283)
	at com.jme3.system.lwjgl.LwjglContext.initContextFirstTime(LwjglContext.java:265)
	at com.jme3.system.lwjgl.LwjglContext.internalCreate(LwjglContext.java:454)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:124)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:221)
	at java.base/java.lang.Thread.run(Thread.java:833)
java.lang.NoSuchMethodException: sun.nio.ch.DirectBuffer.viewedBuffer()
	at java.base/java.lang.Class.getMethod(Class.java:2227)
	at com.jme3.util.ReflectionAllocator.loadMethod(ReflectionAllocator.java:83)
	at com.jme3.util.ReflectionAllocator.<clinit>(ReflectionAllocator.java:65)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)
	at com.jme3.util.BufferAllocatorFactory.create(BufferAllocatorFactory.java:30)
	at com.jme3.util.BufferUtils.<clinit>(BufferUtils.java:65)
	at com.jme3.renderer.lwjgl.LwjglGL.<init>(LwjglGL.java:15)
	at com.jme3.system.lwjgl.LwjglContext.initContext(LwjglContext.java:283)
	at com.jme3.system.lwjgl.LwjglContext.initContextFirstTime(LwjglContext.java:265)
	at com.jme3.system.lwjgl.LwjglContext.internalCreate(LwjglContext.java:454)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:124)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:221)
	at java.base/java.lang.Thread.run(Thread.java:833)
java.lang.reflect.InaccessibleObjectException: Unable to make public abstract java.lang.Object sun.nio.ch.DirectBuffer.attachment() accessible: module java.base does not "exports sun.nio.ch" to unnamed module @4bca8b50
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
	at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
	at com.jme3.util.ReflectionAllocator.loadMethod(ReflectionAllocator.java:84)
	at com.jme3.util.ReflectionAllocator.<clinit>(ReflectionAllocator.java:68)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)
	at com.jme3.util.BufferAllocatorFactory.create(BufferAllocatorFactory.java:30)
	at com.jme3.util.BufferUtils.<clinit>(BufferUtils.java:65)
	at com.jme3.renderer.lwjgl.LwjglGL.<init>(LwjglGL.java:15)
	at com.jme3.system.lwjgl.LwjglContext.initContext(LwjglContext.java:283)
	at com.jme3.system.lwjgl.LwjglContext.initContextFirstTime(LwjglContext.java:265)
	at com.jme3.system.lwjgl.LwjglContext.internalCreate(LwjglContext.java:454)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:124)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:221)

I found out on java 16+ in addition to

"--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED" 

I must also add

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

then error SEVERE: Buffer cannot be destroyed: java.nio.DirectFloatBufferU will go away.

Let’s see when it will break again!

By the way, I am not sure if riccardobl approach is the best way or if we can use a better alternative. But this is another topic and we need to discuss this on the forum.

I am ok with either of these namings:

com.jme3.util.AndroidNativeBufferAllocator

or

com.jme3.util.NativeBufferAllocator

Would it create a conflict when we build the NativeBufferAllocator on desktop because both are at the same package com.jme3.util ?

No, because at the runtime only one of these modules (either lwjgl or android) will be in classpath. On desktop only jme3-lwjgl3 module will be loaded and on android only jme3-android module will be loaded.

i don’t know why generally

Mostly security. Reflection allows access to APIs that may not be safe for general code to use. Even desktop Java is moving to block reflection to a lot of internal packages. But in that case, you should be getting IllegalReflectiveAccess errors. Again, unless they are being swallowed by code.

A class that implements BufferAllocator

It seems we should find a different way to set the allocator, maybe the renderers should have the buffer allocator under the same class name and namespace so that when the modules are switched it gets replaced without setting it “manually”.

To elaborate on this:

The first part of my proposal is to have a NativeAllocator that is implemented by every rendered module internally (eg. lwjgl3 will use its allocator, lwjgl2 will use ReflectionAllocator, android will use the jni allocator) with the same class on the same path: com.jme3.util.NativeAllocator . In this way when a different renderer module is used its native allocator is loaded.

The next step of the solution is to have BufferAllocatorFactory load the allocator that is specified with the property PROPERTY_BUFFER_ALLOCATOR_IMPLEMENTATION like it does now, but with the default always being com.jme3.util.NativeAllocator, and if the specified class doesn’t exist make it fallback to PrimitiveAllocator.

That should solve or provide a path to solve every possible issue. What do you think?

The ReflectionAllocator was an hack to begin with. Does anyone know if it is used outside of the lwjgl2 renderer? If not i assume it will be deprecated with lwjgl2 at some point.

Sometimes the order of these static executions is different, e.g the one in the BufferUtils runs before the LwjglContext, which will cause BufferAllocator allocator to get initialized with Reflectionallocator and bypass the LWJGL3 allocator. open_mouth So best to specify it via JVM args.

It seems we should find a different way to set the allocator, maybe the renderers should have the buffer allocator under the same class name and namespace so that when the modules are switched it gets replaced without setting it “manually”.

Chiming in on @stephengold’s request.

I’m running on an Oracle build of JDK 16 with LWJGL 3 with no issues.

@stephengold can you try with AdoptOpenJDK?

AFAIK LWJGL2 is not compatible with OpenJDK 12+ (also with some versions of OpenJDK 11, IIRC 11.0.6+)

Otherwise, I think you should use LWJGL3 as mentioned by Sailsman63.

I’m sorry I did not clarify this in advance.

Deprecating ReflectionAllocator is another idea that should be discussed at the Forum/Hub ASAP.