jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.82k stars 1.12k forks source link

AndroidNativeBufferAllocator memory leak #1990

Open Ali-RS opened 1 year ago

Ali-RS commented 1 year ago

For regular Java-managed direct buffers, JME does not need to clean them up as the GC will clean them up.

For any memory that Java is not allocating, special support (like PhantomReference) will have to be used to make sure to free the native memory.

I think until we switch to using jme3-alloc (in JME 3.7), for JME 3.6.1 we should either switch to using PrimitiveAllocator on Android or use PhantomReference in AndroidNativeBufferAllocator to free memory. (similar to the jme3-lwjgl3 buffer allocator)

Ali-RS commented 1 year ago

In the below link, you can find out why direct byte buffers allocated via JNI (env->NewDirectByteBuffer()) are not cleaned when GCed but the ones created with ByteBuffer.allocateDirect() are cleaned with GC.

https://stackoverflow.com/a/35364247

stephengold commented 1 year ago

By "cleaned" you mean that the corresponding native objects are freed?

Ali-RS commented 1 year ago

I do not know much about JNI. At this point, my knowledge is limited to what was mentioned in the above link. It says a DirectByteBufer returned by JNI has no Cleaner associated with that to free the native memory but ByteBuffer.allocateDirect() has a Cleaner object associated with it.

The relevant source from DirectByteBuffer I get from OpenJDK 20.

Here is what the cleaner does:

       private static class Deallocator
        implements Runnable
    {

        private long address;
        private long size;
        private int capacity;

        private Deallocator(long address, long size, int capacity) {
            assert (address != 0);
            this.address = address;
            this.size = size;
            this.capacity = capacity;
        }

        public void run() {
            if (address == 0) {
                // Paranoia
                return;
            }
            UNSAFE.freeMemory(address);
            address = 0;
            Bits.unreserveMemory(size, capacity);
        }

    }
   private final Cleaner cleaner;

   public Cleaner cleaner() { return cleaner; }

// Primary constructor
    //
    DirectByteBuffer(int cap) {                   // package-private

        ...

        cleaner = Cleaner.create(this, new Deallocator(base, size, cap));

        ...

    }
pavly-gerges commented 1 year ago

I believe this issue cannot be fixed without hooking JNI implementation to GC, we cannot utilize UnSafe and Bits instead of JNI and GNU stdlib; because we might revert this issue #1678 or similar issues.

I am trying to get the jme-alloc project to a stable release.