Skip to content

Commit

Permalink
Rework exception handling
Browse files Browse the repository at this point in the history
  • Loading branch information
CedNaru committed Sep 17, 2024
1 parent 91c5fda commit 4e11878
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 100 deletions.
14 changes: 12 additions & 2 deletions kt/godot-library/src/main/kotlin/godot/core/Properties.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package godot.core
import godot.PropertyHint
import godot.PropertyUsageFlags
import godot.core.memory.TransferContext
import godot.global.GD
import kotlin.reflect.KMutableProperty1

data class KtPropertyInfo(
Expand Down Expand Up @@ -30,12 +31,21 @@ open class KtProperty<T : KtObject, P : Any?>(
protected val variantConverter: VariantConverter,
) {
open fun callGet(instance: T) {
TransferContext.writeReturnValue(kProperty.get(instance), variantConverter)
try {
TransferContext.writeReturnValue(kProperty.get(instance), variantConverter)
} catch (t: Throwable) {
GD.printErr("Error calling a JVM getter from Godot:", t.stackTraceToString())
TransferContext.writeReturnValue(null, VariantType.NIL)
}
}

open fun callSet(instance: T) {
val arg = extractSetterArgument<P>()
kProperty.set(instance, arg)
try {
kProperty.set(instance, arg)
} catch (t: Throwable) {
GD.printErr("Error calling a JVM setter from Godot:", t.stackTraceToString())
}
}

protected fun <P> extractSetterArgument(): P {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
@file:Suppress("PackageDirectoryMismatch")

package godot.core

object JvmStackTrace {
fun getExceptionStackTrace(throwable: Throwable): String {
return throwable.stackTrace.joinToString("\n")
}

// TODO: Use this method to get the JVM stacktrace when Godot will add the features to script https://github.com/godotengine/godot/pull/91006
fun getCurrentStacktrace() = Thread.currentThread().stackTrace.joinToString("\n")
}
2 changes: 0 additions & 2 deletions kt/godot-library/src/main/kotlin/godot/global/GDPrint.kt
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ internal interface GDPrint {
fun printStacktrace() = print(Thread.currentThread().stackTrace.joinToString("\n"))

private object Bridge {
fun getStacktrace() = Thread.currentThread().stackTrace.joinToString("\n")

external fun print()
external fun printRich()
external fun printVerbose()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,20 @@
{ "name" : "freeObject", "parameterTypes" : ["long"] }
]
},
{
"name":"godot.core.JvmStackTrace",
"fields":[{"name":"INSTANCE"}],
"methods":[
{ "name" : "getExceptionStackTrace","parameterTypes" : ["java.lang.Throwable"] },
{ "name" : "getCurrentStacktrace","parameterTypes" : [] }
]
},
{
"name" : "godot.global.GDPrint$Bridge",
"fields" : [
{ "name" : "INSTANCE" }
],
"methods" : [
{ "name" : "getStacktrace", "parameterTypes" : [] },
{ "name" : "print", "parameterTypes" : [] },
{ "name" : "printRich", "parameterTypes" : [] },
{ "name" : "printVerbose", "parameterTypes" : [] },
Expand Down
78 changes: 23 additions & 55 deletions src/jni/env.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

namespace jni {


void Env::set_exception_handler(void (*p_exception_handler)(Env, JThrowable)) {
exception_handler = p_exception_handler;
}

Env::Env(JNIEnv* env) {
this->env = env;
}
Expand All @@ -25,91 +30,54 @@ namespace jni {
JClass Env::find_class(const char* name) {
jclass cls = env->FindClass(String(name).replace(".", "/").utf8());
JVM_DEV_ASSERT(cls, "Class not found: %s", name);
check_exceptions();
handle_exception();
return JClass(cls);
}

JObject Env::new_string(const char* str) {
auto jstr = env->NewStringUTF(str);
check_exceptions();
handle_exception();
return JObject(jstr);
}

bool Env::exception_check() {
return env->ExceptionCheck();
}

String Env::exception_describe() {
#ifdef DEBUG_ENABLED
if (jthrowable e = env->ExceptionOccurred()) {
env->ExceptionClear(); // needs to be cleared now as otherwise it throws warnings at us in the following two jni calls that we do jni calls with a pending exception
jclass string_writer_class {env->FindClass("java/io/StringWriter")};
jmethodID string_writer_constructor {env->GetMethodID(string_writer_class, "<init>", "()V")};
jobject string_writer {env->NewObject(string_writer_class, string_writer_constructor)};

jclass print_writer_class {env->FindClass("java/io/PrintWriter")};
jmethodID print_writer_constructor {env->GetMethodID(print_writer_class, "<init>", "(Ljava/io/Writer;)V")};
jobject print_writer {env->NewObject(print_writer_class, print_writer_constructor, string_writer)};

jclass throwable_class {env->FindClass("java/lang/Throwable")};
jmethodID print_stack_trace_method {env->GetMethodID(throwable_class, "printStackTrace", "(Ljava/io/PrintWriter;)V")};
env->CallVoidMethod(e, print_stack_trace_method, print_writer);

if (exception_check()) {
// if we get here, it means we got an exception while calling `printStackTrace`. Hence, we cannot retrieve the stacktrace. No need to report that to the user. We just return an empty string.
exception_clear();
return {};
}

jmethodID to_string_method {env->GetMethodID(string_writer_class, "toString", "()Ljava/lang/String;")};
auto j_stack_trace_string {(jstring) env->CallObjectMethod(string_writer, to_string_method)};

if (exception_check()) {
// if we get here, it means we got an exception while calling `toString`. Hence, we cannot retrieve the stacktrace. No need to report that to the user. We just return an empty string.
exception_clear();
return {};
}

const char* c_stack_trace {env->GetStringUTFChars(j_stack_trace_string, nullptr)};
String stack_trace {c_stack_trace};
env->ReleaseStringUTFChars(j_stack_trace_string, c_stack_trace);

return stack_trace;
}
#else
void Env::exception_describe() {
env->ExceptionDescribe();
#endif
return {};
}

void Env::exception_clear() {
// no op if DEBUG_ENABLED. Then the exception is cleared in exception_describe
#ifndef DEBUG_ENABLED
env->ExceptionClear();
#endif
}

void Env::check_exceptions() {
if (exception_check()) {
String exception {exception_describe()};
exception_clear();
JThrowable Env::exception_occurred() {
return env->ExceptionOccurred();
}

if (exception.is_empty()) {
exception = "An exception has occurred!";
void Env::handle_exception() {
if (unlikely(exception_check())) {
if(exception_handler) {
JThrowable throwable = exception_occurred();
exception_clear();
exception_handler(*this, throwable);
} else {
exception_describe();
exception_clear();
}
JVM_ERR_FAIL_MSG(exception);
}
}

void* Env::get_direct_buffer_address(const jni::JObject& buffer) {
void* res = env->GetDirectBufferAddress(buffer.obj);
check_exceptions();
handle_exception();
return res;
}

int Env::get_direct_buffer_capacity(const JObject& buffer) {
auto capacity = env->GetDirectBufferCapacity(buffer.obj);
check_exceptions();
handle_exception();
return capacity;
}

Expand All @@ -121,7 +89,7 @@ namespace jni {
auto jstr = (jstring) str.obj;
auto utfString = env->GetStringUTFChars(jstr, nullptr);
auto ret = String(utfString);
check_exceptions();
handle_exception();
env->ReleaseStringUTFChars(jstr, utfString);
return ret;
}
Expand Down
36 changes: 21 additions & 15 deletions src/jni/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace jni {

class JObject;
class JClass;
class JThrowable;
class JArray;
class JObjectArray;
class JByteArray;
Expand All @@ -20,12 +21,29 @@ namespace jni {
class JString;

class Env {
friend class JObject;
friend class JClass;
friend class JThrowable;
friend class JArray;
friend class JObjectArray;
friend class JByteArray;
friend class JIntArray;
friend class JLongArray;
friend class JFloatArray;
friend class JDoubleArray;

JNIEnv* env;

static inline void(*exception_handler)(Env, JThrowable) = nullptr;

public:
explicit Env(JNIEnv*);

Env(const Env&) = default;
Env& operator=(const Env&) = default;

static void set_exception_handler(void(*p_exception_handler)(Env, JThrowable));

JavaVM* get_jvm();

void push_local_frame(int capacity);
Expand All @@ -37,30 +55,18 @@ namespace jni {
String from_jstring(JString str);

bool exception_check();
String exception_describe();
void exception_describe();
void exception_clear();
JThrowable exception_occurred();

void check_exceptions();
void handle_exception();

void* get_direct_buffer_address(const jni::JObject& buffer);
int get_direct_buffer_capacity(const jni::JObject& buffer);

bool is_same_object(const jni::JObject& obj_1, const jni::JObject& obj_2);

bool is_valid();

private:
JNIEnv* env;

friend class JObject;
friend class JClass;
friend class JArray;
friend class JObjectArray;
friend class JByteArray;
friend class JIntArray;
friend class JLongArray;
friend class JFloatArray;
friend class JDoubleArray;
};
}// namespace jni

Expand Down
14 changes: 7 additions & 7 deletions src/jni/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,27 +68,27 @@ namespace jni {
MethodID JClass::get_method_id(Env& env, const char* name, const char* signature) {
auto id = env.env->GetMethodID((jclass) obj, name, signature);
JVM_DEV_ASSERT(id, "Method not found: %s with signature: %s", name, signature);
env.check_exceptions();
env.handle_exception();
return id;
}

MethodID JClass::get_static_method_id(Env& env, const char* name, const char* signature) {
auto id = env.env->GetStaticMethodID((jclass) obj, name, signature);
JVM_DEV_ASSERT(id, "Method not found: %s with signature: %s", name, signature);
env.check_exceptions();
env.handle_exception();
return id;
}

FieldID JClass::get_static_field_id(Env& env, const char* name, const char* signature) {
auto id = env.env->GetStaticFieldID((jclass) obj, name, signature);
JVM_DEV_ASSERT(id, "Field not found: %s with signature: %s", name, signature);
env.check_exceptions();
env.handle_exception();
return id;
}

void JClass::register_natives(Env& env, Vector<JNativeMethod> methods) {
env.env->RegisterNatives((jclass) obj, methods.ptr(), methods.size());
env.check_exceptions();
env.handle_exception();
}

MethodID JClass::get_constructor_method_id(Env& env, const char* signature) {
Expand All @@ -98,19 +98,19 @@ namespace jni {
JObject JClass::new_instance(Env& env, MethodID ctor, jvalue* args) {
auto ret = env.env->NewObjectA((jclass) obj, ctor, args);
JVM_DEV_ASSERT(ret, "Failed to instantiated object!");
env.check_exceptions();
env.handle_exception();
return JObject(ret);
}

JObject JClass::call_static_object_method(Env& env, MethodID method, jvalue* args) {
auto ret = env.env->CallStaticObjectMethodA((jclass) obj, method, args);
env.check_exceptions();
env.handle_exception();
return JObject(ret);
}

JObject JClass::get_static_object_field(Env& env, FieldID field) {
auto value = env.env->GetStaticObjectField((jclass) obj, field);
env.check_exceptions();
env.handle_exception();
return JObject(value);
}

Expand Down
12 changes: 10 additions & 2 deletions src/jni/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ namespace jni {
};

#ifdef DEV_ENABLED
#define CHECK_EXCEPTION_TEMPLATE p_env.check_exceptions()
#define CHECK_EXCEPTION_TEMPLATE p_env.handle_exception()
#else
#define CHECK_EXCEPTION_TEMPLATE \
if constexpr (CHECK_EXCEPTION) { p_env.check_exceptions(); } \
if constexpr (CHECK_EXCEPTION) { p_env.handle_exception(); } \
(void) 0
#endif

Expand Down Expand Up @@ -273,6 +273,14 @@ namespace jni {
bool is_assignable_from(Env& env, JClass p_other) const;
};

class JThrowable : public JObject {
public:
JThrowable() = default;
JThrowable(jthrowable throwable) : JObject(throwable) {};

explicit JThrowable(JObject jObject) : JObject(jObject) {};
};

}// namespace jni

#endif// GODOT_LOADER_JOBJECT_H
7 changes: 0 additions & 7 deletions src/jvm_wrapper/bridge/gd_print_bridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,4 @@ void GDPrintBridge::push_warning(JNIEnv* p_raw_env, jobject p_instance) {
WARN_PRINT(args[0].operator String());
}

String GDPrintBridge::get_jvm_stacktrace(jni::Env& p_env) {
jni::JString str {wrapped.call_object_method(p_env, PRINT_STACKTRACE)};
String ret {p_env.from_jstring(str)};
str.delete_local_ref(p_env);
return ret;
}

GDPrintBridge::~GDPrintBridge() = default;
6 changes: 0 additions & 6 deletions src/jvm_wrapper/bridge/gd_print_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ namespace bridges {
SINGLETON_CLASS(GDPrintBridge)

// clang-format off
JNI_OBJECT_METHOD(PRINT_STACKTRACE)

INIT_JNI_BINDINGS(
INIT_JNI_METHOD(PRINT_STACKTRACE, "getStacktrace", "()Ljava/lang/String;")
INIT_NATIVE_METHOD("print", "()V", GDPrintBridge::print)
INIT_NATIVE_METHOD("printRich", "()V", GDPrintBridge::print_rich)
INIT_NATIVE_METHOD("printVerbose", "()V", GDPrintBridge::print_verbose2)
Expand All @@ -31,9 +28,6 @@ namespace bridges {
static void print_raw(JNIEnv* p_raw_env, jobject p_instance);
static void push_error(JNIEnv* p_raw_env, jobject p_instance);
static void push_warning(JNIEnv* p_raw_env, jobject p_instance);

// TODO: Use this method to get the JVM stacktrace when Godot will add the features to script https://github.com/godotengine/godot/pull/91006
String get_jvm_stacktrace(jni::Env& p_env);
};

}// namespace bridge
Expand Down
Loading

0 comments on commit 4e11878

Please sign in to comment.