Repository: incubator-impala Updated Branches: refs/heads/master 308bd5d58 -> 2bd010781
IMPALA-6060: Check the return value of JNI exception handling functions When JVM runs out of memory and throws an error to JNI, the error handling code uses JNI to get the exception message, resulting in a null pointer and crashing the process. This patch adds error handling code to JniUtil::GetJniExceptionMsg(). Change-Id: Ie3ed88bf8739c56a066f2402727c8204e96aa116 Reviewed-on: http://gerrit.cloudera.org:8080/8334 Reviewed-by: Alex Behm <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/31c440bf Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/31c440bf Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/31c440bf Branch: refs/heads/master Commit: 31c440bf341c18a838e15360bb4b2e8c90f86c75 Parents: 308bd5d Author: Tianyi Wang <[email protected]> Authored: Wed Oct 18 15:54:14 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Oct 24 21:22:58 2017 +0000 ---------------------------------------------------------------------- be/src/util/jni-util.cc | 23 ++++++++++++++++---- be/src/util/jni-util.h | 4 ++-- be/src/util/logging-support.cc | 1 + .../java/org/apache/impala/common/JniUtil.java | 20 +++++++---------- 4 files changed, 30 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/be/src/util/jni-util.cc ---------------------------------------------------------------------- diff --git a/be/src/util/jni-util.cc b/be/src/util/jni-util.cc index 220eb83..3128697 100644 --- a/be/src/util/jni-util.cc +++ b/be/src/util/jni-util.cc @@ -27,7 +27,8 @@ namespace impala { -Status JniUtfCharGuard::create(JNIEnv *env, jstring jstr, JniUtfCharGuard *out) { +Status JniUtfCharGuard::create(JNIEnv* env, jstring jstr, JniUtfCharGuard* out) { + DCHECK(jstr != nullptr); DCHECK(!env->ExceptionCheck()); const char* utf_chars = env->GetStringUTFChars(jstr, nullptr); bool exception_check = static_cast<bool>(env->ExceptionCheck()); @@ -180,17 +181,31 @@ void JniUtil::InitLibhdfs() { } Status JniUtil::GetJniExceptionMsg(JNIEnv* env, bool log_stack, const string& prefix) { - jthrowable exc = (env)->ExceptionOccurred(); + jthrowable exc = env->ExceptionOccurred(); if (exc == nullptr) return Status::OK(); env->ExceptionClear(); DCHECK(throwable_to_string_id() != nullptr); - auto msg = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(), + const char* oom_msg_template = "$0 threw an unchecked exception. The JVM is likely out " + "of memory (OOM)."; + jstring msg = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(), throwable_to_string_id(), exc)); + if (env->ExceptionOccurred()) { + env->ExceptionClear(); + string oom_msg = Substitute(oom_msg_template, "throwableToString"); + LOG(ERROR) << oom_msg; + return Status(oom_msg); + } JniUtfCharGuard msg_str_guard; RETURN_IF_ERROR(JniUtfCharGuard::create(env, msg, &msg_str_guard)); if (log_stack) { - auto stack = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(), + jstring stack = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(), throwable_to_stack_trace_id(), exc)); + if (env->ExceptionOccurred()) { + env->ExceptionClear(); + string oom_msg = Substitute(oom_msg_template, "throwableToStackTrace"); + LOG(ERROR) << oom_msg; + return Status(oom_msg); + } JniUtfCharGuard c_stack_guard; RETURN_IF_ERROR(JniUtfCharGuard::create(env, stack, &c_stack_guard)); VLOG(1) << c_stack_guard.get(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/be/src/util/jni-util.h ---------------------------------------------------------------------- diff --git a/be/src/util/jni-util.h b/be/src/util/jni-util.h index 77abb4d..43ac131 100644 --- a/be/src/util/jni-util.h +++ b/be/src/util/jni-util.h @@ -154,9 +154,9 @@ class JniUtfCharGuard { if (utf_chars != nullptr) env->ReleaseStringUTFChars(jstr, utf_chars); } - /// Try to get chars from jstring. If error is returned, utf_chars and get() remain + /// Try to get chars from jstr. If error is returned, utf_chars and get() remain /// to be nullptr, otherwise they point to a valid char sequence. The char sequence - /// lives as long as this guard. + /// lives as long as this guard. jstr should not be null. static Status create(JNIEnv* env, jstring jstr, JniUtfCharGuard* out); /// Get the char sequence. Returns nullptr if the guard does hold a char sequence. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/be/src/util/logging-support.cc ---------------------------------------------------------------------- diff --git a/be/src/util/logging-support.cc b/be/src/util/logging-support.cc index 3d90ed6..400bd67 100644 --- a/be/src/util/logging-support.cc +++ b/be/src/util/logging-support.cc @@ -42,6 +42,7 @@ JNIEXPORT void JNICALL Java_org_apache_impala_util_NativeLogger_Log( JNIEnv* env, jclass caller_class, int severity, jstring msg, jstring file, int line_number) { + DCHECK(file != nullptr); JniUtfCharGuard filename_guard; RETURN_VOID_IF_ERROR(JniUtfCharGuard::create(env, file, &filename_guard)); JniUtfCharGuard msg_guard; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/fe/src/main/java/org/apache/impala/common/JniUtil.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/common/JniUtil.java b/fe/src/main/java/org/apache/impala/common/JniUtil.java index 1fb00c8..6aec0d4 100644 --- a/fe/src/main/java/org/apache/impala/common/JniUtil.java +++ b/fe/src/main/java/org/apache/impala/common/JniUtil.java @@ -60,18 +60,14 @@ public class JniUtil { * the chain of causes each in a separate line. */ public static String throwableToString(Throwable t) { - Writer output = new StringWriter(); - try { - output.write(String.format("%s: %s", t.getClass().getSimpleName(), - t.getMessage())); - // Follow the chain of exception causes and print them as well. - Throwable cause = t; - while ((cause = cause.getCause()) != null) { - output.write(String.format("\nCAUSED BY: %s: %s", - cause.getClass().getSimpleName(), cause.getMessage())); - } - } catch (IOException e) { - throw new Error(e); + StringWriter output = new StringWriter(); + output.write(String.format("%s: %s", t.getClass().getSimpleName(), + t.getMessage())); + // Follow the chain of exception causes and print them as well. + Throwable cause = t; + while ((cause = cause.getCause()) != null) { + output.write(String.format("\nCAUSED BY: %s: %s", + cause.getClass().getSimpleName(), cause.getMessage())); } return output.toString(); }
