From 6a8920f988f66fdddf8db5f1fdbd2bbec8254b1a Mon Sep 17 00:00:00 2001 From: Alan Paxton Date: Wed, 14 Dec 2022 10:49:32 -0800 Subject: [PATCH] JNI native memory leak - release array elements (#10981) Summary: Closes https://github.com/facebook/rocksdb/issues/10980 Reproduced as per the suggestion in the ticket, and `$ jcmd VM.native_memory | grep Internal` reports that we are no longer leaking internal memory with the suggested fix. I did the repro in `MultiGetTest.java` which I have optimized imports on. It did not seem helpful to leave the test code around as it would be onerous to build a memory leak reproducer, and regression seems a remote possibility. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10981 Reviewed By: riversand963 Differential Revision: D41498748 Pulled By: ajkr fbshipit-source-id: 8c6dd0d608172879c8bda479c7c9c05c12d34e70 --- java/rocksjni/rocksjni.cc | 9 +++++++++ java/src/test/java/org/rocksdb/MultiGetTest.java | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/java/rocksjni/rocksjni.cc b/java/rocksjni/rocksjni.cc index 23fa604671..a1e0274b4e 100644 --- a/java/rocksjni/rocksjni.cc +++ b/java/rocksjni/rocksjni.cc @@ -1836,6 +1836,10 @@ inline bool keys_from_bytebuffers(JNIEnv* env, jobject jkey = env->GetObjectArrayElement(jkeys, i); if (env->ExceptionCheck()) { // exception thrown: ArrayIndexOutOfBoundsException + // cleanup jkey_off and jkey_len + env->ReleaseIntArrayElements(jkey_lens, jkey_len, JNI_ABORT); + env->ReleaseIntArrayElements(jkey_offs, jkey_off, JNI_ABORT); + return false; } char* key = reinterpret_cast(env->GetDirectBufferAddress(jkey)); @@ -1844,6 +1848,11 @@ inline bool keys_from_bytebuffers(JNIEnv* env, env->DeleteLocalRef(jkey); } + + // cleanup jkey_off and jkey_len + env->ReleaseIntArrayElements(jkey_lens, jkey_len, JNI_ABORT); + env->ReleaseIntArrayElements(jkey_offs, jkey_off, JNI_ABORT); + return true; } diff --git a/java/src/test/java/org/rocksdb/MultiGetTest.java b/java/src/test/java/org/rocksdb/MultiGetTest.java index 323a6b1f40..c391d81f63 100644 --- a/java/src/test/java/org/rocksdb/MultiGetTest.java +++ b/java/src/test/java/org/rocksdb/MultiGetTest.java @@ -11,12 +11,17 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.rocksdb.util.TestUtil; public class MultiGetTest { + @ClassRule + public static final RocksNativeLibraryResource ROCKS_NATIVE_LIBRARY_RESOURCE = + new RocksNativeLibraryResource(); + @Rule public TemporaryFolder dbFolder = new TemporaryFolder(); @Test