Fix JNI Race Condition, Fix Release Builds and Fix Searching

This commit fixes JNI race conditions by usage of a mutex, fixes a bug in release builds due to ProGuard member obfuscation and fix searching by fixing the HeaderAdapter filter.

app/CMakeLists.txt

@@ -1,6 +1,7 @@
 cmake_minimum_required(VERSION 3.8)
 project(Skyline VERSION 0.3)
 
+set(BUILD_TESTS OFF)
 set(BUILD_TESTING OFF)
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
@@ -10,8 +11,10 @@ if (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
     add_compile_definitions(NDEBUG)
 endif()
 
+set(CMAKE_POLICY_DEFAULT_CMP0048 OLD)
 add_subdirectory("libraries/tinyxml2")
 add_subdirectory("libraries/fmt")
+set(CMAKE_POLICY_DEFAULT_CMP0048 NEW)
 
 include_directories(${source_DIR}/skyline)
 
@@ -19,6 +22,7 @@ add_library(skyline SHARED
         ${source_DIR}/main.cpp
         ${source_DIR}/skyline/common.cpp
         ${source_DIR}/skyline/nce.cpp
+        ${source_DIR}/skyline/jvm.cpp
         ${source_DIR}/skyline/gpu.cpp
         ${source_DIR}/skyline/gpu/display.cpp
         ${source_DIR}/skyline/gpu/parcel.cpp

app/proguard-rules.pro

@@ -6,4 +6,4 @@
     void writeObject(java.io.ObjectOutputStream);
     void readObject(java.io.ObjectInputStream);
 }
--keepclassmembernames class emu.skyline.GameActivity { *; }
+-keep class emu.skyline.GameActivity { *; }

app/src/main/cpp/main.cpp

@@ -1,11 +1,12 @@
 #include "skyline/common.h"
 #include "skyline/os.h"
+#include "skyline/jvm.h"
 #include <unistd.h>
-#include <android/native_window_jni.h>
 #include <csignal>
 
-bool Halt{};
+bool Halt;
-uint FaultCount{};
+uint FaultCount;
+std::mutex jniMtx;
 
 void signalHandler(int signal) {
     syslog(LOG_ERR, "Halting program due to signal: %s", strsignal(signal));
@@ -17,6 +18,9 @@ void signalHandler(int signal) {
 }
 
 extern "C" JNIEXPORT void Java_emu_skyline_GameActivity_executeRom(JNIEnv *env, jobject instance, jstring romJstring, jint romType, jint romFd, jint preferenceFd, jint logFd) {
+    Halt = false;
+    FaultCount = 0;
+
     std::signal(SIGTERM, signalHandler);
     std::signal(SIGSEGV, signalHandler);
     std::signal(SIGINT, signalHandler);
@@ -49,3 +53,11 @@ extern "C" JNIEXPORT void Java_emu_skyline_GameActivity_executeRom(JNIEnv *env,
     auto end = std::chrono::steady_clock::now();
     logger->Info("Done in: {} ms", (std::chrono::duration_cast<std::chrono::milliseconds>(end - start).count()));
 }
+
+extern "C" JNIEXPORT void Java_emu_skyline_GameActivity_lockMutex(JNIEnv *env, jobject instance) {
+    jniMtx.lock();
+}
+
+extern "C" JNIEXPORT void Java_emu_skyline_GameActivity_unlockMutex(JNIEnv *env, jobject instance) {
+    jniMtx.unlock();
+}

app/src/main/cpp/skyline/common.cpp

@@ -75,16 +75,6 @@ namespace skyline {
         logFile.flush();
     }
 
-    JvmManager::JvmManager(JNIEnv *env, jobject instance) : env(env), instance(instance), instanceClass(env->GetObjectClass(instance)) {}
-
-    jobject JvmManager::GetField(const char *key, const char *signature) {
-        return env->GetObjectField(instance, env->GetFieldID(instanceClass, key, signature));
-    }
-
-    bool JvmManager::CheckNull(const char *key, const char *signature) {
-        return env->IsSameObject(env->GetObjectField(instance, env->GetFieldID(instanceClass, key, signature)), nullptr);
-    }
-
     DeviceState::DeviceState(kernel::OS *os, std::shared_ptr<kernel::type::KProcess> &thisProcess, std::shared_ptr<kernel::type::KThread> &thisThread, std::shared_ptr<JvmManager> jvmManager, std::shared_ptr<Settings> settings, std::shared_ptr<Logger> logger)
         : os(os), jvmManager(std::move(jvmManager)), settings(std::move(settings)), logger(std::move(logger)), thisProcess(thisProcess), thisThread(thisThread) {
         // We assign these later as they use the state in their constructor and we don't want null pointers

app/src/main/cpp/skyline/common.h

@@ -333,64 +333,6 @@ namespace skyline {
         inline exception(const S &formatStr, Args &&... args) : runtime_error(fmt::format(formatStr, args...)) {}
     };
 
-    /**
-     * @brief The JvmManager class is used to simplify transactions with the Java component
-     */
-    class JvmManager {
-      public:
-        JNIEnv *env; //!< A pointer to the JNI environment
-        jobject instance; //!< A reference to the activity
-        jclass instanceClass; //!< The class of the activity
-
-        /**
-         * @param env A pointer to the JNI environment
-         * @param instance A reference to the activity
-         */
-        JvmManager(JNIEnv *env, jobject instance);
-
-        /**
-         * @brief Retrieves a specific field of the given type from the activity
-         * @tparam objectType The type of the object in the field
-         * @param key The name of the field in the activity class
-         * @return The contents of the field as objectType
-         */
-        template<typename objectType>
-        objectType GetField(const char *key) {
-            if constexpr(std::is_same<objectType, jboolean>())
-                return env->GetBooleanField(instance, env->GetFieldID(instanceClass, key, "Z"));
-            else if constexpr(std::is_same<objectType, jbyte>())
-                return env->GetByteField(instance, env->GetFieldID(instanceClass, key, "B"));
-            else if constexpr(std::is_same<objectType, jchar>())
-                return env->GetCharField(instance, env->GetFieldID(instanceClass, key, "C"));
-            else if constexpr(std::is_same<objectType, jshort>())
-                return env->GetShortField(instance, env->GetFieldID(instanceClass, key, "S"));
-            else if constexpr(std::is_same<objectType, jint>())
-                return env->GetIntField(instance, env->GetFieldID(instanceClass, key, "I"));
-            else if constexpr(std::is_same<objectType, jlong>())
-                return env->GetLongField(instance, env->GetFieldID(instanceClass, key, "J"));
-            else if constexpr(std::is_same<objectType, jfloat>())
-                return env->GetFloatField(instance, env->GetFieldID(instanceClass, key, "F"));
-            else if constexpr(std::is_same<objectType, jdouble>())
-                return env->GetDoubleField(instance, env->GetFieldID(instanceClass, key, "D"));
-        }
-
-        /**
-         * @brief Retrieves a specific field from the activity as a jobject
-         * @param key The name of the field in the activity class
-         * @param signature The signature of the field
-         * @return A jobject of the contents of the field
-         */
-        jobject GetField(const char *key, const char *signature);
-
-        /**
-         * @brief Checks if a specific field from the activity is null or not
-         * @param key The name of the field in the activity class
-         * @param signature The signature of the field
-         * @return If the field is null or not
-         */
-        bool CheckNull(const char *key, const char *signature);
-    };
-
     /**
      * @brief Returns the current time in nanoseconds
      * @return The current time in nanoseconds
@@ -400,6 +342,7 @@ namespace skyline {
     }
 
     class NCE;
+    class JvmManager;
     namespace gpu {
         class GPU;
     }

app/src/main/cpp/skyline/gpu.cpp

@@ -3,6 +3,7 @@
 #include "gpu/devices/nvhost_ctrl_gpu.h"
 #include "gpu/devices/nvhost_channel.h"
 #include "gpu/devices/nvhost_as_gpu.h"
+#include "jvm.h"
 #include <kernel/types/KProcess.h>
 #include <android/native_window_jni.h>
 
@@ -21,18 +22,17 @@ namespace skyline::gpu {
     }
 
     void GPU::Loop() {
-        if(state.jvmManager->CheckNull("surface", "Landroid/view/Surface;")) {
+        if (surfaceUpdate) {
-            while (state.jvmManager->CheckNull("surface", "Landroid/view/Surface;")) {
+            if (!state.jvmManager->CheckNull("surface", "Landroid/view/Surface;")) {
-                if(state.jvmManager->GetField<jboolean>("halt"))
+                window = ANativeWindow_fromSurface(state.jvmManager->env, state.jvmManager->GetField("surface", "Landroid/view/Surface;"));
-                    return;
+                ANativeWindow_acquire(window);
-                sched_yield();
+                resolution.width = static_cast<u32>(ANativeWindow_getWidth(window));
-            }
+                resolution.height = static_cast<u32>(ANativeWindow_getHeight(window));
-            window = ANativeWindow_fromSurface(state.jvmManager->env, state.jvmManager->GetField("surface", "Landroid/view/Surface;"));
+                format = ANativeWindow_getFormat(window);
-            ANativeWindow_acquire(window);
+            } else
-            resolution.width = static_cast<u32>(ANativeWindow_getWidth(window));
+                return;
-            resolution.height = static_cast<u32>(ANativeWindow_getHeight(window));
+        } else
-            format = ANativeWindow_getFormat(window);
+            surfaceUpdate = state.jvmManager->CheckNull("surface", "Landroid/view/Surface;");
-        }
         if (!bufferQueue.displayQueue.empty()) {
             auto &buffer = bufferQueue.displayQueue.front();
             bufferQueue.displayQueue.pop();

app/src/main/cpp/skyline/gpu.h

@@ -17,6 +17,7 @@ namespace skyline::gpu {
         u32 fdIndex{}; //!< Holds the index of a file descriptor
         std::unordered_map<device::NvDeviceType, std::shared_ptr<device::NvDevice>> deviceMap; //!< A map from a NvDeviceType to the NvDevice object
         std::unordered_map<u32, std::shared_ptr<device::NvDevice>> fdMap; //!< A map from an FD to a shared pointer to it's NvDevice object
+        bool surfaceUpdate{}; //!< If the surface needs to be updated
         double prevTime{};
 
       public:

app/src/main/cpp/skyline/jvm.cpp

@@ -0,0 +1,13 @@
+#include "jvm.h"
+
+namespace skyline {
+    JvmManager::JvmManager(JNIEnv *env, jobject instance) : env(env), instance(instance), instanceClass(env->GetObjectClass(instance)) {}
+
+    jobject JvmManager::GetField(const char *key, const char *signature) {
+        return env->GetObjectField(instance, env->GetFieldID(instanceClass, key, signature));
+    }
+
+    bool JvmManager::CheckNull(const char *key, const char *signature) {
+        return env->IsSameObject(env->GetObjectField(instance, env->GetFieldID(instanceClass, key, signature)), nullptr);
+    }
+}

app/src/main/cpp/skyline/jvm.h

@@ -0,0 +1,64 @@
+#pragma once
+
+#include <common.h>
+#include <jni.h>
+
+namespace skyline {
+    /**
+     * @brief The JvmManager class is used to simplify transactions with the Java component
+     */
+    class JvmManager {
+      public:
+        JNIEnv *env; //!< A pointer to the JNI environment
+        jobject instance; //!< A reference to the activity
+        jclass instanceClass; //!< The class of the activity
+
+        /**
+         * @param env A pointer to the JNI environment
+         * @param instance A reference to the activity
+         */
+        JvmManager(JNIEnv *env, jobject instance);
+
+        /**
+         * @brief Retrieves a specific field of the given type from the activity
+         * @tparam objectType The type of the object in the field
+         * @param key The name of the field in the activity class
+         * @return The contents of the field as objectType
+         */
+        template<typename objectType>
+        inline objectType GetField(const char *key) {
+            if constexpr(std::is_same<objectType, jboolean>())
+                return env->GetBooleanField(instance, env->GetFieldID(instanceClass, key, "Z"));
+            else if constexpr(std::is_same<objectType, jbyte>())
+                return env->GetByteField(instance, env->GetFieldID(instanceClass, key, "B"));
+            else if constexpr(std::is_same<objectType, jchar>())
+                return env->GetCharField(instance, env->GetFieldID(instanceClass, key, "C"));
+            else if constexpr(std::is_same<objectType, jshort>())
+                return env->GetShortField(instance, env->GetFieldID(instanceClass, key, "S"));
+            else if constexpr(std::is_same<objectType, jint>())
+                return env->GetIntField(instance, env->GetFieldID(instanceClass, key, "I"));
+            else if constexpr(std::is_same<objectType, jlong>())
+                return env->GetLongField(instance, env->GetFieldID(instanceClass, key, "J"));
+            else if constexpr(std::is_same<objectType, jfloat>())
+                return env->GetFloatField(instance, env->GetFieldID(instanceClass, key, "F"));
+            else if constexpr(std::is_same<objectType, jdouble>())
+                return env->GetDoubleField(instance, env->GetFieldID(instanceClass, key, "D"));
+        }
+
+        /**
+         * @brief Retrieves a specific field from the activity as a jobject
+         * @param key The name of the field in the activity class
+         * @param signature The signature of the field
+         * @return A jobject of the contents of the field
+         */
+        jobject GetField(const char *key, const char *signature);
+
+        /**
+         * @brief Checks if a specific field from the activity is null or not
+         * @param key The name of the field in the activity class
+         * @param signature The signature of the field
+         * @return If the field is null or not
+         */
+        bool CheckNull(const char *key, const char *signature);
+    };
+}

app/src/main/cpp/skyline/nce.cpp

@@ -1,9 +1,11 @@
 #include <sched.h>
 #include <linux/uio.h>
 #include <linux/elf.h>
-#include <os.h>
+#include "os.h"
+#include "jvm.h"
 
 extern bool Halt;
+extern std::mutex jniMtx;
 
 namespace skyline {
     void NCE::ReadRegisters(user_pt_regs &registers, pid_t pid) const {
@@ -32,6 +34,7 @@ namespace skyline {
     void NCE::Execute() {
         int status = 0;
         while (!Halt && !state.os->processMap.empty()) {
+            jniMtx.lock();
             for (const auto &process : state.os->processMap) {
                 state.os->thisProcess = process.second;
                 state.os->thisThread = process.second->threadMap.at(process.first);
@@ -88,6 +91,7 @@ namespace skyline {
             state.os->serviceManager.Loop();
             state.gpu->Loop();
             Halt = state.jvmManager->GetField<jboolean>("halt");
+            jniMtx.unlock();
         }
         for (const auto &process : state.os->processMap) {
             state.os->KillThread(process.first);

app/src/main/java/emu/skyline/GameActivity.kt

@@ -28,6 +28,8 @@ class GameActivity : AppCompatActivity(), SurfaceHolder.Callback, InputQueue.Cal
     private var halt: Boolean = false
 
     private external fun executeRom(romString: String, romType: Int, romFd: Int, preferenceFd: Int, logFd: Int)
+    private external fun lockMutex()
+    private external fun unlockMutex()
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
@@ -62,7 +64,9 @@ class GameActivity : AppCompatActivity(), SurfaceHolder.Callback, InputQueue.Cal
 
     override fun surfaceCreated(holder: SurfaceHolder?) {
         Log.d("surfaceCreated", "Holder: ${holder.toString()}")
+        lockMutex()
         surface = holder!!.surface
+        unlockMutex()
     }
 
     override fun surfaceChanged(holder: SurfaceHolder?, format: Int, width: Int, height: Int) {
@@ -71,7 +75,9 @@ class GameActivity : AppCompatActivity(), SurfaceHolder.Callback, InputQueue.Cal
 
     override fun surfaceDestroyed(holder: SurfaceHolder?) {
         Log.d("surfaceDestroyed", "Holder: ${holder.toString()}")
+        lockMutex()
         surface = null
+        unlockMutex()
     }
 
     override fun onInputQueueCreated(queue: InputQueue?) {

app/src/main/java/emu/skyline/adapter/HeaderAdapter.kt

@@ -73,6 +73,7 @@ internal abstract class HeaderAdapter<ItemType : BaseItem?, HeaderType : BaseHea
     open fun load(file: File) {
         val fileObj = FileInputStream(file)
         val input = ObjectInputStream(fileObj)
+        @Suppress("UNCHECKED_CAST")
         elementArray = input.readObject() as ArrayList<BaseElement?>
         input.close()
         fileObj.close()
@@ -126,7 +127,7 @@ internal abstract class HeaderAdapter<ItemType : BaseItem?, HeaderType : BaseHea
                             keyArray.add(item.key()!!.toLowerCase(Locale.getDefault()))
                         }
                     }
-                    val topResults = FuzzySearch.extractTop(searchTerm, keyArray, searchTerm.length)
+                    val topResults = FuzzySearch.extractSorted(searchTerm, keyArray)
                     val avgScore: Int = topResults.sumBy { it.score } / topResults.size
                     for (result in topResults)
                         if (result.score > avgScore)
@@ -139,6 +140,7 @@ internal abstract class HeaderAdapter<ItemType : BaseItem?, HeaderType : BaseHea
 
             override fun publishResults(charSequence: CharSequence, results: FilterResults) {
                 if (results.values is ArrayList<*>) {
+                    @Suppress("UNCHECKED_CAST")
                     visibleArray = results.values as ArrayList<Int>
                     notifyDataSetChanged()
                 }