From 35c586b27b0563764639d714a79a3bdc5b3df9f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 16:52:35 -0600 Subject: [PATCH 1/8] Android: Stop leaking JNI local references on native threads Local refs are only freed automatically when a native method returns to Java. The content URI functions mostly run on threads we attached ourselves, where nothing ever freed the strings they created - two per file open, one per exists check and so on. From Android 8 that's a slow leak; before it the table has 512 slots and overflowing it aborts. Add JNILocalFrame (PushLocalFrame/PopLocalFrame) and use it in all of AndroidStorage.cpp, in the camera device list, and free the name string in findClass. Also: Android_GetFileInfo never released its GetStringUTFChars buffer, a native leak on every call, and Android_ListContentUri called GetArrayLength on a null array if the Java side returned none. Co-Authored-By: Claude Fable 5.1 --- Common/File/AndroidStorage.cpp | 28 ++++++++++++++++++++++++---- android/jni/app-android.cpp | 8 +++++++- android/jni/app-android.h | 22 ++++++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/Common/File/AndroidStorage.cpp b/Common/File/AndroidStorage.cpp index d65129d91a..fdb8a21372 100644 --- a/Common/File/AndroidStorage.cpp +++ b/Common/File/AndroidStorage.cpp @@ -117,6 +117,7 @@ int Android_OpenContentUriFd(std::string_view filename, Android_OpenContentUriMo fname.pop_back(); auto env = getEnv(); + JNILocalFrame frame(env); const char *modeStr = ""; switch (mode) { case Android_OpenContentUriMode::READ: modeStr = "r"; break; @@ -134,6 +135,7 @@ StorageError Android_CreateDirectory(const std::string &rootTreeUri, const std:: return StorageError::UNKNOWN; } auto env = getEnv(); + JNILocalFrame frame(env); 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)); @@ -144,6 +146,7 @@ StorageError Android_CreateFile(const std::string &parentTreeUri, const std::str return StorageError::UNKNOWN; } auto env = getEnv(); + JNILocalFrame frame(env); 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)); @@ -154,6 +157,7 @@ StorageError Android_CopyFile(const std::string &fileUri, const std::string &des return StorageError::UNKNOWN; } auto env = getEnv(); + JNILocalFrame frame(env); 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)); @@ -164,6 +168,7 @@ StorageError Android_MoveFile(const std::string &fileUri, const std::string &src return StorageError::UNKNOWN; } auto env = getEnv(); + JNILocalFrame frame(env); jstring paramFileName = env->NewStringUTF(fileUri.c_str()); jstring paramSrcParentUri = env->NewStringUTF(srcParentUri.c_str()); jstring paramDestParentUri = env->NewStringUTF(destParentUri.c_str()); @@ -175,6 +180,7 @@ StorageError Android_RemoveFile(const std::string &fileUri) { return StorageError::UNKNOWN; } auto env = getEnv(); + JNILocalFrame frame(env); jstring paramFileName = env->NewStringUTF(fileUri.c_str()); return StorageErrorFromInt(env->CallStaticIntMethod(g_classContentUri, contentUriRemoveFile, g_nativeActivity, paramFileName)); } @@ -184,6 +190,7 @@ StorageError Android_RenameFileTo(const std::string &fileUri, const std::string return StorageError::UNKNOWN; } auto env = getEnv(); + JNILocalFrame frame(env); 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)); @@ -230,6 +237,7 @@ bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *fileInfo) { return false; } auto env = getEnv(); + JNILocalFrame frame(env); jstring paramFileUri = env->NewStringUTF(fileUri.c_str()); jstring str = (jstring)env->CallStaticObjectMethod(g_classContentUri, contentUriGetFileInfo, g_nativeActivity, paramFileUri); @@ -237,10 +245,12 @@ bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *fileInfo) { return false; } const char *charArray = env->GetStringUTFChars(str, 0); + if (!charArray) { + return false; + } bool retval = ParseFileInfo(charArray, fileInfo); + env->ReleaseStringUTFChars(str, charArray); fileInfo->fullName = Path(fileUri); - - env->DeleteLocalRef(str); return retval && fileInfo->exists; } @@ -249,6 +259,7 @@ bool Android_FileExists(const std::string &fileUri) { return false; } auto env = getEnv(); + JNILocalFrame frame(env); jstring paramFileUri = env->NewStringUTF(fileUri.c_str()); bool exists = env->CallStaticBooleanMethod(g_classContentUri, contentUriFileExists, g_nativeActivity, paramFileUri); return exists; @@ -260,6 +271,7 @@ std::vector Android_ListContentUri(const std::string &uri, const return {}; } auto env = getEnv(); + JNILocalFrame frame(env); *exists = true; double start = time_now_d(); @@ -270,6 +282,10 @@ std::vector Android_ListContentUri(const std::string &uri, const jobjectArray fileList = (jobjectArray)retval; std::vector items; + if (!fileList) { + *exists = false; + return items; + } int size = env->GetArrayLength(fileList); for (int i = 0; i < size; i++) { jstring str = (jstring)env->GetObjectArrayElement(fileList, i); @@ -286,11 +302,11 @@ std::vector Android_ListContentUri(const std::string &uri, const // INFO_LOG(Log::IO, "%s", info.name.c_str()); items.push_back(info); } + env->ReleaseStringUTFChars(str, charArray); } - env->ReleaseStringUTFChars(str, charArray); + // Don't leave these to the frame, there can be thousands. env->DeleteLocalRef(str); } - env->DeleteLocalRef(fileList); double elapsed = time_now_d() - start; double threshold = 0.1; @@ -305,6 +321,7 @@ int64_t Android_GetFreeSpaceByContentUri(const std::string &uri) { return false; } auto env = getEnv(); + JNILocalFrame frame(env); jstring param = env->NewStringUTF(uri.c_str()); return env->CallStaticLongMethod(g_classContentUri, contentUriGetFreeStorageSpace, g_nativeActivity, param); @@ -316,6 +333,7 @@ int64_t Android_GetFreeSpaceByFilePath(const std::string &filePath) { return false; } auto env = getEnv(); + JNILocalFrame frame(env); if (System_GetPropertyInt(SYSPROP_SYSTEMVERSION) < 26) { // This is available from Android O. @@ -331,6 +349,7 @@ int64_t Android_ComputeRecursiveDirectorySize(const std::string &uri) { return false; } auto env = getEnv(); + JNILocalFrame frame(env); jstring param = env->NewStringUTF(uri.c_str()); @@ -347,6 +366,7 @@ bool Android_IsExternalStoragePreservedLegacy() { return false; } auto env = getEnv(); + JNILocalFrame frame(env); // Note: No activity param return env->CallStaticBooleanMethod(g_classContentUri, isExternalStoragePreservedLegacy); } diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 492252290a..b4fe6fc4c1 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -198,8 +198,13 @@ JNIEnv* getEnv() { return env; } +// Returns a local reference. jclass findClass(const char* name) { - return static_cast(getEnv()->CallObjectMethod(gClassLoader, gFindClassMethod, getEnv()->NewStringUTF(name))); + JNIEnv *env = getEnv(); + jstring jname = env->NewStringUTF(name); + jclass cls = static_cast(env->CallObjectMethod(gClassLoader, gFindClassMethod, jname)); + env->DeleteLocalRef(jname); + return cls; } void Android_AttachThreadToJNI() { @@ -1421,6 +1426,7 @@ extern "C" jint JNICALL Java_org_ppsspp_ppsspp_NativeApp_getDisplayFramerateMode } std::vector System_GetCameraDeviceList() { + JNILocalFrame frame(getEnv()); jclass cameraClass = findClass("org/ppsspp/ppsspp/CameraHelper"); jmethodID deviceListMethod = getEnv()->GetStaticMethodID(cameraClass, "getDeviceList", "()Ljava/util/ArrayList;"); jobject deviceListObject = getEnv()->CallStaticObjectMethod(cameraClass, deviceListMethod); diff --git a/android/jni/app-android.h b/android/jni/app-android.h index 727e15a7bf..087957d8f4 100644 --- a/android/jni/app-android.h +++ b/android/jni/app-android.h @@ -22,6 +22,28 @@ std::vector Android_GetNativeCrashHistory(int maxEntries); jclass findClass(const char* name); JNIEnv* getEnv(); +// 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 +// only room for 512 of them, and running out aborts the process. +class JNILocalFrame { +public: + explicit JNILocalFrame(JNIEnv *env, int capacity = 16) : env_(env) { + pushed_ = env_->PushLocalFrame(capacity) == 0; + } + ~JNILocalFrame() { + if (pushed_) { + env_->PopLocalFrame(nullptr); + } + } + JNILocalFrame(const JNILocalFrame &) = delete; + JNILocalFrame &operator=(const JNILocalFrame &) = delete; + +private: + JNIEnv *env_; + bool pushed_; +}; + #endif #else From 2f6bb8d55ee3cfd49f5002a79300b79617688a3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 16:54:06 -0600 Subject: [PATCH 2/8] Android: Don't leak the activity on recreate, and hand it over safely registerCallbacks took a new global ref to the activity on every onCreate without releasing the previous one, and unregisterCallbacks only runs on a real shutdown. Since rotation and resizing recreate the activity, each of those leaked the old one, view hierarchy and all. Releasing it means other threads can no longer use the raw global ref, which had been copied into AndroidStorage and TextDrawerAndroid and was read from the emu thread and IO threads. They now take a local ref under a mutex (Android_GetActivity, and the equivalent inside AndroidStorage for the shortcut activity's sake), so a call in flight keeps its activity alive while the UI thread swaps in the new one. The "no activity" bail is a null check on that local ref rather than a separate HasActivity() test beforehand. Between such a test and the call the UI thread can swap the activity out, and we'd hand null to a Java method that wants a Context. The one remaining HasActivity() caller never passes the activity on, so it has nothing to race with. The ContentUri class ref and method IDs are now only looked up once, instead of leaking a class ref per activity. Headless and the unit test executable link Common but not app-android.cpp, so they get a stub for this the same way they already do for getEnv and findClass. Without it the Android CI build fails at link time, where a normal gradle build is perfectly happy. Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01152igCdjTNhT29qvphPyPC --- Common/File/AndroidStorage.cpp | 150 ++++++++++++++--------- Common/Render/Text/draw_text_android.cpp | 7 +- Common/Render/Text/draw_text_android.h | 2 - android/jni/app-android.cpp | 76 +++++++++--- android/jni/app-android.h | 4 + headless/Headless.cpp | 5 + unittest/UnitTest.cpp | 5 + 7 files changed, 166 insertions(+), 83 deletions(-) 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 From 8b829b30977f0a4e5ed214f6cd826e53479cee31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 16:54:46 -0600 Subject: [PATCH 3/8] Android: Share one TextRenderer class ref between TextDrawers Each TextDrawerAndroid took a global ref to the TextRenderer class and never released it (the release had been commented out as crashing), plus leaked the local ref it was made from. A TextDrawer is created every time graphics come up, so that was one of each per resume. Look the class up once and keep it. Also initialize the method IDs, which IsReady() read uninitialized if the class was missing, and don't walk into a null array if renderText throws. Co-Authored-By: Claude Fable 5.1 --- Common/Render/Text/draw_text_android.cpp | 34 ++++++++++++++++++------ Common/Render/Text/draw_text_android.h | 10 +++---- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/Common/Render/Text/draw_text_android.cpp b/Common/Render/Text/draw_text_android.cpp index 5a31d7dbe9..5cbb6f8d4e 100644 --- a/Common/Render/Text/draw_text_android.cpp +++ b/Common/Render/Text/draw_text_android.cpp @@ -16,18 +16,32 @@ #include +// A new TextDrawer is created every time graphics are brought up, which on Android is every resume. +// The class stays valid for the life of the process, so hold one global ref to it, forever. +static jclass GetTextRendererClass(JNIEnv *env) { + static jclass cls; + if (!cls) { + const char *textRendererClassName = "org/ppsspp/ppsspp/TextRenderer"; + jclass localClass = findClass(textRendererClassName); + if (localClass) { + cls = reinterpret_cast(env->NewGlobalRef(localClass)); + env->DeleteLocalRef(localClass); + } else { + env->ExceptionClear(); // ClassNotFoundException + ERROR_LOG(Log::G3D, "Failed to find class: '%s'", textRendererClassName); + } + } + return cls; +} + TextDrawerAndroid::TextDrawerAndroid(Draw::DrawContext *draw) : TextDrawer(draw) { auto env = getEnv(); - const char *textRendererClassName = "org/ppsspp/ppsspp/TextRenderer"; - jclass localClass = findClass(textRendererClassName); - cls_textRenderer = reinterpret_cast(env->NewGlobalRef(localClass)); + cls_textRenderer = GetTextRendererClass(env); if (cls_textRenderer) { method_allocFont = env->GetStaticMethodID(cls_textRenderer, "allocFont", "(Landroid/content/Context;Ljava/lang/String;)I"); method_freeAllFonts = env->GetStaticMethodID(cls_textRenderer, "freeAllFonts", "()V"); method_measureText = env->GetStaticMethodID(cls_textRenderer, "measureText", "(Ljava/lang/String;ID)I"); method_renderText = env->GetStaticMethodID(cls_textRenderer, "renderText", "(Ljava/lang/String;IDII)[I"); - } else { - ERROR_LOG(Log::G3D, "Failed to find class: '%s'", textRendererClassName); } dpiScale_ = CalculateDPIScale(); @@ -35,9 +49,7 @@ TextDrawerAndroid::TextDrawerAndroid(Draw::DrawContext *draw) : TextDrawer(draw) } TextDrawerAndroid::~TextDrawerAndroid() { - // Not sure why we can't do this but it crashes. Likely some deeper threading issue. - // At worst we leak one ref... - // env_->DeleteGlobalRef(cls_textRenderer); + // cls_textRenderer is shared and never released, see GetTextRendererClass. ClearCache(); fontMap_.clear(); // size is precomputed using dpiScale_. } @@ -129,6 +141,12 @@ bool TextDrawerAndroid::DrawStringBitmap(std::vector &bitmapData, TextS // TODO: Handle bold/italic jintArray imageData = (jintArray)env->CallStaticObjectMethod(cls_textRenderer, method_renderText, jstr, iter->second.font, iter->second.size, imageWidth, imageHeight); env->DeleteLocalRef(jstr); + if (!imageData) { + // Most likely an OutOfMemoryError allocating the bitmap. + env->ExceptionClear(); + ERROR_LOG(Log::G3D, "TextRenderer failed to render text"); + return false; + } entry.texture = nullptr; entry.bmWidth = imageWidth; diff --git a/Common/Render/Text/draw_text_android.h b/Common/Render/Text/draw_text_android.h index 75dc56b5f0..3262aa42ab 100644 --- a/Common/Render/Text/draw_text_android.h +++ b/Common/Render/Text/draw_text_android.h @@ -31,11 +31,11 @@ protected: private: // JNI functions - jclass cls_textRenderer; - jmethodID method_allocFont; - jmethodID method_freeAllFonts; - jmethodID method_measureText; - jmethodID method_renderText; + jclass cls_textRenderer = nullptr; + jmethodID method_allocFont = nullptr; + jmethodID method_freeAllFonts = nullptr; + jmethodID method_measureText = nullptr; + jmethodID method_renderText = nullptr; std::map fontMap_; std::map allocatedFonts_; From dc2c1650844506ef3489f1c5b38e742b9dc0a27b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 17:03:07 -0600 Subject: [PATCH 4/8] Android: Unregister the power saving settings observer PowerSaveModeReceiver registered a ContentObserver that captures the activity, and destroy() only unregistered the broadcast receiver. The system keeps registered observers alive, so every recreated activity stayed in memory. Found with a heap dump: 8 of 9 live activities after some rotating were held this way. Co-Authored-By: Claude Fable 5.1 --- android/src/org/ppsspp/ppsspp/PowerSaveModeReceiver.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/PowerSaveModeReceiver.java b/android/src/org/ppsspp/ppsspp/PowerSaveModeReceiver.java index 9b23cbfdf2..087e9c660c 100644 --- a/android/src/org/ppsspp/ppsspp/PowerSaveModeReceiver.java +++ b/android/src/org/ppsspp/ppsspp/PowerSaveModeReceiver.java @@ -14,6 +14,7 @@ import android.util.Log; public class PowerSaveModeReceiver extends BroadcastReceiver { private static final String TAG = PowerSaveModeReceiver.class.getSimpleName(); private static boolean isBatteryLow = false; + private final ContentObserver settingsObserver; @Override public void onReceive(final Context context, final Intent intent) { @@ -36,7 +37,7 @@ public class PowerSaveModeReceiver extends BroadcastReceiver { filter.addAction(PowerManager.ACTION_POWER_SAVE_MODE_CHANGED); activity.registerReceiver(this, filter); - activity.getContentResolver().registerContentObserver(Settings.System.CONTENT_URI, true, new ContentObserver(null) { + settingsObserver = new ContentObserver(null) { @Override public void onChange(boolean selfChange, Uri uri) { super.onChange(selfChange, uri); @@ -50,12 +51,15 @@ public class PowerSaveModeReceiver extends BroadcastReceiver { sendPowerSaving(activity); } } - }); + }; + activity.getContentResolver().registerContentObserver(Settings.System.CONTENT_URI, true, settingsObserver); sendPowerSaving(activity); } public void destroy(final Context context) { context.unregisterReceiver(this); + // The observer holds on to the activity, and the system holds on to a registered observer. + context.getContentResolver().unregisterContentObserver(settingsObserver); } private static boolean getNativePowerSaving(final Context context) { From 6a712a927a732706030dcfe988ffbdf263ed0edf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 17:03:07 -0600 Subject: [PATCH 5/8] Android: Don't create the static location and IR helpers from the activity They're static and created once, and the LocationManager they look up keeps the context it came from, which pinned the first activity for the life of the process. Use the application context. Co-Authored-By: Claude Fable 5.1 --- android/src/org/ppsspp/ppsspp/PpssppActivity.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index 53d069369a..3373cf7cd4 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -478,12 +478,14 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList } } + // These are static and outlive the activity, and the system services they look up hang on to + // the context they came from. So don't give them the activity. if (mLocationHelper == null) { - mLocationHelper = new LocationHelper(this); + mLocationHelper = new LocationHelper(getApplicationContext()); } try { if (mInfraredHelper == null) { - mInfraredHelper = new InfraredHelper(this); + mInfraredHelper = new InfraredHelper(getApplicationContext()); } } catch (Exception e) { mInfraredHelper = null; From 9cae1a6653768085182ed67c75892d27aafd23f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 16:55:55 -0600 Subject: [PATCH 6/8] Android: Don't let a late onDestroy shut down a newer activity's native state After finish(), onDestroy may arrive after a new activity has already been created and resumed, if the user relaunches quickly. The new one sees initialized still set and adopts the native state. The old one's onDestroy then shut down audio and called NativeApp.shutdown(), deleting the graphics context the new render thread was using. Number the activities, and leave the native side alone in onDestroy if a newer one exists. Co-Authored-By: Claude Fable 5.1 --- .../src/org/ppsspp/ppsspp/PpssppActivity.java | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index 3373cf7cd4..272ebf20d0 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -78,6 +78,11 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList // Allows us to skip a lot of initialization on secondary calls to onCreate. private static boolean initialized = false; + // Bumped by every onCreate. An activity whose number is no longer the latest has been replaced + // by a newer one, which now owns the native state. + private static int latestInstance = 0; + private int instance; + // Lifecycle tracker, to detect erroneous states. private final LifeCycle lifeCycle = new LifeCycle(); @@ -652,6 +657,8 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList return; } + instance = ++latestInstance; + WindowCompat.setDecorFitsSystemWindows(getWindow(), false); // In case app launched from homescreen shortcut, get shortcut parameter @@ -928,16 +935,25 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList mPowerSaveModeReceiver = null; } - // TODO: Can we ensure that the GL thread has stopped rendering here? - // I've seen crashes that seem to indicate that sometimes it hasn't... - NativeApp.audioShutdown(); - if (shuttingDown) { - Log.i(TAG, "in onDestroy, shutting down. Calling NativeApp.shutdown()."); - NativeApp.shutdown(); - unregisterCallbacks(); - initialized = false; + if (instance != latestInstance) { + // After finish(), Android is free to deliver onDestroy late, after it has already created + // and resumed a new activity if the user was quick to relaunch. That one found initialized + // still set and adopted the native state, so shutting down here would pull the audio and + // the graphics context out from under it. It's not ours to shut down anymore. + // (recreate() is not affected, it destroys the old activity before creating the new one.) + Log.w(TAG, "in onDestroy, but a newer activity has taken over. Leaving the native side alone."); } else { - Log.i(TAG, "in onDestroy, but not shutting down."); + // TODO: Can we ensure that the GL thread has stopped rendering here? + // I've seen crashes that seem to indicate that sometimes it hasn't... + NativeApp.audioShutdown(); + if (shuttingDown) { + Log.i(TAG, "in onDestroy, shutting down. Calling NativeApp.shutdown()."); + NativeApp.shutdown(); + unregisterCallbacks(); + initialized = false; + } else { + Log.i(TAG, "in onDestroy, but not shutting down."); + } } navigationCallbackView = null; From 68ae1470338a32959b7538f81688c6f210d424fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 21 Sep 2026 17:18:36 -0600 Subject: [PATCH 7/8] Android: A replaced activity must not start or stop the render thread There is only one render thread, shared by whichever activity is current. After a finish() and a quick relaunch, the old activity's surface is destroyed about a second into the new activity's life, and joining the thread from surfaceDestroyed then stopped the new activity's rendering. Found by forcing the finish-then-relaunch order with a test hook: without the previous commit the process died in NativeApp.shutdown(), with it the app survived but went black until this was added. Co-Authored-By: Claude Fable 5.1 --- android/src/org/ppsspp/ppsspp/PpssppActivity.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index 272ebf20d0..44299c0588 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -891,6 +891,10 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList Log.w(TAG, "startRenderLoopThread - not starting thread, needs surface"); return; } + if (instance != latestInstance) { + Log.w(TAG, "startRenderLoopThread - not starting thread, a newer activity has taken over"); + return; + } Log.w(TAG, "startRenderLoopThread: Starting thread"); @@ -899,6 +903,13 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList } private synchronized void joinRenderLoopThread() { + if (instance != latestInstance) { + // There's only one render thread, and it's rendering to the newer activity's surface now. + // We get here when our surface goes away late, after a finish() and a quick relaunch. + Log.w(TAG, "joinRenderLoopThread - leaving the thread alone, a newer activity has taken over"); + return; + } + // This will wait until the thread has exited. Log.i(TAG, "requestExitRenderLoop"); requestExitRenderLoop(); From 0c7990783d5d0c34fda875778eed77234770adf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 8 Oct 2026 18:16:21 +0200 Subject: [PATCH 8/8] Docs: How to test an Android build on a real device Written while testing the activity lifecycle work on a Galaxy S8 and a Quest 2. Mostly about ways the tooling answers a question you didn't ask. The expensive one: versionCode is the git commit count, so installing a feature branch over a longer branch fails with INSTALL_FAILED_VERSION_DOWNGRADE, which -r does not override. The device keeps running the previous build, and if the install output went through a pipe the failure is never seen. An A/B where both installs failed that way agrees with itself perfectly and means nothing. Also: dumpsys meminfo counts activities that are garbage but not yet collected, so the obvious reading looks like a leak whether or not there is one - trim and re-read until it settles; how to force an activity recreate over adb when the device won't rotate on command; why screencap is black on a Quest and what to read instead; and how to tell whether a device exercises the content-URI path at all before concluding a run covered it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01152igCdjTNhT29qvphPyPC --- AGENTS.md | 8 +++ docs/android-device-testing.md | 108 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 docs/android-device-testing.md diff --git a/AGENTS.md b/AGENTS.md index 7999d7af0d..4ea446a975 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,7 @@ for it: | Doc | When you need it | |---|---| | [docs/building.md](docs/building.md) | Build commands for every target (VS/MSBuild, CMake, UWP, legacy Android NDK, libretro), unit tests, pspautotests | +| [docs/android-device-testing.md](docs/android-device-testing.md) | Running a build on a real Android device or a Quest over adb: installing the build you think you are, forcing activity recreates, measuring retained activities, VR | | [docs/debugging.md](docs/debugging.md) | Driving the WebSocket debugger and PPSSPPHeadless from a script, measuring a commercial game with headless, comparing binaries' speed (`Tools/headless_bench.py`), breakpoint reliability per CPU backend, debugging a game that works on hardware | | [docs/DebuggerThreading.md](docs/DebuggerThreading.md) | `Core_RunOnCPUThread` / `g_frameMutex` / shutdown-lock rules - required reading before touching debugger code | | [docs/HLEModules.md](docs/HLEModules.md) | Adding an HLE module or function, and the seven build files a new source file goes in | @@ -175,6 +176,13 @@ took minutes with a Debug build, software rendering and a 300MB log, and about a check that a game renders, `--screenshot-save=FILE.png` beats grepping the log. (pspautotests through `test.py` are different: they have their own per-test timeouts and use `--graphics=software` like CI.) +Testing on a real Android device has its own traps, the worst of which is silent: `adb install -r` +of a shorter branch over a longer one fails with `INSTALL_FAILED_VERSION_DOWNGRADE`, because +`versionCode` is the commit count. The device keeps running the old build and every measurement after +that describes the wrong binary. Use `adb install -r -d`, don't swallow the install output, and check +`adb shell dumpsys package org.ppsspp.ppsspp | grep versionName` afterwards. See +[docs/android-device-testing.md](docs/android-device-testing.md). + New unit tests are added to `availableTests`; large ones go in their own file in `unittest/`, which has to be listed in **three** build files, not two: `CMakeLists.txt`, `unittest/UnitTests.vcxproj` (and its `.filters`), and `android/jni/Android.mk`, which builds a unit test executable of its own. Miss the last diff --git a/docs/android-device-testing.md b/docs/android-device-testing.md new file mode 100644 index 0000000000..73862110cf --- /dev/null +++ b/docs/android-device-testing.md @@ -0,0 +1,108 @@ +# Testing an Android build on a device + +Notes from driving real devices over adb. Most of what follows is not about Android being hard, but +about ways the tooling quietly answers a question you did not ask - you measure the wrong build, or +measure garbage that has not been collected yet, and get a confident wrong answer. + +## Install the build you think you are testing + +`versionCode` is derived from the git commit count (see `gitVersionCode` in +`android/build.gradle.kts`), so a feature branch has a *lower* code than a longer branch. Installing +one over the other fails: + + adb: failed to install ...: Failure [INSTALL_FAILED_VERSION_DOWNGRADE] + +`-r` does not permit downgrades; `-d` does. Use `adb install -r -d `. + +Do not pipe the install through `tail -1` or `grep Success` and move on. The failure is a single +line and easy to swallow, and the device then keeps running whatever was there before - every +measurement afterwards describes the wrong binary, including A/B comparisons where both arms end up +being the same build and therefore agree with each other. + +Verify after every install, not just when something looks odd: + + adb shell dumpsys package org.ppsspp.ppsspp | grep versionName + +A second, independent check is worth it when a result is surprising: grep the log for a symbol that +exists only on the branch under test (a renamed class or log tag, say). + +The version string itself can also lie. The Android build regenerates `git-version.cpp` but does not +always recompile it, so the APK can carry the previous build's string while containing current code. +When the version matters: + + find android/.cxx -name git-version.cpp.o -delete + +## Forcing an activity recreate without touching the device + +- `adb shell settings put system font_scale 1.15`, then back. `fontScale` is not in the activity's + `configChanges`, so each change destroys and recreates the activity. Independent of orientation. +- `adb shell wm density ` also recreates - but only while `density` is absent from + `configChanges`. +- `adb shell settings put system user_rotation 1` does **not** rotate PPSSPP. The activity follows + the sensor, so this needs a physical rotation (or a cooperative device). + +Put back whatever you changed: `font_scale`, `accelerometer_rotation`, `user_rotation`. + +`wm density reset` is actively wrong on a device with a resolution override - a Galaxy S8 running +1080x2220 on a 1440x2960 panel resets to the *physical* panel's density and leaves the UI oversized. +Read the original with `wm density` first and set it back explicitly. + +## Measuring retained activities + +`adb shell dumpsys meminfo ` prints an Objects block with `Activities` and `AppContexts`. + +The counts include objects that are garbage but not yet collected. Read it straight after N +recreates and you will see roughly N whether or not anything leaks, which looks exactly like a leak. +Force collection instead: + + adb shell am send-trim-memory org.ppsspp.ppsspp COMPLETE + +then wait around eight seconds and read. Repeat until two readings agree - three or four rounds is +normal, and a single trim is not enough. + +The protocol that gives an answer you can trust: cold start and read the baseline, do N recreates, +trim until stable, compare. A sound build returns to the baseline; a leaking one sits at baseline+N +and stays there. + +Worked example, Galaxy S8 on Android 9, same device and protocol, both builds confirmed by +`versionName`. Before the activity handover fix: 13 recreates left 13 Activities and 18 AppContexts, +stable across repeated trims. After: the same 13 recreates settled back to 1 and 6. + +A JNI global reference is a GC root, so an activity pinned by one shows up here. This is a usable +check for JNI ownership bugs, not only Java ones. + +## logcat on a noisy device + +Filter to the process, since the tag alone is not enough: + + adb logcat -d --pid=$(adb shell pidof org.ppsspp.ppsspp) + +On a device with a chatty system - a Quest especially - the ring buffer can rotate the app's lines +away within seconds, which reads as "logging stopped" rather than "logging was lost". Start a stream +*before* launching the app instead: + + adb logcat -c && adb logcat -s PPSSPP:V PpssppActivity:V # in the background, then start the app + +## VR (Quest) + +- The VR flavor is a separate package: `./gradlew :android:assembleVrDebug`, installing as + `org.ppsspp.ppssppvr`. It can sit alongside the normal build. +- `adb exec-out screencap` returns solid black for an immersive app - it captures the 2D panel + layer, which such an app does not have. Use the runtime's own line instead: a logcat entry like + `VrApi: FPS=73/72 ... App=2.67ms` proves frames are being submitted and how much GPU time the app + spent on them. Zero app time means the render loop is not producing. +- `am start` makes the activity `topResumedActivity`, but the shell keeps focus, so the app never + gets a surface and the render loop does not restart. Re-entering an immersive app has to be done + from inside the headset. A fresh `am start -S` after a `force-stop` does work. +- `debuggerd -b ` needs root, so no native stacks from a retail headset. + +## Check which paths the device actually exercises + +Before concluding that a test covered something: + + adb shell grep -E '^(MemStickDirectory|GraphicsBackend)' /storage/emulated/0/PSP/SYSTEM/ppsspp.ini + +A plain filesystem path for the memstick means the SAF/content-URI code +(`Common/File/AndroidStorage.cpp`, `ContentUri.java`) is never entered, so a run on that device says +nothing about it - a recents list full of plain paths says the same. The VR flavor reads +`ppssppvr.ini`, not `ppsspp.ini`.