`multiGet` ByteBuffer output helper accumulates local references

[K-ANOY]

File: java/rocksjni/jni_multiget_helpers.cc

Function: MultiGetJNIValues::fillByteBuffersAndStatusObjects

The ByteBuffer-output multiGet helper creates local references inside the result loop and stores some of them into Java arrays, but does not delete the temporary local references before continuing to the next result.

Relevant code:

for (int i = 0; i < static_cast<jint>(values.size()); i++) {
  auto jstatus = ROCKSDB_NAMESPACE::StatusJni::construct(env, s[i]);
  if (jstatus == nullptr) {
    // exception in context
    return;
  }
  env->SetObjectArrayElement(jstatuses, i, jstatus);

  if (s[i].ok()) {
    jobject jvalue_bytebuf = env->GetObjectArrayElement(jvalues, i);
    if (env->ExceptionCheck()) {
      // ArrayIndexOutOfBoundsException is thrown
      return;
    }
    jlong jvalue_capacity = env->GetDirectBufferCapacity(jvalue_bytebuf);
    if (jvalue_capacity == -1) {
      ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(
          env,
          "Invalid value(s) argument (argument is not a valid direct "
          "ByteBuffer)");
      return;
    }
    void* jvalue_address = env->GetDirectBufferAddress(jvalue_bytebuf);
    if (jvalue_address == nullptr) {
      ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(
          env,
          "Invalid value(s) argument (argument is not a valid direct "
          "ByteBuffer)");
      return;
    }
    ...
  }
}

StatusJni::construct() returns a local reference to a Java Status object. SetObjectArrayElement() stores that object in jstatuses, but it does not consume or delete the local reference. Similarly, GetObjectArrayElement() returns a new local reference to the caller-supplied ByteBuffer in jvalues. That jvalue_bytebuf reference is used to query the direct-buffer capacity and address, but it is never deleted.

The helper is used by the JNI binding for ByteBuffer-based RocksDB.multiGet:

ROCKSDB_NAMESPACE::MultiGetJNIValues::fillByteBuffersAndStatusObjects(
    env, values, statuses, jvalues, jvalue_sizes, jstatuses);

Both references are reclaimed when the native method returns, so this is not a leak that persists across calls. The reason it is worth fixing is that the loop bound is the caller's batch size — the whole point of multiGet is to pass many keys at once — so the call keeps one Status reference per result plus one ByteBuffer reference per successful result live simultaneously. There is no PushLocalFrame() anywhere in java/rocksjni/, so nothing bounds the growth, and a large enough batch will produce -Xcheck:jni warnings or local-reference allocation failure.

The same file already establishes the correct pattern. MultiGetJNIValues::byteArrays() is the sibling helper that fills the byte-array form of the same multiGet results, and it deletes the temporary reference on both the exception path and the success path:

env->SetObjectArrayElement(jresults, static_cast<jsize>(i), jentry_value);
if (env->ExceptionCheck()) {
  // exception thrown:
  // ArrayIndexOutOfBoundsException
  env->DeleteLocalRef(jentry_value);
  return nullptr;
}

env->DeleteLocalRef(jentry_value);

Two helpers in the same class, filling results for the same API, disagree on whether the per-iteration reference is deleted, which is why the omission in fillByteBuffersAndStatusObjects looks accidental rather than deliberate.

There is also an inline StatusJni::construct(...) call in the "result too large" branch:

env->SetObjectArrayElement(
    jstatuses, i,
    ROCKSDB_NAMESPACE::StatusJni::construct(
        env, Status::Incomplete("result too large to represent")));

That constructed Status local reference has the same ownership issue. Its result is also not null-checked, unlike the jstatus at the top of the loop, so on construction failure it passes nullptr into SetObjectArrayElement() with an exception already pending.

Suggested fix

Delete temporary local references after they are no longer needed, and also on early-return paths after the reference has been created. For example:

jobject jstatus = ROCKSDB_NAMESPACE::StatusJni::construct(env, s[i]);
if (jstatus == nullptr) {
  return;
}
env->SetObjectArrayElement(jstatuses, i, jstatus);
env->DeleteLocalRef(jstatus);
if (env->ExceptionCheck()) {
  return;
}

jobject jvalue_bytebuf = env->GetObjectArrayElement(jvalues, i);
if (env->ExceptionCheck()) {
  return;
}
...
env->DeleteLocalRef(jvalue_bytebuf);

For the inline incomplete-status branch, store the constructed status in a temporary variable, set it into the array, then delete the local reference.

[aetosdios27]

Thanks for the detailed report and repro of the pattern mismatch with byteArrays().

Confirmed fillByteBuffersAndStatusObjects() in java/rocksjni/jni_multiget_helpers.cc leaks the per-iteration local refs for jstatus (from StatusJni::construct()) and jvalue_bytebuf (from GetObjectArrayElement()), and the inline StatusJni::construct() call in the "result too large" branch has the same issue plus the missing null-check you flagged.

I'm working on a PR that:

• Calls env->DeleteLocalRef() on jstatus right after SetObjectArrayElement(), and checks ExceptionCheck() afterward (matching the byteArrays() sibling helper's pattern).
• Deletes jvalue_bytebuf on every exit path (both error returns and the normal success path after the memcpy).
• Hoists the inline "result too large" StatusJni::construct() call into a named local, null-checks it before use, and deletes its local ref after storing it in jstatuses.

Will link the PR here shortly.