Merge pull request #22454 from hrydgard/android-activity-lifecycle

More Android activity lifecycle fixes

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

Common/File/AndroidStorage.cpp

@@ -1,4 +1,5 @@
 #include <inttypes.h>
+#include <mutex>
 
 #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<std::mutex> 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<std::mutex> 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<std::mutex> 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")) {
@@ -117,6 +135,13 @@ int Android_OpenContentUriFd(std::string_view filename, Android_OpenContentUriMo
 		fname.pop_back();
 
 	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;
@@ -125,68 +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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return StorageError::UNKNOWN;
 	}
-	auto env = getEnv();
 	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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return StorageError::UNKNOWN;
 	}
-	auto env = getEnv();
 	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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return StorageError::UNKNOWN;
 	}
-	auto env = getEnv();
 	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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return StorageError::UNKNOWN;
 	}
-	auto env = getEnv();
 	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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return StorageError::UNKNOWN;
 	}
-	auto env = getEnv();
 	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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return StorageError::UNKNOWN;
 	}
-	auto env = getEnv();
 	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.
@@ -226,50 +263,62 @@ static bool ParseFileInfo(std::string_view line, File::FileInfo *fileInfo) {
 }
 
 bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *fileInfo) {
-	if (!g_nativeActivity) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return false;
 	}
-	auto env = getEnv();
 	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;
 	}
 	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;
 }
 
 bool Android_FileExists(const std::string &fileUri) {
-	if (!g_nativeActivity) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return false;
 	}
-	auto env = getEnv();
 	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<File::FileInfo> 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();
 	*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<File::FileInfo> 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 +335,11 @@ std::vector<File::FileInfo> 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;
@@ -301,21 +350,25 @@ std::vector<File::FileInfo> Android_ListContentUri(const std::string &uri, const
 }
 
 int64_t Android_GetFreeSpaceByContentUri(const std::string &uri) {
-	if (!g_nativeActivity) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return false;
 	}
-	auto env = getEnv();
 
 	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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return false;
 	}
-	auto env = getEnv();
 
 	if (System_GetPropertyInt(SYSPROP_SYSTEMVERSION) < 26) {
 		// This is available from Android O.
@@ -323,19 +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) {
+	auto env = getEnv();
+	JNILocalFrame frame(env);
+	jobject activity = AcquireActivity(env);
+	if (!activity) {
 		return false;
 	}
-	auto env = getEnv();
 
 	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);
@@ -343,10 +398,11 @@ int64_t Android_ComputeRecursiveDirectorySize(const std::string &uri) {
 }
 
 bool Android_IsExternalStoragePreservedLegacy() {
-	if (!g_nativeActivity) {
+	if (!HasActivity()) {
 		return false;
 	}
 	auto env = getEnv();
+	JNILocalFrame frame(env);
 	// Note: No activity param
 	return env->CallStaticBooleanMethod(g_classContentUri, isExternalStoragePreservedLegacy);
 }

Common/Render/Text/draw_text_android.cpp

@@ -16,20 +16,32 @@
 
 #include <jni.h>
 
-jobject TextDrawerAndroid::activity_;
+// 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<jclass>(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<jclass>(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();
 
@@ -37,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_.
 }
@@ -61,9 +71,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;
@@ -130,6 +141,12 @@ bool TextDrawerAndroid::DrawStringBitmap(std::vector<uint8_t> &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;

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<uint8_t> &bitmapData, TextStringEntry &entry, Draw::DataFormat texFormat, std::string_view str, int align, bool fullColor) override;
 
 protected:
@@ -32,12 +31,11 @@ protected:
 
 private:
 	// JNI functions
-	static jobject activity_;
-	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<FontStyle, AndroidFontEntry> fontMap_;
 	std::map<std::string, int> allocatedFonts_;

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<bool> exitRenderLoop;
 static std::atomic<bool> renderLoopRunning;
@@ -198,8 +200,13 @@ JNIEnv* getEnv() {
 	return env;
 }
 
+// Returns a local reference.
 jclass findClass(const char* name) {
-	return static_cast<jclass>(getEnv()->CallObjectMethod(gClassLoader, gFindClassMethod, getEnv()->NewStringUTF(name)));
+	JNIEnv *env = getEnv();
+	jstring jname = env->NewStringUTF(name);
+	jclass cls = static_cast<jclass>(env->CallObjectMethod(gClassLoader, gFindClassMethod, jname));
+	env->DeleteLocalRef(jname);
+	return cls;
 }
 
 void Android_AttachThreadToJNI() {
@@ -249,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<std::mutex> guard(g_activityLock);
+	return ppssppActivity ? env->NewLocalRef(ppssppActivity) : nullptr;
+}
+
 static void PushCommand(std::string_view cmd, std::string_view param) {
 	std::lock_guard<std::mutex> guard(frameCommandLock);
 	g_frameCommands.emplace_back(std::string(cmd), std::string(param));
@@ -450,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<std::string> Android_GetNativeCrashHistory(int maxEntries) {
 	std::vector<std::string> 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;
 	}
@@ -508,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<std::mutex> 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;");
@@ -519,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<std::mutex> 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.
@@ -1421,6 +1457,7 @@ extern "C" jint JNICALL Java_org_ppsspp_ppsspp_NativeApp_getDisplayFramerateMode
 }
 
 std::vector<std::string> 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);
@@ -1470,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<FrameCommand> frameCommands;
 	{
 		std::lock_guard<std::mutex> 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);
 		}
@@ -1485,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;

android/jni/app-android.h

@@ -22,6 +22,32 @@ std::vector<std::string> 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
+// 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

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) {

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();
 
@@ -478,12 +483,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;
@@ -650,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
@@ -882,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");
 
@@ -890,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();
@@ -926,16 +946,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;
 

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 <apk>`.
+
+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 <n>` 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 <pkg>` 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 <tid>` 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`.

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

unittest/UnitTest.cpp

@@ -178,6 +178,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