diff --git a/src/main/native/jni_utils.cc b/src/main/native/jni_utils.cc index 84caf06..94867d0 100644 --- a/src/main/native/jni_utils.cc +++ b/src/main/native/jni_utils.cc @@ -1,6 +1,9 @@ #include "jni_utils.h" +#include -extern JavaVM *jvm; // FIXME(bzz): double-check and document +// TODO(bzz): double-check and document. Suggestion and more context at +// https://github.com/bblfsh/client-scala/pull/84#discussion_r288347756 +extern JavaVM *jvm; JNIEnv *getJNIEnv() { JNIEnv *pEnv = NULL; @@ -20,6 +23,8 @@ JNIEnv *getJNIEnv() { // Class fully qualified names const char *CLS_NODE = "org/bblfsh/client/v2/Node"; const char *CLS_CTX = "org/bblfsh/client/v2/Context"; +const char *CLS_OBJ = "java/lang/Object"; +const char *CLS_RE = "java/lang/RuntimeException"; const char *CLS_JNODE = "org/bblfsh/client/v2/JNode"; const char *CLS_JNULL = "org/bblfsh/client/v2/JNull"; @@ -31,6 +36,7 @@ const char *CLS_JUINT = "org/bblfsh/client/v2/JUint"; const char *CLS_JARR = "org/bblfsh/client/v2/JArray"; const char *CLS_JOBJ = "org/bblfsh/client/v2/JObject"; +// Method signatures const char *METHOD_JNODE_KEY_AT = "(I)Ljava/lang/String;"; const char *METHOD_JNODE_VALUE_AT = "(I)Lorg/bblfsh/client/v2/JNode;"; const char *METHOD_JOBJ_ADD = @@ -39,53 +45,66 @@ const char *METHOD_JOBJ_ADD = const char *METHOD_JARR_ADD = "(Lorg/bblfsh/client/v2/JNode;)Lscala/collection/mutable/Buffer;"; +const char *METHOD_OBJ_TO_STR = "()Ljava/lang/String;"; + +void checkJvmException(std::string msg) { + JNIEnv *env = getJNIEnv(); + if (env->ExceptionCheck()) { + // env->ExceptionDescribe(); // prints to stdout, un-comment for debuging + auto err = env->ExceptionOccurred(); + + jmethodID toString = env->GetMethodID(env->FindClass(CLS_OBJ), "toString", + METHOD_OBJ_TO_STR); + jstring s = (jstring)env->CallObjectMethod(err, toString); + const char *utf = env->GetStringUTFChars(s, 0); + env->ReleaseStringUTFChars(s, utf); + + env->ExceptionClear(); + + auto exception = env->FindClass(CLS_RE); + env->ThrowNew(exception, msg.append(": ").append(utf).data()); + } +} + jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign, ...) { jclass cls = env->FindClass(className); - if (env->ExceptionOccurred() || !cls) { - return nullptr; - } + checkJvmException(std::string("failed to find a class ").append(className)); jmethodID initId = env->GetMethodID(cls, "", initSign); - if (env->ExceptionOccurred() || !initId) { - return nullptr; - } + checkJvmException( + std::string("failed to call constructor for ").append(className)); va_list varargs; va_start(varargs, initSign); jobject instance = env->NewObjectV(cls, initId, varargs); va_end(varargs); - if (env->ExceptionOccurred() || !instance) { - return nullptr; - } + checkJvmException( + std::string("failed get varargs for constructor of ").append(className)); return instance; } jfieldID getField(JNIEnv *env, jobject obj, const char *name) { jclass cls = env->GetObjectClass(obj); - if (env->ExceptionOccurred() || !cls) { - return nullptr; - } + checkJvmException("failed get an object class"); jfieldID jfid = env->GetFieldID(cls, name, "J"); - if (env->ExceptionOccurred() || !jfid) { - return nullptr; - } + checkJvmException(std::string("failed get a field ").append(name)); + return jfid; } static jmethodID MethodID(JNIEnv *env, const char *method, const char *signature, const char *className) { jclass cls = env->FindClass(className); - if (env->ExceptionOccurred() || !cls) { - return nullptr; - } + checkJvmException(std::string("failed to find a class ").append(className)); jmethodID mId = env->GetMethodID(cls, method, signature); - if (env->ExceptionOccurred()) { - return nullptr; - } + checkJvmException(std::string("failed get a method ") + .append(className) + .append(".") + .append(method)); return mId; } @@ -93,16 +112,18 @@ static jmethodID MethodID(JNIEnv *env, const char *method, jint IntMethod(JNIEnv *env, const char *method, const char *signature, const char *className, const jobject *object) { jmethodID mId = MethodID(env, method, signature, className); - if (env->ExceptionOccurred() || !mId) return 0; - // TODO(bzz): add better error handling - // ExceptionOccurred() - // ExceptionDescribe() - // ExceptionClear() - // ExceptionCheck() - // ThrowNew("failed to call $className.$method") to JVM + checkJvmException(std::string("failed get a method ") + .append(className) + .append(".") + .append(method)); jint res = env->CallIntMethod(*object, mId); - if (env->ExceptionOccurred()) return 0; + checkJvmException(std::string("failed call a method ") + .append(className) + .append(".") + .append(method) + .append(" using signature ") + .append(signature)); return res; } @@ -110,13 +131,19 @@ jint IntMethod(JNIEnv *env, const char *method, const char *signature, jobject ObjectMethod(JNIEnv *env, const char *method, const char *signature, const char *className, const jobject *object, ...) { jmethodID mId = MethodID(env, method, signature, className); - if (env->ExceptionOccurred() || !mId) return nullptr; + checkJvmException(std::string("failed get a method ") + .append(className) + .append(".") + .append(method)); va_list varargs; va_start(varargs, object); jobject res = env->CallObjectMethodV(*object, mId, varargs); va_end(varargs); - if (env->ExceptionOccurred() || !res) return nullptr; + checkJvmException(std::string("failed get varargs for ") + .append(className) + .append(".") + .append(method)); return res; } diff --git a/src/main/native/jni_utils.h b/src/main/native/jni_utils.h index f3ab76f..a04818c 100644 --- a/src/main/native/jni_utils.h +++ b/src/main/native/jni_utils.h @@ -2,9 +2,12 @@ #define _Included_org_bblfsh_client_libuast_Libuast_jni_utils #include +#include extern const char *CLS_NODE; extern const char *CLS_CTX; +extern const char *CLS_OBJ; +extern const char *CLS_RE; extern const char *CLS_JNODE; extern const char *CLS_JNULL; @@ -21,6 +24,10 @@ extern const char *METHOD_JNODE_VALUE_AT; extern const char *METHOD_JOBJ_ADD; extern const char *METHOD_JARR_ADD; +extern const char *METHOD_OBJ_TO_STR; + +void checkJvmException(std::string); + JNIEnv *getJNIEnv(); jobject NewJavaObject(JNIEnv *, const char *, const char *, ...); jfieldID getField(JNIEnv *env, jobject obj, const char *name); diff --git a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc index 050acfb..9b8768c 100644 --- a/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc +++ b/src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc @@ -189,7 +189,7 @@ class Node : public uast::Node { NodeKind Kind() { return kind; } - // TODO(#90) all 'As*' are unused stubs, make them buletprof and test + // TODO(#90): implement and test (all 'As*' are unused stubs for now) std::string *AsString() { if (!str) { JNIEnv *env = getJNIEnv(); @@ -263,20 +263,18 @@ class Node : public uast::Node { } void SetValue(size_t i, Node *val) { + JNIEnv *env = getJNIEnv(); jobject v = nullptr; if (val && val->obj) { v = val->obj; - } else { // FIXME(bzz) - // v = "new JNull instance"; // Py_None; + } else { + v = NewJavaObject(env, CLS_JNULL, "()V"); } - jobject res = - ObjectMethod(getJNIEnv(), "add", METHOD_JARR_ADD, CLS_JARR, &obj, v); - if (res == nullptr) { // FIXME(bzz) replace by ThrowNew() to JVM - throw std::runtime_error(std::string("failed to call ") - .append(CLS_JARR) - .append(".add() from Node::SetValue()")); - } + ObjectMethod(getJNIEnv(), "add", METHOD_JARR_ADD, CLS_JARR, &obj, v); + checkJvmException(std::string("failed to call ") + .append(CLS_JARR) + .append(".add() from Node::SetValue()")); } void SetKeyValue(std::string key, Node *val) { JNIEnv *env = getJNIEnv(); @@ -291,12 +289,10 @@ class Node : public uast::Node { jobject res = ObjectMethod(env, "add", METHOD_JOBJ_ADD, CLS_JOBJ, &obj, k, v); - if (res == nullptr) { // FIXME(bzz) replace by ThrowNew() to JVM - throw std::runtime_error( - std::string("failed to call JObject.add() from Node::SetKeyValue(") - .append(key) - .append(")")); - } + checkJvmException( + std::string("failed to call JObject.add() from Node::SetKeyValue(") + .append(key) + .append(")")); } }; @@ -435,21 +431,16 @@ class Context { JNIEnv *env = getJNIEnv(); ContextExt *nodeExtCtx = getHandle(env, src, "ctx"); - if (!nodeExtCtx) { - throw std::runtime_error("Cannot get Node.ctx"); - } + checkJvmException("failed to get Node.ctx"); + auto sctx = nodeExtCtx->ctx; NodeHandle snode = reinterpret_cast(getHandle(env, src, "handle")); - if (!snode) { - throw std::runtime_error("Cannot get Node.handle"); - } + checkJvmException("failed to get Node.handle"); Node *node = uast::Load(sctx, snode, ctx); - if (!node) { - throw std::runtime_error("Failed to uast::Load()"); - } - return toJ(node); // new ref + checkJvmException("failed to uast::Load()"); + return toJ(node); } }; @@ -465,13 +456,10 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( // works only with ByteBuffer.allocateDirect() void *buf = env->GetDirectBufferAddress(directBuf); - if (env->ExceptionCheck() == JNI_TRUE) { - return nullptr; - } + checkJvmException("failed to use buffer for direct access"); + jlong len = env->GetDirectBufferCapacity(directBuf); - if (env->ExceptionCheck() == JNI_TRUE) { - return nullptr; - } + checkJvmException("failed to get buffer capacity"); // another option (instead of XXX) is to use // GetPrimitiveArrayCritical @@ -482,12 +470,11 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_libuast_Libuast_decode( auto p = new ContextExt(ctx); jobject jCtxExt = NewJavaObject(env, CLS_CTX, "(J)V", p); - if (env->ExceptionCheck() == JNI_TRUE || !jCtxExt) { + if (env->ExceptionCheck() || !jCtxExt) { jCtxExt = nullptr; delete (ctx); delete (p); - env->ExceptionDescribe(); - throw std::runtime_error("failed to instantiate Context class"); + checkJvmException("failed to instantiate Context class"); } return jCtxExt; @@ -547,7 +534,7 @@ JNIEXPORT jobject JNICALL Java_org_bblfsh_client_v2_Node_load(JNIEnv *env, JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *reserved) { JNIEnv *env; if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_8) != JNI_OK) { - return -1; + return JNI_ERR; } jvm = vm;