Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Commit

Permalink
v2: better error handling
Browse files Browse the repository at this point in the history 
Replace std::runtime_error and in-band error codes
with JVM exceptions.

Signed-off-by: Alexander Bezzubov <bzz@apache.org>
  • Loading branch information
@bzz
bzz committed Jul 1, 2019
1 parent 5ec22ca commit 81df993
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 68 deletions.
91 changes: 59 additions & 32 deletions src/main/native/jni_utils.cc
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include "jni_utils.h"
#include <string>

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;
Expand All @@ -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";
Expand All @@ -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 =
Expand All @@ -39,84 +45,105 @@ 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, "<init>", initSign);
if (env->ExceptionOccurred() || !initId) {
return nullptr;
}
checkJvmException(
std::string("failed to call <inti> 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;
}

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;
}

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;
}
7 changes: 7 additions & 0 deletions src/main/native/jni_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
#define _Included_org_bblfsh_client_libuast_Libuast_jni_utils

#include <jni.h>
#include <string>

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;
Expand All @@ -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);
Expand Down
59 changes: 23 additions & 36 deletions src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class Node : public uast::Node<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();
Expand Down Expand Up @@ -263,20 +263,18 @@ class Node : public uast::Node<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();
Expand All @@ -291,12 +289,10 @@ class Node : public uast::Node<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(")"));
}
};

Expand Down Expand Up @@ -435,21 +431,16 @@ class Context {
JNIEnv *env = getJNIEnv();

ContextExt *nodeExtCtx = getHandle<ContextExt>(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<NodeHandle>(getHandle<NodeHandle>(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);
}
};

Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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<void **>(&env), JNI_VERSION_1_8) != JNI_OK) {
return -1;
return JNI_ERR;
}
jvm = vm;

Expand Down

0 comments on commit 81df993

Please sign in to comment.