[SOLVED] [BUG] Exception when destroying direct memory in android

Hi,

I’ve found the following stack traces:

E/com.jme3.util.BufferUtils(19845): SEVERE {0}
E/com.jme3.util.BufferUtils(19845): java.lang.IllegalArgumentException: Expected receiver of type java.nio.DirectByteBuffer, but got java.nio.ByteBufferAsFloatBuffer
E/com.jme3.util.BufferUtils(19845): at java.lang.reflect.Method.invoke(Native Method)
E/com.jme3.util.BufferUtils(19845): at java.lang.reflect.Method.invoke(Method.java:372)
E/com.jme3.util.BufferUtils(19845): at com.jme3.util.ReflectionAllocator.destroyDirectBuffer(ReflectionAllocator.java:103)
E/com.jme3.util.BufferUtils(19845): at com.jme3.util.BufferUtils.destroyDirectBuffer(BufferUtils.java:1277)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedGeometry.setMaxNumInstances(InstancedGeometry.java:197)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedGeometry.addInstance(InstancedGeometry.java:324)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedNode.addToInstancedGeometry(InstancedNode.java:235)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:289)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:296)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:296)
E/com.jme3.util.BufferUtils(19845): at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:302)
....
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: Grave {0}
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: java.lang.IllegalArgumentException: Expected receiver of type sun.nio.ch.DirectBuffer, but got java.nio.ByteBufferAsFloatBuffer
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at java.lang.reflect.Method.invoke(Native Method)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.util.ReflectionAllocator.destroyDirectBuffer(ReflectionAllocator.java:116)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.util.BufferUtils.destroyDirectBuffer(BufferUtils.java:1277)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedGeometry.setMaxNumInstances(InstancedGeometry.java:197)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedGeometry.addInstance(InstancedGeometry.java:324)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedNode.addToInstancedGeometry(InstancedNode.java:235)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:289)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:296)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:296)
11-09 03:19:02.337 31548 31643 E com.jme3.util.BufferUtils: at com.jme3.scene.instancing.InstancedNode.instance(InstancedNode.java:302)
....

It’s happening at least each time I call instancedNode.instance() in android not in desktop. If I’m not wrong the direct memory is not being freed.

Thanks

Seems related

Do you also get the below message?

com.jme3.renderer.RendererException: Mesh instancing is not supported by the video hardware

Edit:
Please use code block for attaching logs.

I forgot to add some more details to my previous post. I’m using my android-gl3 fork which is still not merged. Instancing works and performs good but I’ve found this exception which I was not aware of.

Yes, the post you linked is somehow related because it’s the same stack trace but was closed because instancing was not supported in android (previously running GLES2).

Now that it’s almost supported (once my PR is merged) we should fix also this issue and I’m not that good at java inner workings to be able to fix it by myself :frowning: That’s why I’m asking. Also I’m worried about if this method ( BufferUtils.destroyDirectBuffer) is being used more in jme3 code and could lead to mem leaks when running any jme3 based app in android.

** I’ve modified my previous post to add the code blocks. Sorry about that

Hmm… seems ByteBuffer is wrapped into a ByteBufferAsFloatBuffer class somewhere? (probably using the ByteBuffer.asFloatBuffer())

And it seems it does no expose a method for freeing or getting the wrapped ByteBuffer. Maybe using reflection we can get the wrapped ByteBuffer and free it up in ReflectionAllocator?

Note ByteBufferAsFloatBuffer does not implement sun.nio.ch.DirectBuffer and is only extending FloatBuffer. That’s the reason for this exception I guess:

java.lang.IllegalArgumentException: Expected receiver of type sun.nio.ch.DirectBuffer, but got java.nio.ByteBufferAsFloatBuffer

Seems this is a complete mess by Android :wink:

Looking further seems they changed it since 2015 in this commit:

https://android.googlesource.com/platform/libcore/+/46edfaf

Please see the deleted files.

Edit:

@joliver82 I guess you can pass this exception by switching to PrimitiveAllocator

Just need to add this into your Main class I guess

static {
    System.setProperty(BufferAllocatorFactory.PROPERTY_BUFFER_ALLOCATOR_IMPLEMENTATION, PrimitiveAllocator.class.getName());
}

Thanks @Ali_RS for the detailed analysis of the issue.

Although using PrimitiveAllocator will bypass the exception it won’t free the memory at all. I’ll try to modify the ReflectionAllocator to get the inner ByteBuffer and free it properly instead.

I’ll get back to you once I have something :wink:

1 Like

It is done automatically by the GC. You can find more info here:

Hmm… maybe instead of modifying ReflectionAllocator to support Android mess we better to add a new AndroidBufferAllocater in jme3-android project. What do you think?

Then we can set it to be the default for Android in OGLESContext:

Similar to what we are doing for LWJGL3:

@pspeed, @RiccardoBlb any thought?

Yes, I know the memory will be GCed automatically later. So… why to implement ReflectionAllocator.destroyDirectBuffer? If I’m not wrong, not manually releasing the memory could generate some random OOMs (issue that I’m facing in my app).

Yes, it could be even better trying not to mess with it and make an android specific allocator instead but for old android versions (<5.0) we should keep using the reflection method because java.nio.DirectBuffer<[R]S|U> classes don’t have an inner ByteBuffer and implement sun.nio.ch.DirectBuffer

One additional comment, reading the history of ReflectionAllocator, there was an attempt to improve it for android but that code was removed (latest two commits):

1 Like

I’ve implemented a simple AndroidAllocator that avoids the exception and really releases the memory as follows:

package com.jme3.util;

import java.lang.reflect.Field;
import java.nio.Buffer;
import java.nio.ByteBuffer;


public final class AndroidAllocator implements BufferAllocator {
    // We make use of the ReflectionAllocator to remove the inner buffer
    private static final ReflectionAllocator reflectionAllocator = new ReflectionAllocator();

    private static final String [] possibleBufferClassNames =
                {
                        "java.nio.ByteBufferAsFloatBuffer",
                        "java.nio.ByteBufferAsIntBuffer",
                        "java.nio.ByteBufferAsDoubleBuffer",
                        "java.nio.ByteBufferAsShortBuffer",
                        "java.nio.ByteBufferAsLongBuffer",
                        "java.nio.ByteBufferAsCharBuffer",
                };
    private static final String [] possibleBufferFieldNames = {"bb", "byteBuffer"};

    @Override
    /**
     * This function search the inner direct buffer of the android specific wrapped buffer classes
     * and destroys it using the reflection allocator method.
     * 
     * @param toBeDestroyed
     *            The direct buffer that will be "cleaned".
     * 
     */
    public void destroyDirectBuffer(Buffer toBeDestroyed) {
        boolean freed=false;

        // loop for all possible android buffer classes
        for(int i=0 ; i<possibleBufferClassNames.length && !freed ; ++i) {
            String className="";
            try {
                className=possibleBufferClassNames[i];
                Class clazz = Class.forName(className);

                // we found the proper class, get the inner buffer field
                if (clazz.isInstance(toBeDestroyed)) {
                    // loop for all possible field names in android
                    for (int j=0 ; j<possibleBufferFieldNames.length && ! freed ; ++j) {
                        String fieldName = possibleBufferFieldNames[j];
                        try {
                            Field field = clazz.getDeclaredField(fieldName);
                            field.setAccessible(true);
                            ByteBuffer innerBuffer = (ByteBuffer)field.get(toBeDestroyed);

                            // if the field is found we it'll be freed or it's null so we marked it as free first
                            freed=true;
                            if(innerBuffer!=null)
                            {
                                // Destroy it using the reflection method
                                reflectionAllocator.destroyDirectBuffer(innerBuffer);
                            }
                        } catch (NoSuchFieldException e) {
                        } catch (IllegalAccessException e) {
                        }
                    }
                }
            } catch (ClassNotFoundException e) {
            }
        }

        // If we could not free it using previous reflection we're running a really old android (<5.0)
        // use default reflection allocator to remove it instead
        if(!freed)
        {
            reflectionAllocator.destroyDirectBuffer(toBeDestroyed);
        }
    }

    @Override
    public ByteBuffer allocate(int size) {
        return ByteBuffer.allocateDirect(size);
    }

}

And included it as default in OGLESContext :

diff --git a/jme3-android/src/main/java/com/jme3/system/android/OGLESContext.java b/jme3-android/src/main/java/com/jme3/system/android/OGLESContext.java
index d22995e..8624bcf 100644
--- a/jme3-android/src/main/java/com/jme3/system/android/OGLESContext.java
+++ b/jme3-android/src/main/java/com/jme3/system/android/OGLESContext.java
@@ -60,6 +60,8 @@ import com.jme3.renderer.opengl.GLFbo;
 import com.jme3.renderer.opengl.GLRenderer;
 import com.jme3.renderer.opengl.GLTracer;
 import com.jme3.system.*;
+import com.jme3.util.AndroidAllocator;
+import com.jme3.util.BufferAllocatorFactory;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -82,6 +84,14 @@ public class OGLESContext implements JmeContext, GLSurfaceView.Renderer, SoftTex
     protected long minFrameDuration = 0;                   // No FPS cap
     protected long lastUpdateTime = 0;
 
+    static {
+        final String implementation = BufferAllocatorFactory.PROPERTY_BUFFER_ALLOCATOR_IMPLEMENTATION;
+
+        if (System.getProperty(implementation) == null) {
+            System.setProperty(implementation, AndroidAllocator.class.getName());
+        }
+    }
+
     public OGLESContext() {
     }

I’ve tested it and I get much less OOMs while running my app in android than before (10 minute playing to 30minute playing). Yes, I may have some project code leaks also :thinking: :confused:

I’m thinking on adding this also to the current android PR not merged already. What do you think?

1 Like

Thanks, @joliver82

Probably better to make a new PR, please.

I did a bit of clean up and renamed it to AndroidBufferAllocator:

package com.jme3.util;

import java.lang.reflect.Field;
import java.nio.Buffer;
import java.nio.ByteBuffer;
import java.util.HashMap;
import java.util.Map;

/**
 * @author Jesus Oliver
 */
public class AndroidBufferAllocator implements BufferAllocator {

    // We make use of the ReflectionAllocator to remove the inner buffer
    private static final ReflectionAllocator reflectionAllocator = new ReflectionAllocator();

    private static final String[] wrapperClassNames = {
            "java.nio.ByteBufferAsFloatBuffer",
            "java.nio.ByteBufferAsIntBuffer",
            "java.nio.ByteBufferAsDoubleBuffer",
            "java.nio.ByteBufferAsShortBuffer",
            "java.nio.ByteBufferAsLongBuffer",
            "java.nio.ByteBufferAsCharBuffer",
    };
    private static final String[] possibleBufferFieldNames = {"bb", "byteBuffer"};

    // Keep track of ByteBuffer field by the wrapper class
    private static final Map<Class, Field> fieldIndex = new HashMap<>();

    static {
        for (String className : wrapperClassNames) {
            try {
                Class clazz = Class.forName(className);

                // loop for all possible field names in android
                for (String fieldName : possibleBufferFieldNames) {
                    try {
                        Field field = clazz.getDeclaredField(fieldName);
                        field.setAccessible(true);
                        fieldIndex.put(clazz, field);
                        break;
                    } catch (NoSuchFieldException e) {
                    }
                }
            } catch (ClassNotFoundException ex) {
            }
        }
    }

    @Override
    /**
     * This function search the inner direct buffer of the android specific wrapped buffer classes
     * and destroys it using the reflection allocator method.
     *
     * @param toBeDestroyed The direct buffer that will be "cleaned".
     *
     */
    public void destroyDirectBuffer(Buffer toBeDestroyed) {
        // If it is a wrapped buffer, get it's inner direct buffer field and destroy it
        Field field = fieldIndex.get(toBeDestroyed.getClass());
        if (field != null) {
            try {
                ByteBuffer innerBuffer = (ByteBuffer) field.get(toBeDestroyed);
                if (innerBuffer != null) {
                    // Destroy it using the reflection method
                    reflectionAllocator.destroyDirectBuffer(innerBuffer);
                }
            } catch (IllegalAccessException ex) {
            }

        } else {
            // It is not a wrapped buffer, use default reflection allocator to remove it instead.
            reflectionAllocator.destroyDirectBuffer(toBeDestroyed);
        }
    }

    @Override
    public ByteBuffer allocate(int size) {
        return ByteBuffer.allocateDirect(size);
    }
}

Are you Ok with these changes?
(Note I have not tested it on my side)

Your changes look really good. I’ll check it later today and get back with the results. Thanks

1 Like

Tested and working great. I’ve just created a PR for this: https://github.com/jMonkeyEngine/jmonkeyengine/pull/1214

2 Likes

Thanks so much for the PR. Let’s keep it open for a few days in case someone wants to comment.

Ok, great! A quick comment although obviously it’s not related but mac natives compilation is failing…

Yes, I am aware of it, I have no idea why it is happening.

We used a pretty old version of xcode to compile for 32bit, it has now been deprecated and removed from the latest macOs, on top of that, it was signed with old apple certificates that expired.
I’m working on it.

2 Likes