From 9c0437866c7308794283b76125483adecdea9067 Mon Sep 17 00:00:00 2001
From: Dean Ellis <dellis1972@googlemail.com>
Date: Fri, 30 Apr 2021 18:41:27 +0100
Subject: [PATCH] [monodroid] `AndroidEnablePreloadAssemblies` causes runtime
 crash. (#5883)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Context: https://github.com/xamarin/xamarin-android/issues/5838

Setting `$(AndroidEnablePreloadAssemblies)`=False, which disables
assembly preloading (see af02a65a), could result in a crash when:

 1. The launched `Activity` isn't located within the "main App
    assembly", the assembly built as part of the `App.apk` build, and

 2. The launched `Activity` uses Android Resource IDs, and

 3. Those Android Resource IDs differ between the Library project
    build and the Application project build (usually the case).

This setup is exemplified by the
`DebuggingTest.ClassLibraryMainLauncherRuns()` unit test.

Our current Resource architecture assumes/ensures that there is
Only One™ `Resource` type mentioned by an assembly-level
`ResourceDesignerAttribute` custom attribute, with
`ResourceDesignerAttribute.IsApplication`=true:

	[assembly: global::Android.Runtime.ResourceDesignerAttribute("My.Resource", IsApplication=true)]

The `ResourceIdManager.UpdateIdValues()` method looks for this
attribute in all loaded assemblies, and when (if) found, will
invoke the `UpdateIdValues()` static method on the specified type.

The problem is that only the *App assembly* `Resource.UpdateIdValues()`
method does anything, and if the App assembly isn't loaded -- which
may be the case when the `Activity` loaded isn't from the App assembly
and assembly preload is disabled -- then:

 1. `ResourceIdManager.UpdateIdValues()` never finds the
    `ResourceDesignerAttribute`, and

 2. `Resource.UpdateIdValues()` is never invoked, and

 3. Android Resource IDs in non-App assemblies are never updated to
    contain their correct values.

Thus, attempting to use an Android resource may result in unexpected
failures:

	var button = FindViewById<Button> (Resource.Id.myButton);
	// `button` is null, because `Resource.Id.myButton` has the wrong value.

This can result in `NullReferenceException`s:

	android.runtime.JavaProxyThrowable: System.NullReferenceException: Object reference not set to an instance of an object
	    at MyLibrary.MainActivity.OnCreate(Bundle bundle)
	    at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState)
	    at com.xamarin.classlibrarymainlauncherruns.MainActivity.n_onCreate(Native Method)
	    at com.xamarin.classlibrarymainlauncherruns.MainActivity.onCreate(MainActivity.java:29)

Fix this by *always* loading the App assembly during process startup,
which also happens to be the *first* assembly listed within the
generated `mono.MonoPackageManager_Resources.Assemblies` array, which
is provided to `Runtime.initInternal()`.
---
 src/monodroid/jni/monodroid-glue-internal.hh          | 2 +-
 src/monodroid/jni/monodroid-glue.cc                   | 9 ++++++---
 tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/monodroid/jni/monodroid-glue-internal.hh b/src/monodroid/jni/monodroid-glue-internal.hh
index 6bcbe179e4e..f93b52bb936 100644
--- a/src/monodroid/jni/monodroid-glue-internal.hh
+++ b/src/monodroid/jni/monodroid-glue-internal.hh
@@ -224,7 +224,7 @@ namespace xamarin::android::internal
 		void disable_external_signal_handlers ();
 		void lookup_bridge_info (MonoDomain *domain, MonoImage *image, const OSBridge::MonoJavaGCBridgeType *type, OSBridge::MonoJavaGCBridgeInfo *info);
 		void load_assembly (MonoDomain *domain, jstring_wrapper &assembly);
-		void load_assemblies (MonoDomain *domain, jstring_array_wrapper &assemblies);
+		void load_assemblies (MonoDomain *domain, bool preload, jstring_array_wrapper &assemblies);
 		void set_debug_options ();
 		void parse_gdb_options ();
 		void mono_runtime_init (dynamic_local_string<PROPERTY_VALUE_BUFFER_LEN>& runtime_args);
diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc
index bf7e293ed61..4a242caff94 100644
--- a/src/monodroid/jni/monodroid-glue.cc
+++ b/src/monodroid/jni/monodroid-glue.cc
@@ -1595,7 +1595,7 @@ MonodroidRuntime::load_assembly (MonoDomain *domain, jstring_wrapper &assembly)
 }
 
 inline void
-MonodroidRuntime::load_assemblies (MonoDomain *domain, jstring_array_wrapper &assemblies)
+MonodroidRuntime::load_assemblies (MonoDomain *domain, bool preload, jstring_array_wrapper &assemblies)
 {
 	timing_period total_time;
 	if (XA_UNLIKELY (utils.should_log (LOG_TIMING)))
@@ -1604,6 +1604,9 @@ MonodroidRuntime::load_assemblies (MonoDomain *domain, jstring_array_wrapper &as
 	for (size_t i = 0; i < assemblies.get_length (); ++i) {
 		jstring_wrapper &assembly = assemblies [i];
 		load_assembly (domain, assembly);
+		// only load the first "main" assembly if we are not preloading.
+		if (!preload)
+			break;
 	}
 
 	if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) {
@@ -1638,8 +1641,8 @@ MonodroidRuntime::create_and_initialize_domain (JNIEnv* env, jclass runtimeClass
 	if (assembliesBytes != nullptr)
 		designerAssemblies.add_or_update_from_java (domain, env, assemblies, assembliesBytes, assembliesPaths);
 #endif
-	if (androidSystem.is_assembly_preload_enabled () || (is_running_on_desktop && force_preload_assemblies))
-		load_assemblies (domain, assemblies);
+	bool preload = (androidSystem.is_assembly_preload_enabled () || (is_running_on_desktop && force_preload_assemblies));
+	load_assemblies (domain, preload, assemblies);
 	init_android_runtime (domain, env, runtimeClass, loader);
 
 	osBridge.add_monodroid_domain (domain);
diff --git a/tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs b/tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs
index df16904c5be..693bad0e871 100755
--- a/tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs
+++ b/tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs
@@ -72,7 +72,7 @@ public void ApplicationRunsWithoutDebugger ([Values (false, true)] bool isReleas
 		}
 
 		[Test]
-		public void ClassLibraryMainLauncherRuns ()
+		public void ClassLibraryMainLauncherRuns ([Values (true, false)] bool preloadAssemblies)
 		{
 			AssertHasDevices ();
 
@@ -85,6 +85,7 @@ public void ClassLibraryMainLauncherRuns ()
 				app.SetAndroidSupportedAbis ("armeabi-v7a", "x86");
 			}
 			app.SetDefaultTargetDevice ();
+			app.SetProperty ("AndroidEnablePreloadAssemblies", preloadAssemblies.ToString ());
 
 			var lib = new XamarinAndroidLibraryProject {
 				ProjectName = "MyLibrary"