diff --git a/Common/File/AndroidStorage.cpp b/Common/File/AndroidStorage.cpp index fdb8a21372..08dcdf8075 100644 --- a/Common/File/AndroidStorage.cpp +++ b/Common/File/AndroidStorage.cpp @@ -1,4 +1,5 @@ #include +#include #include "Common/File/AndroidStorage.h" #include "Common/StringUtils.h" @@ -26,14 +27,37 @@ static jmethodID filePathGetFreeStorageSpace; static jmethodID isExternalStoragePreservedLegacy; static jmethodID computeRecursiveDirectorySize; +// A global ref owned by whoever calls Android_StorageSetActivity, who may replace it and delete the old +// one at any time. So we only look at it under the mutex, and call with a local ref of our own. static jobject g_nativeActivity; +static std::mutex g_nativeActivityLock; static jclass g_classContentUri; void Android_StorageSetActivity(jobject nativeActivity) { + std::lock_guard guard(g_nativeActivityLock); g_nativeActivity = nativeActivity; } +// Only for the one call that doesn't pass the activity to Java. Anything that does must use +// AcquireActivity and check the ref it got - between a check here and the call, the UI thread +// can swap the activity out from under us. +static bool HasActivity() { + std::lock_guard guard(g_nativeActivityLock); + return g_nativeActivity != nullptr; +} + +// Returns a local ref, to be freed by the caller's JNILocalFrame. Can be null if we just lost the activity. +static jobject AcquireActivity(JNIEnv *env) { + std::lock_guard guard(g_nativeActivityLock); + return g_nativeActivity ? env->NewLocalRef(g_nativeActivity) : nullptr; +} + void Android_RegisterStorageCallbacks(JNIEnv * env, jobject obj) { + if (g_classContentUri) { + // Already done. This gets called for each new activity, but the class and the method IDs stay + // valid, and other threads may be using them right now. + return; + } jclass localClass = env->FindClass("org/ppsspp/ppsspp/ContentUri"); _dbg_assert_(localClass); @@ -75,7 +99,7 @@ void Android_UnregisterStorageCallbacks(JNIEnv * env) { env->DeleteGlobalRef(g_classContentUri); g_classContentUri = nullptr; } - g_nativeActivity = nullptr; + Android_StorageSetActivity(nullptr); openContentUri = nullptr; listContentUriDir = nullptr; contentUriCreateFile = nullptr; @@ -97,12 +121,6 @@ bool Android_IsContentUri(std::string_view filename) { } int Android_OpenContentUriFd(std::string_view filename, Android_OpenContentUriMode mode) { - if (!g_nativeActivity) { - // Hit this in shortcut creation. - ERROR_LOG(Log::IO, "Android_OpenContentUriFd: No native activity"); - return -1; - } - /* // Should breakpoint here to try to find and move as many of these off the EmuThread as possible if (!strcmp(GetCurrentThreadName(), "EmuThread")) { @@ -118,6 +136,12 @@ int Android_OpenContentUriFd(std::string_view filename, Android_OpenContentUriMo auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + // Hit this in shortcut creation. + ERROR_LOG(Log::IO, "Android_OpenContentUriFd: No native activity"); + return -1; + } const char *modeStr = ""; switch (mode) { case Android_OpenContentUriMode::READ: modeStr = "r"; break; @@ -126,74 +150,80 @@ int Android_OpenContentUriFd(std::string_view filename, Android_OpenContentUriMo } jstring j_filename = env->NewStringUTF(fname.c_str()); jstring j_mode = env->NewStringUTF(modeStr); - int fd = env->CallStaticIntMethod(g_classContentUri, openContentUri, g_nativeActivity, j_filename, j_mode); + int fd = env->CallStaticIntMethod(g_classContentUri, openContentUri, activity, j_filename, j_mode); return fd; } StorageError Android_CreateDirectory(const std::string &rootTreeUri, const std::string &dirName) { - if (!g_nativeActivity) { - return StorageError::UNKNOWN; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return StorageError::UNKNOWN; + } jstring paramRoot = env->NewStringUTF(rootTreeUri.c_str()); jstring paramDirName = env->NewStringUTF(dirName.c_str()); - return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriCreateDirectory, g_nativeActivity, paramRoot, paramDirName)); + return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriCreateDirectory, activity, paramRoot, paramDirName)); } StorageError Android_CreateFile(const std::string &parentTreeUri, const std::string &fileName) { - if (!g_nativeActivity) { - return StorageError::UNKNOWN; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return StorageError::UNKNOWN; + } jstring paramRoot = env->NewStringUTF(parentTreeUri.c_str()); jstring paramFileName = env->NewStringUTF(fileName.c_str()); - return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriCreateFile, g_nativeActivity, paramRoot, paramFileName)); + return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriCreateFile, activity, paramRoot, paramFileName)); } StorageError Android_CopyFile(const std::string &fileUri, const std::string &destParentUri) { - if (!g_nativeActivity) { - return StorageError::UNKNOWN; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return StorageError::UNKNOWN; + } jstring paramFileName = env->NewStringUTF(fileUri.c_str()); jstring paramDestParentUri = env->NewStringUTF(destParentUri.c_str()); - return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriCopyFile, g_nativeActivity, paramFileName, paramDestParentUri)); + return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriCopyFile, activity, paramFileName, paramDestParentUri)); } StorageError Android_MoveFile(const std::string &fileUri, const std::string &srcParentUri, const std::string &destParentUri) { - if (!g_nativeActivity) { - return StorageError::UNKNOWN; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return StorageError::UNKNOWN; + } jstring paramFileName = env->NewStringUTF(fileUri.c_str()); jstring paramSrcParentUri = env->NewStringUTF(srcParentUri.c_str()); jstring paramDestParentUri = env->NewStringUTF(destParentUri.c_str()); - return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriMoveFile, g_nativeActivity, paramFileName, paramSrcParentUri, paramDestParentUri)); + return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriMoveFile, activity, paramFileName, paramSrcParentUri, paramDestParentUri)); } StorageError Android_RemoveFile(const std::string &fileUri) { - if (!g_nativeActivity) { - return StorageError::UNKNOWN; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return StorageError::UNKNOWN; + } jstring paramFileName = env->NewStringUTF(fileUri.c_str()); - return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriRemoveFile, g_nativeActivity, paramFileName)); + return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriRemoveFile, activity, paramFileName)); } StorageError Android_RenameFileTo(const std::string &fileUri, const std::string &newName) { - if (!g_nativeActivity) { - return StorageError::UNKNOWN; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return StorageError::UNKNOWN; + } jstring paramFileUri = env->NewStringUTF(fileUri.c_str()); jstring paramNewName = env->NewStringUTF(newName.c_str()); - return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriRenameFileTo, g_nativeActivity, paramFileUri, paramNewName)); + return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriRenameFileTo, activity, paramFileUri, paramNewName)); } // NOTE: Does not set fullName - you're supposed to already know it. @@ -233,14 +263,15 @@ static bool ParseFileInfo(std::string_view line, File::FileInfo *fileInfo) { } bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *fileInfo) { - if (!g_nativeActivity) { - return false; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return false; + } jstring paramFileUri = env->NewStringUTF(fileUri.c_str()); - jstring str = (jstring)env->CallStaticObjectMethod(g_classContentUri, contentUriGetFileInfo, g_nativeActivity, paramFileUri); + jstring str = (jstring)env->CallStaticObjectMethod(g_classContentUri, contentUriGetFileInfo, activity, paramFileUri); if (!str) { return false; } @@ -255,30 +286,32 @@ bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *fileInfo) { } bool Android_FileExists(const std::string &fileUri) { - if (!g_nativeActivity) { - return false; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return false; + } jstring paramFileUri = env->NewStringUTF(fileUri.c_str()); - bool exists = env->CallStaticBooleanMethod(g_classContentUri, contentUriFileExists, g_nativeActivity, paramFileUri); + bool exists = env->CallStaticBooleanMethod(g_classContentUri, contentUriFileExists, activity, paramFileUri); return exists; } std::vector Android_ListContentUri(const std::string &uri, const std::string &prefix, bool *exists) { - if (!g_nativeActivity) { + auto env = getEnv(); + JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { *exists = false; return {}; } - auto env = getEnv(); - JNILocalFrame frame(env); *exists = true; double start = time_now_d(); jstring param = env->NewStringUTF(uri.c_str()); jstring filenamePrefix = prefix.empty() ? nullptr : env->NewStringUTF(prefix.c_str()); - jobject retval = env->CallStaticObjectMethod(g_classContentUri, listContentUriDir, g_nativeActivity, param, filenamePrefix); + jobject retval = env->CallStaticObjectMethod(g_classContentUri, listContentUriDir, activity, param, filenamePrefix); jobjectArray fileList = (jobjectArray)retval; std::vector items; @@ -317,23 +350,25 @@ std::vector Android_ListContentUri(const std::string &uri, const } int64_t Android_GetFreeSpaceByContentUri(const std::string &uri) { - if (!g_nativeActivity) { - return false; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return false; + } jstring param = env->NewStringUTF(uri.c_str()); - return env->CallStaticLongMethod(g_classContentUri, contentUriGetFreeStorageSpace, g_nativeActivity, param); + return env->CallStaticLongMethod(g_classContentUri, contentUriGetFreeStorageSpace, activity, param); } // Hm, this is never used? We use statvfs instead. int64_t Android_GetFreeSpaceByFilePath(const std::string &filePath) { - if (!g_nativeActivity) { - return false; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return false; + } if (System_GetPropertyInt(SYSPROP_SYSTEMVERSION) < 26) { // This is available from Android O. @@ -341,20 +376,21 @@ int64_t Android_GetFreeSpaceByFilePath(const std::string &filePath) { } jstring param = env->NewStringUTF(filePath.c_str()); - return env->CallStaticLongMethod(g_classContentUri, filePathGetFreeStorageSpace, g_nativeActivity, param); + return env->CallStaticLongMethod(g_classContentUri, filePathGetFreeStorageSpace, activity, param); } int64_t Android_ComputeRecursiveDirectorySize(const std::string &uri) { - if (!g_nativeActivity) { - return false; - } auto env = getEnv(); JNILocalFrame frame(env); + jobject activity = AcquireActivity(env); + if (!activity) { + return false; + } jstring param = env->NewStringUTF(uri.c_str()); double start = time_now_d(); - int64_t size = env->CallStaticLongMethod(g_classContentUri, computeRecursiveDirectorySize, g_nativeActivity, param); + int64_t size = env->CallStaticLongMethod(g_classContentUri, computeRecursiveDirectorySize, activity, param); double elapsed = time_now_d() - start; INFO_LOG(Log::IO, "ComputeRecursiveDirectorySize(%s) in %0.3f s", uri.c_str(), elapsed); @@ -362,7 +398,7 @@ int64_t Android_ComputeRecursiveDirectorySize(const std::string &uri) { } bool Android_IsExternalStoragePreservedLegacy() { - if (!g_nativeActivity) { + if (!HasActivity()) { return false; } auto env = getEnv(); diff --git a/Common/Render/Text/draw_text_android.cpp b/Common/Render/Text/draw_text_android.cpp index 4f0ea7742e..5a31d7dbe9 100644 --- a/Common/Render/Text/draw_text_android.cpp +++ b/Common/Render/Text/draw_text_android.cpp @@ -16,8 +16,6 @@ #include -jobject TextDrawerAndroid::activity_; - TextDrawerAndroid::TextDrawerAndroid(Draw::DrawContext *draw) : TextDrawer(draw) { auto env = getEnv(); const char *textRendererClassName = "org/ppsspp/ppsspp/TextRenderer"; @@ -61,9 +59,10 @@ void TextDrawerAndroid::SetOrCreateFont(const FontStyle &style) { auto fontIter = allocatedFonts_.find(filename); if (fontIter == allocatedFonts_.end()) { auto env = getEnv(); + JNILocalFrame frame(env); + jobject activity = Android_GetActivity(env); jstring jstr = env->NewStringUTF(filename.c_str()); - int fontId = env->CallStaticIntMethod(cls_textRenderer, method_allocFont, activity_, jstr); - env->DeleteLocalRef(jstr); + int fontId = env->CallStaticIntMethod(cls_textRenderer, method_allocFont, activity, jstr); if (fontId >= 0) { allocatedFonts_[filename] = fontId; diff --git a/Common/Render/Text/draw_text_android.h b/Common/Render/Text/draw_text_android.h index a097694266..75dc56b5f0 100644 --- a/Common/Render/Text/draw_text_android.h +++ b/Common/Render/Text/draw_text_android.h @@ -21,7 +21,6 @@ public: bool IsReady() const override; void SetOrCreateFont(const FontStyle &style) override; - static void SetActivity(jobject activity) { activity_ = activity; } bool DrawStringBitmap(std::vector &bitmapData, TextStringEntry &entry, Draw::DataFormat texFormat, std::string_view str, int align, bool fullColor) override; protected: @@ -32,7 +31,6 @@ protected: private: // JNI functions - static jobject activity_; jclass cls_textRenderer; jmethodID method_allocFont; jmethodID method_freeAllFonts; diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index b4fe6fc4c1..6143c270ba 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -164,7 +164,9 @@ static jmethodID postCommand; static jmethodID getDebugString; static jmethodID getNativeCrashHistory; +// Global ref. Written on the UI thread under g_activityLock. Other threads go through Android_GetActivity. static jobject ppssppActivity; +static std::mutex g_activityLock; static std::atomic exitRenderLoop; static std::atomic renderLoopRunning; @@ -254,6 +256,11 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *pjvm, void *reserved) { return JNI_VERSION_1_6; } +jobject Android_GetActivity(JNIEnv *env) { + std::lock_guard guard(g_activityLock); + return ppssppActivity ? env->NewLocalRef(ppssppActivity) : nullptr; +} + static void PushCommand(std::string_view cmd, std::string_view param) { std::lock_guard guard(frameCommandLock); g_frameCommands.emplace_back(std::string(cmd), std::string(param)); @@ -455,33 +462,39 @@ bool System_GetPropertyBool(SystemProperty prop) { } std::string Android_GetInputDeviceDebugString() { - if (!ppssppActivity) { + auto env = getEnv(); + JNILocalFrame frame(env); + jobject activity = Android_GetActivity(env); + if (!activity) { return "(N/A)"; } - auto env = getEnv(); jstring jparam = env->NewStringUTF("InputDevice"); - jstring jstr = (jstring)env->CallObjectMethod(ppssppActivity, getDebugString, jparam); + jstring jstr = (jstring)env->CallObjectMethod(activity, getDebugString, jparam); if (!jstr) { - env->DeleteLocalRef(jparam); return "(N/A)"; } const char *charArray = env->GetStringUTFChars(jstr, nullptr); - std::string retVal = charArray; - env->ReleaseStringUTFChars(jstr, charArray); - env->DeleteLocalRef(jstr); - env->DeleteLocalRef(jparam); + std::string retVal = charArray ? charArray : "(N/A)"; + if (charArray) { + env->ReleaseStringUTFChars(jstr, charArray); + } return retVal; } std::vector Android_GetNativeCrashHistory(int maxEntries) { std::vector crashHistory; - if (!ppssppActivity || !getNativeCrashHistory) { + if (!getNativeCrashHistory) { return crashHistory; } auto env = getEnv(); - jobject jlist = env->CallObjectMethod(ppssppActivity, getNativeCrashHistory, maxEntries); + JNILocalFrame frame(env); + jobject activity = Android_GetActivity(env); + if (!activity) { + return crashHistory; + } + jobject jlist = env->CallObjectMethod(activity, getNativeCrashHistory, maxEntries); if (!jlist) { return crashHistory; } @@ -513,8 +526,13 @@ std::string GetJavaString(JNIEnv *env, jstring jstr) { } extern "C" void Java_org_ppsspp_ppsspp_PpssppActivity_registerCallbacks(JNIEnv *env, jobject obj) { - ppssppActivity = env->NewGlobalRef(obj); - TextDrawerAndroid::SetActivity(ppssppActivity); + jobject newActivity = env->NewGlobalRef(obj); + jobject oldActivity; + { + std::lock_guard guard(g_activityLock); + oldActivity = ppssppActivity; + ppssppActivity = newActivity; + } postCommand = env->GetMethodID(env->GetObjectClass(obj), "postCommand", "(Ljava/lang/String;Ljava/lang/String;)V"); getDebugString = env->GetMethodID(env->GetObjectClass(obj), "getDebugString", "(Ljava/lang/String;)Ljava/lang/String;"); getNativeCrashHistory = env->GetMethodID(env->GetObjectClass(obj), "getNativeCrashHistory", "(I)Ljava/util/ArrayList;"); @@ -524,13 +542,26 @@ extern "C" void Java_org_ppsspp_ppsspp_PpssppActivity_registerCallbacks(JNIEnv * // It's OK if getNativeCrashHistory is missing. Android_RegisterStorageCallbacks(env, obj); - Android_StorageSetActivity(ppssppActivity); + Android_StorageSetActivity(newActivity); + + // An activity that gets recreated (rotation, resize) never unregisters, so this is where we let go + // of it. Used to leak the whole activity, once per rotation. + if (oldActivity) { + env->DeleteGlobalRef(oldActivity); + } } extern "C" void Java_org_ppsspp_ppsspp_PpssppActivity_unregisterCallbacks(JNIEnv *env, jobject obj) { Android_StorageSetActivity(nullptr); - env->DeleteGlobalRef(ppssppActivity); - ppssppActivity = nullptr; + jobject oldActivity; + { + std::lock_guard guard(g_activityLock); + oldActivity = ppssppActivity; + ppssppActivity = nullptr; + } + if (oldActivity) { + env->DeleteGlobalRef(oldActivity); + } } // This is now only used as a trigger for GetAppInfo as a function to all before Init. @@ -1476,14 +1507,16 @@ extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeApp_pushCameraImageAndroid( } } -// Call this under frameCommandLock. static void ProcessFrameCommands() { JNIEnv *env = getEnv(); + jobject activity = Android_GetActivity(env); std::vector frameCommands; { std::lock_guard guard(frameCommandLock); - if (!ppssppActivity) { - ERROR_LOG(Log::System, "No activity, clearing commands"); + if (!activity) { + if (!g_frameCommands.empty()) { + ERROR_LOG(Log::System, "No activity, clearing commands"); + } } else { frameCommands = std::move(g_frameCommands); } @@ -1491,17 +1524,20 @@ static void ProcessFrameCommands() { } if (!frameCommands.empty()) { - INFO_LOG(Log::System, "Processing %zu frame commands", g_frameCommands.size()); + INFO_LOG(Log::System, "Processing %zu frame commands", frameCommands.size()); for (const FrameCommand &frameCmd : frameCommands) { DEBUG_LOG(Log::System, "frameCommand '%s' '%s'", frameCmd.command.c_str(), frameCmd.params.c_str()); jstring cmd = env->NewStringUTF(frameCmd.command.c_str()); jstring param = env->NewStringUTF(frameCmd.params.c_str()); - env->CallVoidMethod(ppssppActivity, postCommand, cmd, param); + env->CallVoidMethod(activity, postCommand, cmd, param); env->DeleteLocalRef(cmd); env->DeleteLocalRef(param); } } + if (activity) { + env->DeleteLocalRef(activity); + } } std::thread g_renderLoopThread; diff --git a/android/jni/app-android.h b/android/jni/app-android.h index 087957d8f4..e1af557b13 100644 --- a/android/jni/app-android.h +++ b/android/jni/app-android.h @@ -22,6 +22,10 @@ std::vector Android_GetNativeCrashHistory(int maxEntries); jclass findClass(const char* name); JNIEnv* getEnv(); +// Returns a new local reference to the current activity, or null if there is none. The activity gets +// replaced whenever Android recreates it (rotation, resize), so don't hang on to anything longer-lived. +jobject Android_GetActivity(JNIEnv *env); + // Frees the JNI local references created in its scope. Java only does that for us when a native // method returns to it, which never happens on the threads we attach ourselves (EmuThread, IO and // worker threads), so there every local ref leaks until the thread detaches. Before Android 8 there's diff --git a/headless/Headless.cpp b/headless/Headless.cpp index bb96ff3c7b..a54269d8c0 100644 --- a/headless/Headless.cpp +++ b/headless/Headless.cpp @@ -112,6 +112,11 @@ jclass findClass(const char *name) { return nullptr; } +// Defined in app-android.cpp for the app itself, which this doesn't link. +jobject Android_GetActivity(JNIEnv *env) { + return nullptr; +} + bool System_AudioRecordingIsAvailable() { return false; } bool System_AudioRecordingState() { return false; } #endif diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 44411c379d..10b7747ad2 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -177,6 +177,11 @@ jclass findClass(const char *name) { return nullptr; } +// Defined in app-android.cpp for the app itself, which this doesn't link. +jobject Android_GetActivity(JNIEnv *env) { + return nullptr; +} + bool System_AudioRecordingIsAvailable() { return false; } bool System_AudioRecordingState() { return false; } #endif