Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[monodroid] AndroidEnablePreloadAssemblies causes runtime crash. #5883

Merged
merged 2 commits into from
Apr 30, 2021

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Apr 28, 2021

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 NullReferenceExceptions:

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().

Fixes dotnet#5838

Turning off Assembly preloading results in the following runtime error

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

This only occurs on one unit test [`DebuggingTest.ClassLibraryMainLauncherRuns()`](https://github.com/xamarin/xamarin-android/blob/bf4f4f42af26cdddb46087deed59aae8424f7942/tests/MSBuildDeviceIntegration/Tests/DebuggingTest.cs#L74-L124)
This happens because the `MainActivity` in that test is in a library project.
As a result when we do not preload assemblies the main assembly is NEVER
loaded. This means `ResourceIdManager.UpdateIdValues()` never finds the
`Resource` class from the main assembly, and as such none of the ids are
updated.

To fix this , when running without assembly preloading we will ALWAYS
load the first assembly in the assembly list. This will always be the
main assembly and will contain the `Resource` class.
@jonpryor
Copy link
Member

jonpryor commented Apr 29, 2021

@dellis1972: as per current understanding, any app should crash if/when:

  1. $(AndroidEnablePreloadAssemblies)=False, and

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

  3. The launched Activity uses Android Resource IDs, and

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

for both .NET 6 & Legacy.

Could you please update/"overload" the DebuggingTest.ClassLibraryMainLauncherRuns() unit test so that $(AndroidEnablePreloadAssemblies)=False?

@jonpryor
Copy link
Member

WIP commit message:

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()`.

@jonpryor
Copy link
Member

@dellis1972: additionally, we should verify behavior with an F# app, as F# apps don't include the Resources into the "App" assembly. How's that work?

@dellis1972
Copy link
Contributor Author

@jonpryor it took a while but I managed to put a F# project together which repo's the error on the stable release. Testing this fix with it works, so this seems to work for both C# and F#.

@jonpryor jonpryor merged commit 9c04378 into dotnet:main Apr 30, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants