hubgeter commented on code in PR #51913:
URL: https://github.com/apache/doris/pull/51913#discussion_r2192513178


##########
be/src/util/jni-util.cpp:
##########
@@ -464,20 +480,33 @@ Status JniUtil::init_jni_scanner_loader(JNIEnv* env) {
 Status JniUtil::clean_udf_class_load_cache(const std::string& 
function_signature) {
     JNIEnv* env = nullptr;
     RETURN_IF_ERROR(JniUtil::GetJNIEnv(&env));
+    jstring function_signature_jstr = 
env->NewStringUTF(function_signature.c_str());
+    RETURN_ERROR_IF_EXC(env);
     env->CallVoidMethod(jni_scanner_loader_obj_, _clean_udf_cache_method_id,
-                        env->NewStringUTF(function_signature.c_str()));
+                        function_signature_jstr);
+    RETURN_ERROR_IF_EXC(env);
+    env->DeleteLocalRef(function_signature_jstr);
     RETURN_ERROR_IF_EXC(env);
     return Status::OK();
 }
 
 Status JniUtil::get_jni_scanner_class(JNIEnv* env, const char* classname,
                                       jclass* jni_scanner_class) {
     // Get JNI scanner class by class name;
-    jobject loaded_class_obj = env->CallObjectMethod(
-            jni_scanner_loader_obj_, jni_scanner_loader_method_, 
env->NewStringUTF(classname));
+    jstring class_name_str = env->NewStringUTF(classname);

Review Comment:
   Yes, we need to delete every local ref.
   
   I will use RAII to solve this problem in the next pr.



##########
be/src/util/jni-util.cpp:
##########
@@ -311,59 +311,76 @@ Status JniUtil::GetJniExceptionMsg(JNIEnv* env, bool 
log_stack, const string& pr
     return Status::RuntimeError("{}{}", prefix, msg_str_guard.get());
 }
 
-jobject JniUtil::convert_to_java_map(JNIEnv* env, const std::map<std::string, 
std::string>& map) {
-    //TODO: ADD EXCEPTION CHECK.
+Status JniUtil::convert_to_java_map(JNIEnv* env, const std::map<std::string, 
std::string>& map,
+                                    jobject* hashmap_object) {
     jclass hashmap_class = env->FindClass("java/util/HashMap");
+    RETURN_ERROR_IF_EXC(env);
     jmethodID hashmap_constructor = env->GetMethodID(hashmap_class, "<init>", 
"(I)V");
-    jobject hashmap_object = env->NewObject(hashmap_class, 
hashmap_constructor, map.size());
+    RETURN_ERROR_IF_EXC(env);
+    jobject hashmap_local_object = env->NewObject(hashmap_class, 
hashmap_constructor, map.size());
+    RETURN_ERROR_IF_EXC(env);
     jmethodID hashmap_put = env->GetMethodID(
             hashmap_class, "put", 
"(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
+    RETURN_ERROR_IF_EXC(env);
     for (const auto& it : map) {
         jstring key = env->NewStringUTF(it.first.c_str());
         jstring value = env->NewStringUTF(it.second.c_str());
-        env->CallObjectMethod(hashmap_object, hashmap_put, key, value);
+        env->CallObjectMethod(hashmap_local_object, hashmap_put, key, value);
+        RETURN_ERROR_IF_EXC(env);
         env->DeleteLocalRef(key);
         env->DeleteLocalRef(value);
     }
     env->DeleteLocalRef(hashmap_class);
-    return hashmap_object;
+    RETURN_IF_ERROR(LocalToGlobalRef(env, hashmap_local_object, 
hashmap_object));
+    return Status::OK();
 }
 
-std::map<std::string, std::string> JniUtil::convert_to_cpp_map(JNIEnv* env, 
jobject map) {
-    std::map<std::string, std::string> resultMap;
-
+Status JniUtil::convert_to_cpp_map(JNIEnv* env, jobject map,
+                                   std::map<std::string, std::string>& 
resultMap) {

Review Comment:
   good. fixed



##########
be/src/util/jni-util.cpp:
##########
@@ -439,13 +456,12 @@ Status JniUtil::init_jni_scanner_loader(JNIEnv* env) {
             env->GetMethodID(jni_scanner_loader_cls, "loadAllScannerJars", 
"()V");
     RETURN_ERROR_IF_EXC(env);
 
-    jni_scanner_loader_obj_ =
+    jobject jni_scanner_loader_local_obj =

Review Comment:
   `NewObject` return Local reference   
   `NewGlobalRef` return Global reference



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to