Ah no, static doesn’t work

We need the MultiGetKeys object to hold a vector of byte* which are part of the Slices, but need to be deallocated. That’s what RAII is for..
facebook · Sep 26, 2023 · 5fe9745 · 5fe9745
1 parent 1f49666
commit 5fe9745
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 17 deletions.
diff --git a/java/rocksjni/jni_get_helpers.cc b/java/rocksjni/jni_get_helpers.cc
@@ -107,47 +107,58 @@ jint rocksjni_get_helper(JNIEnv* env, const ROCKSDB_NAMESPACE::FnGet& fn_get,
   return pinnable_value_len;
 }
 
-std::unique_ptr<std::vector<ROCKSDB_NAMESPACE::Slice>> MultiGetKeys::fromByteArrays(JNIEnv* env, jobjectArray jkeys, jintArray jkey_offs,
-                      jintArray jkey_lens) {
+MultiGetKeys::~MultiGetKeys() {
+  for (auto key : key_bufs_to_free) {
+    delete[] key;
+  }
+  key_bufs_to_free.clear();
+}
 
+bool MultiGetKeys::fromByteArrays(JNIEnv* env, jobjectArray jkeys,
+                                  jintArray jkey_offs, jintArray jkey_lens) {
   const jsize num_keys = env->GetArrayLength(jkeys);
 
   std::unique_ptr<jint[]> key_offs = std::make_unique<jint[]>(num_keys);
   env->GetIntArrayRegion(jkey_offs, 0, num_keys, key_offs.get());
   if (env->ExceptionCheck()) {
-    return nullptr;  // exception thrown: ArrayIndexOutOfBoundsException
+    return false;  // exception thrown: ArrayIndexOutOfBoundsException
   }
 
   std::unique_ptr<jint[]> key_lens = std::make_unique<jint[]>(num_keys);
   env->GetIntArrayRegion(jkey_lens, 0, num_keys, key_lens.get());
   if (env->ExceptionCheck()) {
-    return nullptr;  // exception thrown: ArrayIndexOutOfBoundsException
+    return false;  // exception thrown: ArrayIndexOutOfBoundsException
   }
 
-  auto slices = std::make_unique<std::vector<ROCKSDB_NAMESPACE::Slice>>(0);
   for (jsize i = 0; i < num_keys; i++) {
     jobject jkey = env->GetObjectArrayElement(jkeys, i);
     if (env->ExceptionCheck()) {
       // exception thrown: ArrayIndexOutOfBoundsException
-      return nullptr;
+      return false;
     }
 
     jbyteArray jkey_ba = reinterpret_cast<jbyteArray>(jkey);
     const jint len_key = key_lens[i];
     jbyte* key = new jbyte[len_key];
+    key_bufs_to_free.push_back(key);
     env->GetByteArrayRegion(jkey_ba, key_offs[i], len_key, key);
     if (env->ExceptionCheck()) {
       // exception thrown: ArrayIndexOutOfBoundsException
-      delete[] key;
       env->DeleteLocalRef(jkey);
-      return nullptr;
+      return false;
     }
 
-    slices->push_back(
+    slices.push_back(
         ROCKSDB_NAMESPACE::Slice(reinterpret_cast<char*>(key), len_key));
     env->DeleteLocalRef(jkey);
   }
-  return slices;
+  return true;
+}
+
+ROCKSDB_NAMESPACE::Slice* MultiGetKeys::data() { return slices.data(); }
+
+std::vector<ROCKSDB_NAMESPACE::Slice>::size_type MultiGetKeys::size() {
+  return slices.size();
 }
 
 template <class TValue>

diff --git a/java/rocksjni/jni_get_helpers.h b/java/rocksjni/jni_get_helpers.h
@@ -36,11 +36,18 @@ jint rocksjni_get_helper(JNIEnv* env, const ROCKSDB_NAMESPACE::FnGet& fn_get,
  *
  */
 class MultiGetKeys {
+ private:
+  std::vector<ROCKSDB_NAMESPACE::Slice> slices;
+  std::vector<jbyte*> key_bufs_to_free;
 
  public:
-  static std::unique_ptr<std::vector<ROCKSDB_NAMESPACE::Slice>> fromByteArrays(JNIEnv* env, jobjectArray jkeys, jintArray jkey_offs,
-                      jintArray jkey_lens);                  
+  ~MultiGetKeys();
 
+  bool fromByteArrays(JNIEnv* env, jobjectArray jkeys, jintArray jkey_offs,
+                      jintArray jkey_lens);
+
+  ROCKSDB_NAMESPACE::Slice* data();
+  std::vector<ROCKSDB_NAMESPACE::Slice>::size_type size();
 };
 
 /**

diff --git a/java/rocksjni/rocksjni.cc b/java/rocksjni/rocksjni.cc
@@ -1991,16 +1991,16 @@ void multi_get_helper_direct(JNIEnv* env, jobject, ROCKSDB_NAMESPACE::DB* db,
 jobjectArray Java_org_rocksdb_RocksDB_multiGet__J_3_3B_3I_3I(
     JNIEnv* env, jobject, jlong jdb_handle, jobjectArray jkeys,
     jintArray jkey_offs, jintArray jkey_lens) {
+  ROCKSDB_NAMESPACE::MultiGetKeys keys;
 
-  auto keys = ROCKSDB_NAMESPACE::MultiGetKeys::fromByteArrays(env, jkeys, jkey_offs, jkey_lens);
-  if (!keys) {
+  if (!keys.fromByteArrays(env, jkeys, jkey_offs, jkey_lens)) {
     return nullptr;
   }
-  std::vector<ROCKSDB_NAMESPACE::PinnableSlice> values(keys->size());
-  std::vector<ROCKSDB_NAMESPACE::Status> statuses(keys->size());
+  std::vector<ROCKSDB_NAMESPACE::PinnableSlice> values(keys.size());
+  std::vector<ROCKSDB_NAMESPACE::Status> statuses(keys.size());
   auto* db = reinterpret_cast<ROCKSDB_NAMESPACE::DB*>(jdb_handle);
   db->MultiGet(ROCKSDB_NAMESPACE::ReadOptions(), db->DefaultColumnFamily(),
-               keys->size(), keys->data(), values.data(), statuses.data(),
+               keys.size(), keys.data(), values.data(), statuses.data(),
                false /* sorted_input*/);
   return ROCKSDB_NAMESPACE::MultiGetValues::byteArrays(env, values, statuses);
 }