Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AndroidNativeBufferAllocator memory leak #1990

Open
Ali-RS opened this issue Mar 28, 2023 · 4 comments
Open

AndroidNativeBufferAllocator memory leak #1990

Ali-RS opened this issue Mar 28, 2023 · 4 comments
Labels
Android bug Something that is supposed to work, but doesn't. More severe than a "defect".

Comments

@Ali-RS
Copy link
Member

Ali-RS commented Mar 28, 2023

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 Ali-RS added bug Something that is supposed to work, but doesn't. More severe than a "defect". Android labels Mar 28, 2023
@Ali-RS Ali-RS modified the milestones: Future Release, v3.6.1 Mar 28, 2023
@Ali-RS Ali-RS removed this from the v3.6.1 milestone Jun 4, 2023
@Ali-RS
Copy link
Member Author

Ali-RS commented Jun 4, 2023

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

@Ali-RS Ali-RS added this to the Future Release milestone Jun 5, 2023
@stephengold
Copy link
Member

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

@Ali-RS
Copy link
Member Author

Ali-RS commented Jun 5, 2023

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
Copy link
Contributor

pavly-gerges commented Jun 6, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android bug Something that is supposed to work, but doesn't. More severe than a "defect".
Projects
None yet
Development

No branches or pull requests

3 participants