[impala] 02/13: IMPALA-7150: fixes jni frame scope

[ta...@apache.org]

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

tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit e9643393ec22a76de255c161bf55420ccad2824d
Author: Vuk Ercegovac <ve...@cloudera.com>
AuthorDate: Thu Jun 14 23:59:09 2018 -0700

    IMPALA-7150: fixes jni frame scope
    
    A previous change (https://gerrit.cloudera.org/#/c/9968/)
    reduced the time that a lib-cache entry was held when initializing
    a udf. It did so by introducing a new scope. However, that scope
    captured a jni frame, which resulted in an earlier frame pop
    than before. This change decouples the frame scope from the lib-cache
    scope so that the frame is popped at the end of the method (as before)
    instread of with the lib-cache scope.
    
    Testing:
    - core, asan tests pass
    
    Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
    Reviewed-on: http://gerrit.cloudera.org:8080/10746
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exprs/hive-udf-call.cc | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/be/src/exprs/hive-udf-call.cc b/be/src/exprs/hive-udf-call.cc
index b7715b7..22683ca 100644
--- a/be/src/exprs/hive-udf-call.cc
+++ b/be/src/exprs/hive-udf-call.cc
@@ -198,7 +198,10 @@ Status HiveUdfCall::OpenEvaluator(FunctionContext::FunctionStateScope scope,
   JNIEnv* env = getJNIEnv();
   if (env == NULL) return Status("Failed to get/create JVM");
 
+  // Add a scoped cleanup jni reference object. This cleans up local refs made below.
+  JniLocalFrame jni_frame;
   {
+    // Scoped handle for libCache entry.
     LibCacheEntryHandle handle;
     string local_location;
     RETURN_IF_ERROR(LibCache::instance()->GetLocalPath(fn_.hdfs_location,
@@ -219,12 +222,13 @@ Status HiveUdfCall::OpenEvaluator(FunctionContext::FunctionStateScope scope,
 
     jbyteArray ctor_params_bytes;
 
-    // Add a scoped cleanup jni reference object. This cleans up local refs made below.
-    JniLocalFrame jni_frame;
+    // Pushed frame will be popped when jni_frame goes out-of-scope.
     RETURN_IF_ERROR(jni_frame.push(env));
 
     RETURN_IF_ERROR(SerializeThriftMsg(env, &ctor_params, &ctor_params_bytes));
-    // Create the java executor object
+    // Create the java executor object. The jar referenced by the libCache handle
+    // is not needed after the next call, so it is safe for the handle to go
+    // out-of-scope after the java object is instantiated.
     jni_ctx->executor = env->NewObject(executor_cl_, executor_ctor_id_, ctor_params_bytes);
   }
   RETURN_ERROR_IF_EXC(env);