[impala] 01/02: IMPALA-12274: Fix memory leak from unreleased local reference

[db...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dbecker pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 748c3b894f943d0bba5bb25e066365f762e7ee43
Author: qqzhang <zh...@sensorsdata.cn>
AuthorDate: Mon Jul 10 10:46:18 2023 +0800

    IMPALA-12274: Fix memory leak from unreleased local reference
    
    In the constructor of the catalog class, after converting the catalog
    object created by NewObject to a global reference, the local reference
    was forgotten to be released, resulting in a minor memory leak.
    
    The solution is to use JniLocalFrame to automatically manage the local
    reference, ensuring proper memory management.
    
    Testing: manually test
    
    Change-Id: Ic93ea3270dcba3ad4903aa053cc283d4f700e948
    Reviewed-on: http://gerrit.cloudera.org:8080/20175
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/catalog/catalog.cc | 10 +++++++---
 be/src/catalog/catalog.h  |  3 ---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/be/src/catalog/catalog.cc b/be/src/catalog/catalog.cc
index dec23e053..3efc4b464 100644
--- a/be/src/catalog/catalog.cc
+++ b/be/src/catalog/catalog.cc
@@ -73,19 +73,23 @@ Catalog::Catalog() {
   };
 
   JNIEnv* jni_env = JniUtil::GetJNIEnv();
+  // Used to release local references promptly
+  JniLocalFrame jni_frame;
+  ABORT_IF_ERROR(jni_frame.push(jni_env));
+
   // Create an instance of the java class JniCatalog
-  catalog_class_ = jni_env->FindClass("org/apache/impala/service/JniCatalog");
+  jclass catalog_class = jni_env->FindClass("org/apache/impala/service/JniCatalog");
   ABORT_IF_EXC(jni_env);
 
   uint32_t num_methods = sizeof(methods) / sizeof(methods[0]);
   for (int i = 0; i < num_methods; ++i) {
-    ABORT_IF_ERROR(JniUtil::LoadJniMethod(jni_env, catalog_class_, &(methods[i])));
+    ABORT_IF_ERROR(JniUtil::LoadJniMethod(jni_env, catalog_class, &(methods[i])));
   }
 
   jbyteArray cfg_bytes;
   ABORT_IF_ERROR(GetThriftBackendGFlagsForJNI(jni_env, &cfg_bytes));
 
-  jobject catalog = jni_env->NewObject(catalog_class_, catalog_ctor_, cfg_bytes);
+  jobject catalog = jni_env->NewObject(catalog_class, catalog_ctor_, cfg_bytes);
   CLEAN_EXIT_IF_EXC(jni_env);
   ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, catalog, &catalog_));
 }
diff --git a/be/src/catalog/catalog.h b/be/src/catalog/catalog.h
index 315f72463..242383e1c 100644
--- a/be/src/catalog/catalog.h
+++ b/be/src/catalog/catalog.h
@@ -142,9 +142,6 @@ class Catalog {
   void RegenerateServiceId();
 
  private:
-  /// Descriptor of Java Catalog class itself, used to create a new instance.
-  jclass catalog_class_;
-
   jobject catalog_;  // instance of org.apache.impala.service.JniCatalog
   jmethodID update_metastore_id_;  // JniCatalog.updateMetaastore()
   jmethodID exec_ddl_id_;  // JniCatalog.execDdl()