Skip to content

Commit

Permalink
[monodroid] AndroidEnablePreloadAssemblies causes runtime crash. (#…
Browse files Browse the repository at this point in the history
…5883)

Context: #5838

Setting `$(AndroidEnablePreloadAssemblies)`=False, which disables
assembly preloading (see af02a65), 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()`.
  • Loading branch information
dellis1972 authored Apr 30, 2021
1 parent 032d840 commit 9c04378
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/monodroid/jni/monodroid-glue-internal.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions src/monodroid/jni/monodroid-glue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand All @@ -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))) {
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ();

Expand All @@ -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"
Expand Down

0 comments on commit 9c04378

Please sign in to comment.