Merge pull request #94661 from m4gr3d/fix_android_render_thread_cleanup

Fix the cleanup logic for the Android render thread

platform/android/java/editor/src/main/java/org/godotengine/editor/GodotEditor.kt

@@ -203,7 +203,14 @@ open class GodotEditor : GodotActivity() {
 		}
 		if (editorWindowInfo.windowClassName == javaClass.name) {
 			Log.d(TAG, "Restarting ${editorWindowInfo.windowClassName} with parameters ${args.contentToString()}")
-			ProcessPhoenix.triggerRebirth(this, newInstance)
+			val godot = godot
+			if (godot != null) {
+				godot.destroyAndKillProcess {
+					ProcessPhoenix.triggerRebirth(this, newInstance)
+				}
+			} else {
+				ProcessPhoenix.triggerRebirth(this, newInstance)
+			}
 		} else {
 			Log.d(TAG, "Starting ${editorWindowInfo.windowClassName} with parameters ${args.contentToString()}")
 			newInstance.putExtra(EXTRA_NEW_LAUNCH, true)

platform/android/java/lib/src/org/godotengine/godot/Godot.kt

@@ -73,6 +73,7 @@ import java.io.InputStream
 import java.lang.Exception
 import java.security.MessageDigest
 import java.util.*
+import java.util.concurrent.atomic.AtomicReference
 
 /**
  * Core component used to interface with the native layer of the engine.
@@ -127,6 +128,11 @@ class Godot(private val context: Context) : SensorEventListener {
 	val netUtils = GodotNetUtils(context)
 	private val commandLineFileParser = CommandLineFileParser()
 
+	/**
+	 * Task to run when the engine terminates.
+	 */
+	private val runOnTerminate = AtomicReference<Runnable>()
+
 	/**
 	 * Tracks whether [onCreate] was completed successfully.
 	 */
@@ -577,10 +583,7 @@ class Godot(private val context: Context) : SensorEventListener {
 			plugin.onMainDestroy()
 		}
 
-		runOnRenderThread {
-			GodotLib.ondestroy()
-			forceQuit()
-		}
+		renderView?.onActivityDestroyed()
 	}
 
 	/**
@@ -663,6 +666,15 @@ class Godot(private val context: Context) : SensorEventListener {
 		primaryHost?.onGodotMainLoopStarted()
 	}
 
+	/**
+	 * Invoked on the render thread when the engine is about to terminate.
+	 */
+	@Keep
+	private fun onGodotTerminating() {
+		Log.v(TAG, "OnGodotTerminating")
+		runOnTerminate.get()?.run()
+	}
+
 	private fun restart() {
 		primaryHost?.onGodotRestartRequested(this)
 	}
@@ -798,8 +810,28 @@ class Godot(private val context: Context) : SensorEventListener {
 		mClipboard.setPrimaryClip(clip)
 	}
 
-	fun forceQuit() {
-		forceQuit(0)
+	/**
+	 * Destroys the Godot Engine and kill the process it's running in.
+	 */
+	@JvmOverloads
+	fun destroyAndKillProcess(destroyRunnable: Runnable? = null) {
+		val host = primaryHost
+		val activity = host?.activity
+		if (host == null || activity == null) {
+			// Run the destroyRunnable right away as we are about to force quit.
+			destroyRunnable?.run()
+
+			// Fallback to force quit
+			forceQuit(0)
+			return
+		}
+
+		// Store the destroyRunnable so it can be run when the engine is terminating
+		runOnTerminate.set(destroyRunnable)
+
+		runOnUiThread {
+			onDestroy(host)
+		}
 	}
 
 	@Keep
@@ -814,11 +846,7 @@ class Godot(private val context: Context) : SensorEventListener {
 		} ?: return false
 	}
 
-	fun onBackPressed(host: GodotHost) {
-		if (host != primaryHost) {
-			return
-		}
-
+	fun onBackPressed() {
 		var shouldQuit = true
 		for (plugin in pluginRegistry.allPlugins) {
 			if (plugin.onMainBackPressed()) {

platform/android/java/lib/src/org/godotengine/godot/GodotActivity.kt

@@ -85,12 +85,8 @@ abstract class GodotActivity : FragmentActivity(), GodotHost {
 	protected open fun getGodotAppLayout() = R.layout.godot_app_layout
 
 	override fun onDestroy() {
-		Log.v(TAG, "Destroying Godot app...")
+		Log.v(TAG, "Destroying GodotActivity $this...")
 		super.onDestroy()
-
-		godotFragment?.let {
-			terminateGodotInstance(it.godot)
-		}
 	}
 
 	override fun onGodotForceQuit(instance: Godot) {

platform/android/java/lib/src/org/godotengine/godot/GodotFragment.java

@@ -187,7 +187,12 @@ public class GodotFragment extends Fragment implements IDownloaderClient, GodotH
 		final Activity activity = getActivity();
 		mCurrentIntent = activity.getIntent();
 
-		godot = new Godot(requireContext());
+		if (parentHost != null) {
+			godot = parentHost.getGodot();
+		}
+		if (godot == null) {
+			godot = new Godot(requireContext());
+		}
 		performEngineInitialization();
 		BenchmarkUtils.endBenchmarkMeasure("Startup", "GodotFragment::onCreate");
 	}
@@ -209,7 +214,7 @@ public class GodotFragment extends Fragment implements IDownloaderClient, GodotH
 			final String errorMessage = TextUtils.isEmpty(e.getMessage())
 					? getString(R.string.error_engine_setup_message)
 					: e.getMessage();
-			godot.alert(errorMessage, getString(R.string.text_error_title), godot::forceQuit);
+			godot.alert(errorMessage, getString(R.string.text_error_title), godot::destroyAndKillProcess);
 		} catch (IllegalArgumentException ignored) {
 			final Activity activity = getActivity();
 			Intent notifierIntent = new Intent(activity, activity.getClass());
@@ -325,7 +330,7 @@ public class GodotFragment extends Fragment implements IDownloaderClient, GodotH
 	}
 
 	public void onBackPressed() {
-		godot.onBackPressed(this);
+		godot.onBackPressed();
 	}
 
 	/**

platform/android/java/lib/src/org/godotengine/godot/GodotGLRenderView.java

@@ -42,7 +42,6 @@ import org.godotengine.godot.xr.regular.RegularContextFactory;
 import org.godotengine.godot.xr.regular.RegularFallbackConfigChooser;
 
 import android.annotation.SuppressLint;
-import android.content.Context;
 import android.content.res.AssetManager;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
@@ -77,7 +76,7 @@ import java.io.InputStream;
  *   that matches it exactly (with regards to red/green/blue/alpha channels
  *   bit depths). Failure to do so would result in an EGL_BAD_MATCH error.
  */
-public class GodotGLRenderView extends GLSurfaceView implements GodotRenderView {
+class GodotGLRenderView extends GLSurfaceView implements GodotRenderView {
 	private final GodotHost host;
 	private final Godot godot;
 	private final GodotInputHandler inputHandler;
@@ -140,9 +139,14 @@ public class GodotGLRenderView extends GLSurfaceView implements GodotRenderView
 		resumeGLThread();
 	}
 
+	@Override
+	public void onActivityDestroyed() {
+		requestRenderThreadExitAndWait();
+	}
+
 	@Override
 	public void onBackPressed() {
-		godot.onBackPressed(host);
+		godot.onBackPressed();
 	}
 
 	@Override

platform/android/java/lib/src/org/godotengine/godot/GodotRenderView.java

@@ -44,6 +44,9 @@ public interface GodotRenderView {
 	 */
 	void startRenderer();
 
+	/**
+	 * Queues a runnable to be run on the rendering thread.
+	 */
 	void queueOnRenderThread(Runnable event);
 
 	void onActivityPaused();
@@ -54,6 +57,8 @@ public interface GodotRenderView {
 
 	void onActivityStarted();
 
+	void onActivityDestroyed();
+
 	void onBackPressed();
 
 	GodotInputHandler getInputHandler();

platform/android/java/lib/src/org/godotengine/godot/GodotVulkanRenderView.java

@@ -50,7 +50,7 @@ import androidx.annotation.Keep;
 
 import java.io.InputStream;
 
-public class GodotVulkanRenderView extends VkSurfaceView implements GodotRenderView {
+class GodotVulkanRenderView extends VkSurfaceView implements GodotRenderView {
 	private final GodotHost host;
 	private final Godot godot;
 	private final GodotInputHandler mInputHandler;
@@ -118,9 +118,14 @@ public class GodotVulkanRenderView extends VkSurfaceView implements GodotRenderV
 		});
 	}
 
+	@Override
+	public void onActivityDestroyed() {
+		requestRenderThreadExitAndWait();
+	}
+
 	@Override
 	public void onBackPressed() {
-		godot.onBackPressed(host);
+		godot.onBackPressed();
 	}
 
 	@Override

platform/android/java/lib/src/org/godotengine/godot/gl/GLSurfaceView.java

@@ -595,6 +595,15 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
 	protected final void resumeGLThread() {
 		mGLThread.onResume();
 	}
+
+	/**
+	 * Requests the render thread to exit and block until it does.
+	 */
+	protected final void requestRenderThreadExitAndWait() {
+		if (mGLThread != null) {
+			mGLThread.requestExitAndWait();
+		}
+	}
 	// -- GODOT end --
 
 	/**
@@ -783,6 +792,11 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
 		 * @return true if the buffers should be swapped, false otherwise.
 		 */
 		boolean onDrawFrame(GL10 gl);
+
+		/**
+		 * Invoked when the render thread is in the process of shutting down.
+		 */
+		void onRenderThreadExiting();
 		// -- GODOT end --
 	}
 
@@ -1621,6 +1635,12 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
 				 * clean-up everything...
 				 */
 				synchronized (sGLThreadManager) {
+					Log.d("GLThread", "Exiting render thread");
+					GLSurfaceView view = mGLSurfaceViewWeakRef.get();
+					if (view != null) {
+						view.mRenderer.onRenderThreadExiting();
+					}
+
 					stopEglSurfaceLocked();
 					stopEglContextLocked();
 				}

platform/android/java/lib/src/org/godotengine/godot/gl/GodotRenderer.java

@@ -34,6 +34,8 @@ import org.godotengine.godot.GodotLib;
 import org.godotengine.godot.plugin.GodotPlugin;
 import org.godotengine.godot.plugin.GodotPluginRegistry;
 
+import android.util.Log;
+
 import javax.microedition.khronos.egl.EGLConfig;
 import javax.microedition.khronos.opengles.GL10;
 
@@ -41,6 +43,8 @@ import javax.microedition.khronos.opengles.GL10;
  * Godot's GL renderer implementation.
  */
 public class GodotRenderer implements GLSurfaceView.Renderer {
+	private final String TAG = GodotRenderer.class.getSimpleName();
+
 	private final GodotPluginRegistry pluginRegistry;
 	private boolean activityJustResumed = false;
 
@@ -62,6 +66,12 @@ public class GodotRenderer implements GLSurfaceView.Renderer {
 		return swapBuffers;
 	}
 
+	@Override
+	public void onRenderThreadExiting() {
+		Log.d(TAG, "Destroying Godot Engine");
+		GodotLib.ondestroy();
+	}
+
 	public void onSurfaceChanged(GL10 gl, int width, int height) {
 		GodotLib.resize(null, width, height);
 		for (GodotPlugin plugin : pluginRegistry.getAllPlugins()) {

platform/android/java/lib/src/org/godotengine/godot/vulkan/VkRenderer.kt

@@ -31,11 +31,9 @@
 @file:JvmName("VkRenderer")
 package org.godotengine.godot.vulkan
 
+import android.util.Log
 import android.view.Surface
-
-import org.godotengine.godot.Godot
 import org.godotengine.godot.GodotLib
-import org.godotengine.godot.plugin.GodotPlugin
 import org.godotengine.godot.plugin.GodotPluginRegistry
 
 /**
@@ -52,6 +50,11 @@ import org.godotengine.godot.plugin.GodotPluginRegistry
  * @see [VkSurfaceView.startRenderer]
  */
 internal class VkRenderer {
+
+	companion object {
+		private val TAG = VkRenderer::class.java.simpleName
+	}
+
 	private val pluginRegistry: GodotPluginRegistry = GodotPluginRegistry.getPluginRegistry()
 
 	/**
@@ -101,8 +104,10 @@ internal class VkRenderer {
 	}
 
 	/**
-	 * Called when the rendering thread is destroyed and used as signal to tear down the Vulkan logic.
+	 * Invoked when the render thread is in the process of shutting down.
 	 */
-	fun onVkDestroy() {
+	fun onRenderThreadExiting() {
+		Log.d(TAG, "Destroying Godot Engine")
+		GodotLib.ondestroy()
 	}
 }

platform/android/java/lib/src/org/godotengine/godot/vulkan/VkSurfaceView.kt

@@ -113,12 +113,10 @@ open internal class VkSurfaceView(context: Context) : SurfaceView(context), Surf
 	}
 
 	/**
-	 * Tear down the rendering thread.
-	 *
-	 * Must not be called before a [VkRenderer] has been set.
+	 * Requests the render thread to exit and block until it does.
 	 */
-	fun onDestroy() {
-		vkThread.blockingExit()
+	fun requestRenderThreadExitAndWait() {
+		vkThread.requestExitAndWait()
 	}
 
 	override fun surfaceChanged(holder: SurfaceHolder, format: Int, width: Int, height: Int) {

platform/android/java/lib/src/org/godotengine/godot/vulkan/VkThread.kt

@@ -75,6 +75,9 @@ internal class VkThread(private val vkSurfaceView: VkSurfaceView, private val vk
 
 	private fun threadExiting() {
 		lock.withLock {
+			Log.d(TAG, "Exiting render thread")
+			vkRenderer.onRenderThreadExiting()
+
 			exited = true
 			lockCondition.signalAll()
 		}
@@ -93,7 +96,7 @@ internal class VkThread(private val vkSurfaceView: VkSurfaceView, private val vk
 	/**
 	 * Request the thread to exit and block until it's done.
 	 */
-	fun blockingExit() {
+	fun requestExitAndWait() {
 		lock.withLock {
 			shouldExit = true
 			lockCondition.signalAll()
@@ -171,7 +174,6 @@ internal class VkThread(private val vkSurfaceView: VkSurfaceView, private val vk
 					while (true) {
 						// Code path for exiting the thread loop.
 						if (shouldExit) {
-							vkRenderer.onVkDestroy()
 							return
 						}
 

platform/android/java_godot_lib_jni.cpp

@@ -114,6 +114,7 @@ static void _terminate(JNIEnv *env, bool p_restart = false) {
 	NetSocketAndroid::terminate();
 
 	if (godot_java) {
+		godot_java->on_godot_terminating(env);
 		if (!restart_on_cleanup) {
 			if (p_restart) {
 				godot_java->restart(env);

platform/android/java_godot_wrapper.cpp

@@ -76,6 +76,7 @@ GodotJavaWrapper::GodotJavaWrapper(JNIEnv *p_env, jobject p_activity, jobject p_
 	_get_input_fallback_mapping = p_env->GetMethodID(godot_class, "getInputFallbackMapping", "()Ljava/lang/String;");
 	_on_godot_setup_completed = p_env->GetMethodID(godot_class, "onGodotSetupCompleted", "()V");
 	_on_godot_main_loop_started = p_env->GetMethodID(godot_class, "onGodotMainLoopStarted", "()V");
+	_on_godot_terminating = p_env->GetMethodID(godot_class, "onGodotTerminating", "()V");
 	_create_new_godot_instance = p_env->GetMethodID(godot_class, "createNewGodotInstance", "([Ljava/lang/String;)I");
 	_get_render_view = p_env->GetMethodID(godot_class, "getRenderView", "()Lorg/godotengine/godot/GodotRenderView;");
 	_begin_benchmark_measure = p_env->GetMethodID(godot_class, "nativeBeginBenchmarkMeasure", "(Ljava/lang/String;Ljava/lang/String;)V");
@@ -136,6 +137,16 @@ void GodotJavaWrapper::on_godot_main_loop_started(JNIEnv *p_env) {
 	}
 }
 
+void GodotJavaWrapper::on_godot_terminating(JNIEnv *p_env) {
+	if (_on_godot_terminating) {
+		if (p_env == nullptr) {
+			p_env = get_jni_env();
+		}
+		ERR_FAIL_NULL(p_env);
+		p_env->CallVoidMethod(godot_instance, _on_godot_terminating);
+	}
+}
+
 void GodotJavaWrapper::restart(JNIEnv *p_env) {
 	if (_restart) {
 		if (p_env == nullptr) {

platform/android/java_godot_wrapper.h

@@ -68,6 +68,7 @@ private:
 	jmethodID _get_input_fallback_mapping = nullptr;
 	jmethodID _on_godot_setup_completed = nullptr;
 	jmethodID _on_godot_main_loop_started = nullptr;
+	jmethodID _on_godot_terminating = nullptr;
 	jmethodID _create_new_godot_instance = nullptr;
 	jmethodID _get_render_view = nullptr;
 	jmethodID _begin_benchmark_measure = nullptr;
@@ -85,6 +86,7 @@ public:
 
 	void on_godot_setup_completed(JNIEnv *p_env = nullptr);
 	void on_godot_main_loop_started(JNIEnv *p_env = nullptr);
+	void on_godot_terminating(JNIEnv *p_env = nullptr);
 	void restart(JNIEnv *p_env = nullptr);
 	bool force_quit(JNIEnv *p_env = nullptr, int p_instance_id = 0);
 	void set_keep_screen_on(bool p_enabled);