ReflectionAllocator is broken on JDK 16

[Ali-RS]

trafficstars

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

[Ali-RS]

Another idea might be to consider deprecating ReflectionAllocator.

[stephengold]

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

[Ali-RS]

Sorry @stephengold, I thought I have already deleted that comment. I guess it's too late to delete it now but I will adjust it to sense a bit nice;)

You are right deprecating is another topic that should be discussed on the forum.

Regards

[stephengold]

Here's the diagnostic message I got when I ran TestReleaseDirectMemory on 64-bit Linux using OpenJDK 16.0.1:

Dec 08, 2021 12:55:58 PM com.jme3.system.JmeDesktopSystem initialize
INFO: Running on jMonkeyEngine 3.5.0-SNAPSHOT
 * Branch: master
 * Git Hash: 645b1bb
 * Build Date: 2021-12-08
Inconsistency detected by ld.so: dl-lookup.c: 111: check_match: Assertion `version->filename == NULL || ! _dl_name_match_p (version->filename, map)' failed!

[stephengold]

Using OpenJDK 14.0.2, I get additional diagnostic messages:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.jme3.util.ReflectionAllocator (file:/home/sgold/Git/jmonkeyengine/jme3-core/build/libs/jme3-core-3.5.0-SNAPSHOT.jar) to method sun.nio.ch.DirectBuffer.cleaner()
WARNING: Please consider reporting this to the maintainers of com.jme3.util.ReflectionAllocator
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
Dec 08, 2021 1:08:29 PM com.jme3.system.JmeDesktopSystem initialize
INFO: Running on jMonkeyEngine 3.5.0-SNAPSHOT
 * Branch: master
 * Git Hash: 645b1bb
 * Build Date: 2021-12-08
Inconsistency detected by ld.so: dl-lookup.c: 111: check_match: Assertion `version->filename == NULL || ! _dl_name_match_p (version->filename, map)' failed!

After adding the "--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED" JVM argument, the test still crashes similarly with OpenJDK 14.0.2 and even with OpenJDK 11.0.11 . So I don't think this crash has much to do with JDK 16.

[stephengold]

@Ali-RS: I need some help reproducing the "Buffer cannot be destroyed" crash. So far, I haven't even managed to create a buffer with anything other than OpenJDK 8.

[Sailsman63]

Inconsistency detected by ld.so: dl-lookup.c: 111: check_match: Assertion `version->filename == NULL || ! _dl_name_match_p (version->filename, map)' failed!

This looks very familiar. This error is typical of binary incompatibility between native libraries and newer builds of the JRE. Last time this came up, I think the only solution was to use lwjgl3, which was built against the newer runtimes.

There may be more details on the hub.

[Ali-RS]

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

[danielperano]

Chiming in on @stephengold's request.

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

[Ali-RS]

@stephengold were you able to reproduce the "Buffer cannot be destroyed" crash?

[stephengold]

I never got that far.

[Ali-RS]

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. :open_mouth:
  So best to specify it via JVM args.

[riccardobl]

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

[Ali-RS]

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.

[riccardobl]

We already have the jme3-android-native module, the JNI allocator can be added there. Using PrimitiveAllocator might be problematic especially with low memory devices.

[riccardobl]

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?

[Ali-RS]

Is NativeAllocator an interface? or a class that implements the BufferAllocator interface and should exist in all render modules (lwjgl3, lwjgl, android)?

[riccardobl]

A class that implements BufferAllocator

[Ali-RS]

Ok, that sounds good to me.

[pavly-gerges]

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

[Ali-RS]

Thank you @Scrappers-glitch

[pavly-gerges]

Thank you @Scrappers-glitch

Btw, android internals use the same maneuver under android.hardware.HardwareBuffer, so we are on the right path :-) :

The native binding for allocating mem on gpu : https://developer.android.com/reference/android/hardware/HardwareBuffer

The native lib : https://developer.android.com/ndk/reference/group/a-hardware-buffer

Sources at Android cs : https://cs.android.com/android/platform/superproject/+/master:external/mesa3d/include/android_stub/android/hardware_buffer.h;l=46?q=hardware_buffer.h&ss=android

https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/nativewindow/AHardwareBuffer.cpp;l=426;drc=0ef48802e3fbef559287b8b99b6e4f948093bff4?q=HardwareBuffer.c&sq=&ss=android%2Fplatform%2Fsuperproject

[pavly-gerges]

@Ali-RS This may be why you have experienced this bug on JDK-16 and it's absent on other jdks, ReflectionAllocator makes a direct call to sun.* packages, "Internal method signature are subject to change across jdks" : https://www.oracle.com/java/technologies/faq-sun-packages.html

[Sailsman63]

And in particular, there has been a serious push to remove the sun.* packages altogether.

However, if that were the cause you should be getting java.lang.NoClassDefFoundError, unless it's being swallowed somewhere.

[pavly-gerges]

Or guarded against reflective calls...i don't know why generally, but on android they made java reflection and invocation api a black list.

[Sailsman63]

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.

[pavly-gerges]

I am done with Android Native buffer allocator, but there is something which is inconsistent, on com.jme3.utl package in the module jme3-android, you will find similar classes on jme3-core but designed for android but prefixed by Android which is not the case on other packages like com.jme3.input and com.jme3.system, they have their classes inside com.jme3,input.android and com.jme3.system.android, so i know this is another issue but what should i do about the Android native buffer ? Should i just name it AndroidNativeBufferAllocator or should i create a package android under com.jme3.util and keep the name NativeBufferAllocator ?

[Ali-RS]

Based on riccardobl suggestion:

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.

I suppose we should put it under the com.jme3.util package and the name it NativeAllocator or maybe better to name it NativeBufferAllocator.

[pavly-gerges]

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

EDIT : We should never use NativeAllocator as a name, it is vague, might imply different things to allocate rather than a buffer..

I just want to know, to avoid conflict with desktop native buffer allocator, should we name the android allocator as AndroidNativeBufferAllocator or create a new package android under com.jme3.util and keep the name NativeBufferAllocator (but this is an inconsistency with the other classes in the same package, as they all are prefixed with Android to differentiate them from desktop) ?

[pavly-gerges]

However, other android packages like com.jme3.input and system has a sub folder android and the classes inside aren't prefixed by Android.