morningman commented on code in PR #51913:
URL: https://github.com/apache/doris/pull/51913#discussion_r2160012130
##########
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:
What is diff between `NewObject` and `NewGlobalRef`?
##########
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:
how about use pointer to indicate that this is a out parameter
##########
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:
Maybe we should define a macro to simple this logic?
##########
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:
Looks like we need to delete every local ref?
But if the code return in the middle of process, the final
`env->DeleteLocalRef` will not be called.
--
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]
