Skip to content

Comparator thread-local reusable buffers do not delete their native wrapper #14975

Description

@K-ANOY

File: java/rocksjni/comparatorjnicallback.cc

Functions: ComparatorJniCallback::ComparatorJniCallback, ComparatorJniCallback::GetBuffer

When comparator buffer reuse is configured with ReusedSynchronisationType::THREAD_LOCAL, GetBuffer allocates a ThreadLocalBuf for each thread-local reused buffer:

jobject jtl_buf = env->NewGlobalRef(ByteBufferJni::construct(
    env, m_options->direct_buffer, m_options->max_reused_buffer_size,
    m_jbytebuffer_clazz));
if (jtl_buf == nullptr) {
  // exception thrown: OutOfMemoryError
  return nullptr;
}
tlb = new ThreadLocalBuf(m_jvm, m_options->direct_buffer, jtl_buf);
tl_buf->Reset(tlb);

The associated ThreadLocalPtr unref handler releases the direct-buffer backing
memory and deletes the JNI global reference, but it never deletes the
ThreadLocalBuf object itself:

UnrefHandler unref = [](void* ptr) {
  ThreadLocalBuf* tlb = reinterpret_cast<ThreadLocalBuf*>(ptr);
  jboolean attached_thread = JNI_FALSE;
  JNIEnv* _env = JniUtil::getJniEnv(tlb->jvm, &attached_thread);
  if (_env != nullptr) {
    if (tlb->direct_buffer) {
      void* buf = _env->GetDirectBufferAddress(tlb->jbuf);
      delete[] static_cast<char*>(buf);
    }
    _env->DeleteGlobalRef(tlb->jbuf);
    JniUtil::releaseJniEnv(tlb->jvm, attached_thread);
  }
};

ThreadLocalPtr does call the handler for stored values on thread exit and when
the ThreadLocalPtr is destroyed, so the missing delete tlb leaks the native
wrapper allocated for every thread-local comparator buffer.

Suggested fix

Delete the wrapper after releasing its JNI/direct-buffer resources:

 UnrefHandler unref = [](void* ptr) {
   ThreadLocalBuf* tlb = reinterpret_cast<ThreadLocalBuf*>(ptr);
   jboolean attached_thread = JNI_FALSE;
   JNIEnv* _env = JniUtil::getJniEnv(tlb->jvm, &attached_thread);
   if (_env != nullptr) {
     if (tlb->direct_buffer) {
       void* buf = _env->GetDirectBufferAddress(tlb->jbuf);
       delete[] static_cast<char*>(buf);
     }
     _env->DeleteGlobalRef(tlb->jbuf);
     JniUtil::releaseJniEnv(tlb->jvm, attached_thread);
   }
+  delete tlb;
 };

If getJniEnv can fail during shutdown, the code may also need a policy for
the still-held JNI global reference. The immediate native leak is the missing
delete of ThreadLocalBuf.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions