Stop leaving JNI exceptions pending across the native boundary

daemon/src/main/jni/dex2oat.cpp

@@ -106,6 +106,12 @@ extern "C" JNIEXPORT jboolean JNICALL
 Java_org_matrix_vector_daemon_env_Dex2OatServer_setSockCreateContext(JNIEnv *env, jclass,
                                                                      jstring contextStr) {
     const char *context = contextStr ? env->GetStringUTFChars(contextStr, nullptr) : nullptr;
+    if (contextStr && !context) {
+        // Only OutOfMemoryError puts us here, and it is pending: returning into Java with it still
+        // set would surface it at the next unrelated call.
+        env->ExceptionClear();
+        return false;
+    }
     int ret = setsockcreatecon_raw(context);
     if (context) env->ReleaseStringUTFChars(contextStr, context);
     return ret == 0;

daemon/src/main/jni/logcat.cpp

@@ -1,5 +1,9 @@
 #include "logcat.h"
 
+#include <utils/jni_helper.hpp>
+
+#include "logging.h"
+
 #include <android/log.h>
 #include <jni.h>
 #include <sys/system_properties.h>
@@ -241,8 +245,14 @@ void Logcat::Run() {
 
 extern "C" JNIEXPORT void JNICALL
 Java_org_matrix_vector_daemon_env_LogcatMonitor_runLogcat(JNIEnv* env, jobject thiz) {
-    jclass clazz = env->GetObjectClass(thiz);
-    jmethodID method = env->GetMethodID(clazz, "refreshFd", "(Z)I");
+    auto clazz = lsplant::JNI_GetObjectClass(env, thiz);
+    auto method = lsplant::JNI_GetMethodID(env, clazz, "refreshFd", "(Z)I");
+    if (!method) {
+        // The wrapper has already logged and cleared the NoSuchMethodError. Running with a null
+        // method id would abort inside the first refresh instead of saying why.
+        LOGE("LogcatMonitor.refreshFd is missing; not starting the log reader");
+        return;
+    }
     Logcat daemon(env, thiz, method);
     daemon.Run();
 }

daemon/src/main/jni/obfuscation.cpp

@@ -119,30 +119,30 @@ static void ensureInitialized(JNIEnv *env) {
     });
 }
 
+// Through the lsplant wrappers rather than raw JNI: each one clears a pending exception and logs
+// the Java stack behind it, which is what a failed lookup here would otherwise cost. Returning a
+// null jclass while leaving NoClassDefFoundError pending -- as the raw form did -- hands the next
+// JNI call undefined behaviour. They also return scoped references, so the local refs this loop
+// used to leak per entry are released on the spot.
 static jobject stringMapToJavaHashMap(JNIEnv *env, const std::map<std::string, std::string> &map) {
-    jclass mapClass = env->FindClass("java/util/HashMap");
-    if (mapClass == nullptr) return nullptr;
+    auto map_class = lsplant::JNI_FindClass(env, "java/util/HashMap");
+    if (!map_class) return nullptr;
 
-    jmethodID init = env->GetMethodID(mapClass, "<init>", "()V");
-    jobject hashMap = env->NewObject(mapClass, init);
-    jmethodID put = env->GetMethodID(mapClass, "put",
-                                     "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
+    auto init = lsplant::JNI_GetMethodID(env, map_class, "<init>", "()V");
+    auto put = lsplant::JNI_GetMethodID(env, map_class, "put",
+                                        "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
+    if (!init || !put) return nullptr;
 
-    for (const auto &[key, value] : map) {
-        jstring keyJava = env->NewStringUTF(key.c_str());
-        jstring valueJava = env->NewStringUTF(value.c_str());
-
-        env->CallObjectMethod(hashMap, put, keyJava, valueJava);
+    auto hash_map = lsplant::JNI_NewObject(env, map_class, init);
+    if (!hash_map) return nullptr;
 
-        env->DeleteLocalRef(keyJava);
-        env->DeleteLocalRef(valueJava);
+    for (const auto &[key, value] : map) {
+        auto key_java = lsplant::JNI_NewStringUTF(env, key);
+        auto value_java = lsplant::JNI_NewStringUTF(env, value);
+        lsplant::JNI_CallObjectMethod(env, hash_map, put, key_java, value_java);
     }
 
-    jobject hashMapGlobal = env->NewGlobalRef(hashMap);
-    env->DeleteLocalRef(hashMap);
-    env->DeleteLocalRef(mapClass);
-
-    return hashMapGlobal;
+    return lsplant::JNI_NewGlobalRef(env, hash_map);
 }
 
 extern "C" JNIEXPORT jobject JNICALL

native/include/core/context.h

@@ -145,27 +145,46 @@ class Context {
      *
      * A utility for internal communication between the native and Java layers.
      *
+     * A Java method that throws does not unwind into C++: the exception is left *pending* on this
+     * thread, and until it is cleared almost every JNI function is illegal to call. With CheckJNI
+     * on, the next one aborts the process; without it, the exception stays pending until control
+     * returns to Java and is then thrown somewhere with nothing to do with us — typically inside
+     * the starting application, where no stack frame points back here. So the exception is reported
+     * where it happened, with the Java stack that only exists at this moment, and whether the call
+     * arrived is something the caller can act on.
+     *
      * @tparam Args Argument types for the method call.
      * @param env The JNI environment.
      * @param method_name The name of the static method.
      * @param method_sig The JNI signature of the method.
      * @param args The arguments to pass to the method.
+     * @return Whether the method was found and returned without throwing.
      */
     template <typename... Args>
-    void FindAndCall(JNIEnv *env, std::string_view method_name, std::string_view method_sig,
+    bool FindAndCall(JNIEnv *env, std::string_view method_name, std::string_view method_sig,
                      Args &&...args) const {
         if (!entry_class_) {
             LOGE("Cannot call method '{}', entry class is null", method_name.data());
-            return;
+            return false;
         }
         jmethodID mid = lsplant::JNI_GetStaticMethodID(env, entry_class_, method_name, method_sig);
-        if (mid) {
-            env->CallStaticVoidMethod(entry_class_, mid,
-                                      lsplant::UnwrapScope(std::forward<Args>(args))...);
-        } else {
+        if (!mid) {
             LOGE("Static method '{}' with signature '{}' not found", method_name.data(),
                  method_sig.data());
+            return false;
+        }
+        env->CallStaticVoidMethod(entry_class_, mid,
+                                  lsplant::UnwrapScope(std::forward<Args>(args))...);
+        // ClearException, not ExceptionDescribe: the latter writes the trace to stderr, and a
+        // process forked from the zygote has nowhere for stderr to go, so the trace is simply lost.
+        // This asks Java to render it and logs the result under our own tag, which is the only
+        // place the stack still exists to be read.
+        if (auto trace = lsplant::ClearException(env)) {
+            LOGE("Java entry '{}' threw:\n{}", method_name.data(),
+                 lsplant::JUTFString(env, trace.get()).get());
+            return false;
         }
+        return true;
     }
 
     // --- Virtual methods for platform-specific implementations ---

native/include/jni/jni_bridge.h

@@ -71,7 +71,9 @@ inline bool RegisterNativeMethodsInternal(JNIEnv *env, std::string_view class_na
         LOGF("JNI class not found: {}", class_name.data());
         return false;
     }
-    return env->RegisterNatives(clazz.get(), methods, method_count) == JNI_OK;
+    // Wrapped: a failed registration throws NoSuchMethodError, and returning false while that
+    // exception is still pending would hand the next JNI call undefined behaviour.
+    return lsplant::JNI_RegisterNatives(env, clazz, methods, method_count) == JNI_OK;
 }
 
 // A helper cast for the native method function pointers.

native/src/jni/resources_hook.cpp

@@ -123,17 +123,20 @@ VECTOR_DEF_NATIVE_METHOD(jboolean, ResourcesHook, initXResourcesNative) {
     std::string x_resources_jni_name = "L" + x_resources_class_name + ";";
     std::replace(x_resources_jni_name.begin(), x_resources_jni_name.end(), '.', '/');
 
-    methodXResourcesTranslateResId = env->GetStaticMethodID(
-        classXResources, "translateResId",
-        fmt::format("(I{}Landroid/content/res/Resources;)I", x_resources_jni_name).c_str());
+    // Wrapped, like the lookup below: a missing method throws NoSuchMethodError, and the raw form
+    // returned JNI_FALSE to Java with that exception still pending, so the caller saw a throw where
+    // it had asked for a boolean.
+    methodXResourcesTranslateResId = lsplant::JNI_GetStaticMethodID(
+        env, classXResources, "translateResId",
+        fmt::format("(I{}Landroid/content/res/Resources;)I", x_resources_jni_name));
     if (!methodXResourcesTranslateResId) {
         LOGE("Failed to find method: XResources.translateResId");
         return JNI_FALSE;
     }
 
-    methodXResourcesTranslateAttrId = env->GetStaticMethodID(
-        classXResources, "translateAttrId",
-        fmt::format("(Ljava/lang/String;{})I", x_resources_jni_name).c_str());
+    methodXResourcesTranslateAttrId = lsplant::JNI_GetStaticMethodID(
+        env, classXResources, "translateAttrId",
+        fmt::format("(Ljava/lang/String;{})I", x_resources_jni_name));
     if (!methodXResourcesTranslateAttrId) {
         LOGE("Failed to find method: XResources.translateAttrId");
         return JNI_FALSE;
@@ -173,9 +176,10 @@ VECTOR_DEF_NATIVE_METHOD(jobject, ResourcesHook, buildDummyClassLoader, jobject
 
     // Cache the class and constructor for InMemoryDexClassLoader.
     static auto in_memory_classloader =
-        (jclass)env->NewGlobalRef(env->FindClass("dalvik/system/InMemoryDexClassLoader"));
-    static jmethodID initMid = env->GetMethodID(in_memory_classloader, "<init>",
-                                                "(Ljava/nio/ByteBuffer;Ljava/lang/ClassLoader;)V");
+        lsplant::JNI_NewGlobalRef(env, lsplant::JNI_FindClass(env, "dalvik/system/InMemoryDexClassLoader"));
+    static jmethodID initMid = lsplant::JNI_GetMethodID(
+        env, in_memory_classloader, "<init>", "(Ljava/nio/ByteBuffer;Ljava/lang/ClassLoader;)V");
+    if (!in_memory_classloader || !initMid) return nullptr;
 
     DexBuilder dex_file;
 
@@ -195,10 +199,14 @@ VECTOR_DEF_NATIVE_METHOD(jobject, ResourcesHook, buildDummyClassLoader, jobject
     slicer::MemView image{dex_file.CreateImage()};
 
     // Wrap the memory buffer in a Java ByteBuffer.
-    auto dex_buffer = env->NewDirectByteBuffer(const_cast<void *>(image.ptr()), image.size());
-
-    // Create and return a new InMemoryDexClassLoader instance.
-    return env->NewObject(in_memory_classloader, initMid, dex_buffer, parent);
+    auto dex_buffer = lsplant::JNI_NewDirectByteBuffer(env, const_cast<void *>(image.ptr()),
+                                                       image.size());
+    if (!dex_buffer) return nullptr;
+
+    // Create and return a new InMemoryDexClassLoader instance. Released from its scope because it
+    // is handed straight back to Java.
+    return lsplant::JNI_NewObject(env, in_memory_classloader, initMid, dex_buffer, parent)
+        .release();
 }
 
 /**

zygisk/src/main/cpp/module.cpp

@@ -361,12 +361,19 @@ void VectorModule::postAppSpecialize(const zygisk::AppSpecializeArgs *args) {
     this->SetupEntryClass(env_);
 
     // Hand off control to the Java side of the framework.
-    this->FindAndCall(
+    bool entered = this->FindAndCall(
         env_, "forkCommon", "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V",
         JNI_FALSE, JNI_FALSE, args->nice_name, args->app_data_dir, binder.get(), is_manager_app_);
 
-    LOGV("Injected Vector framework into '{}'.", nice_name_str.get());
-    SetAllowUnload(false);  // We are injected, PREVENT module unloading.
+    if (entered) {
+        LOGV("Injected Vector framework into '{}'.", nice_name_str.get());
+    } else {
+        LOGE("Framework entry failed in '{}'; this process runs without Xposed.",
+             nice_name_str.get());
+    }
+    // Unconditionally: the ART and JNI hooks were installed before the entry ran, and their
+    // trampolines point into this library. Letting it be unloaded now would leave them dangling.
+    SetAllowUnload(false);
 }
 
 void VectorModule::preServerSpecialize(zygisk::ServerSpecializeArgs *args) {
@@ -448,13 +455,18 @@ void VectorModule::postServerSpecialize(const zygisk::ServerSpecializeArgs *args
     this->SetupEntryClass(env_);
 
     auto system_name = lsplant::ScopedLocalRef(env_, env_->NewStringUTF("system"));
-    this->FindAndCall(env_, "forkCommon",
-                      "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V", JNI_TRUE,
-                      is_late_inject, system_name.get(), nullptr, manager_binder.get(),
-                      is_manager_app_);
+    bool entered = this->FindAndCall(
+        env_, "forkCommon", "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V",
+        JNI_TRUE, is_late_inject, system_name.get(), nullptr, manager_binder.get(),
+        is_manager_app_);
 
-    LOGI("Injected Vector framework into system_server.");
-    SetAllowUnload(false);  // We are injected, PREVENT module unloading.
+    if (entered) {
+        LOGI("Injected Vector framework into system_server.");
+    } else {
+        LOGE("Framework entry failed in system_server; it runs without Xposed.");
+    }
+    // See postAppSpecialize: the hooks outlive a failed entry, so the library must stay.
+    SetAllowUnload(false);
 }
 
 void VectorModule::SetAllowUnload(bool unload) {