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

Link assemblies causes app crashes if you have an EditText in VS2019 Preview 2 #3263

Closed
FANMixco opened this issue Jun 19, 2019 · 22 comments · Fixed by #3364
Closed

Link assemblies causes app crashes if you have an EditText in VS2019 Preview 2 #3263

FANMixco opened this issue Jun 19, 2019 · 22 comments · Fixed by #3364
Labels
Area: App Runtime Issues in `libmonodroid.so`. bug Component does not function as intended. vs-sync For internal use only; creates a VSTS "mirror" issue.
Milestone

Comments

@FANMixco
Copy link

FANMixco commented Jun 19, 2019

Steps to Reproduce

  1. Add an EditText
  2. Choose TextChanged. (Example: txtSearch.TextChanged += TxtSearch_TextChanged;)
  3. Choose Linking with either "Sdk Assemblies Only" or "Sdk and User Assemblies".
  4. Execute the program.

Expected Behavior

Be able to execute the TextChanged on Linking with either "Sdk Assemblies Only" or "Sdk and User Assemblies".

Actual Behavior

App crashes because it cannot load ITextWatcherInvoker.

Version Information

Windows 10, May Update.
Visual Studio 2019 Preview 2.

Log File

System.TypeLoadException: 'Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.'

Annotation 2019-06-20 003519

P.S.

It's working in Visual Studio for Mac.

VS bug #947890

@FANMixco FANMixco changed the title Link assemblies causes caused task crash if you have an EditText in VS2019 Preview 2 Link assemblies causes app crashes if you have an EditText in VS2019 Preview 2 Jun 19, 2019
@pjcollins pjcollins added the need-info Issues that need more information from the author. label Jun 20, 2019
@pjcollins pjcollins added this to the Under Consideration milestone Jun 20, 2019
@pjcollins
Copy link
Member

Thanks for the report! I assume you're referring to VS 2019 16.2 Preview 2? Could you paste the information displayed in Help -> About Microsoft Visual Studio? I was unable to reproduce this with the upcoming VS 2019 16.2 Preview 3 release, though my repro may not have been sufficient. Could you also attach a solution file that demonstrates this crash?

@FANMixco
Copy link
Author

Thanks for the report! I assume you're referring to VS 2019 16.2 Preview 2? Could you paste the information displayed in Help -> About Microsoft Visual Studio? I was unable to reproduce this with the upcoming VS 2019 16.2 Preview 3 release, though my repro may not have been sufficient. Could you also attach a solution file that demonstrates this crash?

Yes, it's in the Preview 2 of VS 2019 for Windows only, I could try maybe sharing a OneDrive link because the solution is somehow huge more than a 100Mb.

@gmck
Copy link

gmck commented Jul 3, 2019

I have exactly the same error however I'm using 16.2.0 Preview 3. I'm going to try an reproduce in a test project

@pjcollins
Copy link
Member

I've got a repro in https://github.com/xamarin/monodroid-samples/blob/master/android5.0/Topeka/Topeka/Fragments/SignInFragment.cs#L90, just build and deploy the app in release mode and it'll crash on startup.

@pjcollins pjcollins added Area: App Runtime Issues in `libmonodroid.so`. Area: Mono Runtime Mono-related issues: BCL bugs, AOT issues, etc. regression and removed need-info Issues that need more information from the author. Area: Mono Runtime Mono-related issues: BCL bugs, AOT issues, etc. labels Jul 3, 2019
@pjcollins pjcollins modified the milestones: Under Consideration, d16-2 Jul 3, 2019
@FANMixco
Copy link
Author

FANMixco commented Jul 7, 2019

I've got a repro in https://github.com/xamarin/monodroid-samples/blob/master/android5.0/Topeka/Topeka/Fragments/SignInFragment.cs#L90, just build and deploy the app in release mode and it'll crash on startup.

The Issue expanded to the latest version of Visual Studio for Mac.

@FANMixco
Copy link
Author

Hi @radekdoulik , do we have any news in which release is going to be a fix about this issue? Preview 5? Or Later? Thanks.

@radekdoulik
Copy link
Member

Don't know about the release version yet. I can reproduce the issue locally and I am looking into it.

@JonDouglas JonDouglas added vs-sync For internal use only; creates a VSTS "mirror" issue. bug Component does not function as intended. labels Jul 16, 2019
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Jul 17, 2019
Fixes dotnet#3263

Looks like
dotnet@5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](dotnet@5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Jul 18, 2019
radekdoulik added a commit that referenced this issue Jul 19, 2019
[linker] Make sure we mark *Invoker types

Fixes #3263

Looks like
5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.

Added test to Mono.Android runtime test, which should catch the issue
if it happens again.
@FANMixco
Copy link
Author

Don't know about the release version yet. I can reproduce the issue locally and I am looking into it.

I have a similar issue with CoordinatorLayouts, should I share the request here too?

jonpryor pushed a commit that referenced this issue Jul 19, 2019
[linker] Make sure we mark *Invoker types

Fixes #3263

Looks like
5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.

Added test to Mono.Android runtime test, which should catch the issue
if it happens again.
@jonpryor
Copy link
Member

@FANMixco asked:

I have a similar issue with CoordinatorLayouts, should I share the request here too?

Please do. Is it an TypeLoadException with the same ITextWatcherInvoker type, or is it a different exception or missing type?

Are you able to use any of the recent d16-3 or master builds to see if that fixes things for you?

Thanks,

  • Jon

@FANMixco
Copy link
Author

FANMixco commented Jul 22, 2019

@FANMixco asked:

I have a similar issue with CoordinatorLayouts, should I share the request here too?

Please do. Is it an TypeLoadException with the same ITextWatcherInvoker type, or is it a different exception or missing type?

Are you able to use any of the recent d16-3 or master builds to see if that fixes things for you?

Thanks,

  • Jon

I'm going to check during the week, also, here is the new bug:

#3376

@pjcollins
Copy link
Member

The Topeka repro mentioned above is now working for me with the latest d16-3 build.

I was unable to reproduce the issue mentioned in #3376 in both 16.1 and 16.3 just by quickly modifying a new template, so I think we'll want to follow up on that issue separately. If you could attach logcat output and/or a project which reproduces the crash to the new issue you've filed that would be very helpful!

@brendanzagaeski

This comment has been minimized.

@MichaelBourkeCF
Copy link

Workaround notes for "System.TypeLoadException: 'Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android"

In case any other users come across this issue when using Xamarin.Android 9.4, a possible workaround is to use a custom linker configuration to preserve the missing types. To do that, add a new linker.xml file to the project, set the Build Action to LinkDescription, and add the XML lines to preserve the missing types. For example, for the ITextWatcherInvoker error, add the following lines to the file:

<linker>
  <assembly fullname="Xamarin.Android">
    <type fullname="Android.Text.ITextWatcherInvoker" preserve="all" />
  </assembly>
</linker>

(Note: This workaround is for the symptom described at the beginning of this issue.)

Hi, this should be "Mono.Android" rather than "Xamarin.Android" in the xml file. It won't compile otherwise.

@brendanzagaeski
Copy link
Contributor

Correcting my previous comment. Thanks MichaelBourkeCF!

Workaround notes for "System.TypeLoadException: 'Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android"

In case any other users come across this issue when using Xamarin.Android 9.4, a possible workaround is to use a custom linker configuration to preserve the missing types. To do that, add a new linker.xml file to the project, set the Build Action to LinkDescription, and add the XML lines to preserve the missing types. For example, for the ITextWatcherInvoker error, add the following lines to the file:

<linker>
  <assembly fullname="Mono.Android">
    <type fullname="Android.Text.ITextWatcherInvoker" preserve="all" />
  </assembly>
</linker>

(Note: This workaround is for the symptom described at the beginning of this issue.)

@ushenkodmitry
Copy link

I have the same error in Visual Studio Community 2019 16.2.0.

@lexboss777
Copy link

lexboss777 commented Jul 31, 2019

I have rolled back to xamarin.android 9.2.3-0 to bypass this issue. 9.2.3-0 is the most stable version from recent ones imho.
Rolling back is easy. If you already have an old version on your system just change the "Current" symlink in /Library/Frameworks/Xamarin.Android.framework/Versions. If don't, then just download it first.

jonpryor pushed a commit that referenced this issue Jul 31, 2019
[linker] Make sure we mark *Invoker types

Fixes #3263

Looks like
5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.

Added test to Mono.Android runtime test, which should catch the issue
if it happens again.
@brendanzagaeski
Copy link
Contributor

Windows fix published. The new Xamarin.Android SDK version 9.4.1.0 that includes a fix for this issue has now been published as part of Visual Studio 2019 version 16.2.1. Check for the latest updates or install the most recent release from https://visualstudio.microsoft.com/downloads/ to get the fix.

macOS fix published. The new Xamarin.Android SDK version 9.4.1.0 that includes a fix for this issue has now been released in the Stable updater channel in Visual Studio for Mac. Check for the latest updates or install the most recent release from https://visualstudio.microsoft.com/downloads/ to get the fix.

@subsembly
Copy link

Thanks for the fix. However, it seems that now there is no linking done at all. My build now produces a much larger APK output, regardless of whether I select link SDK or link None. Is this the expected behaviour?

@ushenkodmitry
Copy link

Size of my apk has increased from 26mb to 31mb after the update.

@FANMixco
Copy link
Author

FANMixco commented Aug 9, 2019

Windows fix published. The new Xamarin.Android SDK version 9.4.1.0 that includes a fix for this issue has now been published as part of Visual Studio 2019 version 16.2.1. Check for the latest updates or install the most recent release from https://visualstudio.microsoft.com/downloads/ to get the fix.

macOS fix published. The new Xamarin.Android SDK version 9.4.1.0 that includes a fix for this issue has now been released in the Stable updater channel in Visual Studio for Mac. Check for the latest updates or install the most recent release from https://visualstudio.microsoft.com/downloads/ to get the fix.

Hi @brendanzagaeski any idea about the previous issues? Because it seems the issue is a new one then.

@brendanzagaeski
Copy link
Contributor

Thanks for the heads-up! I looked through the open issues, noticed a similar report in #3454, and was able to confirm the problematic linker behavior using a new blank app project as suggested in that issue.

The size increase problem can now be tracked in that issue: #3454.

I'll keep an eye on that issue to make sure the status stays updated to reflect the investigation and release.

radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Aug 20, 2019
Improves fix of dotnet#3263

The
dotnet@4d8c28f
fix increased the apk size more than needed. It marked all the
*Invoker types, beause MarkJavaObjects substep iterates over all
Java.Object derived types, before all the linking.

Turned out that the original regression came from the linker
itself. The new linker optimization was introduced in
dotnet/linker@13ea158
and so the ITextWatcher interface was not marked anymore - as it is
only accessed by reflection and instantiated during the runtime. And
thus the ITeextWatcherInvoker type was linked away too.

It is now fixed by overriding the `ShouldMarkInterfaceImplementation`
method in `MarkStep`, which returns true for registered interfaces.

The crash in Topeka sample:

    I MonoDroid: System.TypeLoadException: Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
    I MonoDroid:   at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool)
    I MonoDroid:   at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName) [0x0008d] in <ab232a84cff64754aaec9d455f50a794>:0
    I MonoDroid:   at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark) [0x0000e] in <ab232a84cff64754aaec9d455f50a794>:0
    I MonoDroid:   at System.Type.GetType (System.String typeName, System.Boolean throwOnError) [0x00002] in <ab232a84cff64754aaec9d455f50a794>:0
    I MonoDroid:   at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType jniType, System.Type type, System.String methods) [0x00131] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len) [0x00128] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_call_static_object_method_a(intptr,intptr&,intptr,intptr,Java.Interop.JniArgumentValue*)
    I MonoDroid:   at Java.Interop.JniEnvironment+StaticMethods.CallStaticObjectMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args) [0x00073] in <fbb1482d0c6b44009a68f9f7e8c6389a>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue* parms) [0x0000f] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue[] parms) [0x0001a] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.FindClass (System.String classname) [0x0003f] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.AllocObject (System.String jniClassName) [0x00001] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue* constructorParameters) [0x0000a] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue[] constructorParameters) [0x00019] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Text.TextWatcherImplementor..ctor (System.Object inst, System.EventHandler`1[TEventArgs] changed_handler, System.EventHandler`1[TEventArgs] before_handler, System.EventHandler`1[TEventArgs] after_handler) [0x00010] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Android.Widget.TextView.add_TextChanged (System.EventHandler`1[TEventArgs] value) [0x0001f] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at Topeka.Fragments.SignInFragment.InitContentViews (Android.Views.View view) [0x00011] in <201b9318435b446d821a19dfd1fa7bfb>:0
    I MonoDroid:   at Topeka.Fragments.SignInFragment.OnViewCreated (Android.Views.View view, Android.OS.Bundle savedInstanceState) [0x0003e] in <201b9318435b446d821a19dfd1fa7bfb>:0
    I MonoDroid:   at Android.App.Fragment.n_OnViewCreated_Landroid_view_View_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.IntPtr native_savedInstanceState) [0x0001a] in <4bc2304bf5d8408baea9dd059d7ea365>:0
    I MonoDroid:   at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.9(intptr,intptr,intptr,intptr)

The comparison of apk sizes with original and this fix (Topeka sample,
which was also used as repro):

Before @4d8c28    21434941 bytes
After  @4d8c28    28242405 bytes
This fix          21638678 bytes

So the previous fix increased the apk size by around 6.5Mbytes, this
fix adds less than 199kbytes instead.
jonpryor pushed a commit that referenced this issue Aug 21, 2019
Fixes: #3454

Context: #3525 (comment)

Improves fix of #3263

The fix in commit
4d8c28f
increased the apk size more than needed.  It marked all the
`*Invoker` types, because the `MarkJavaObjects` substep iterates over
all `Java.Lang.Object` derived types, before all the linking.

Turned out that the original regression came from the linker
itself. The new linker optimization was introduced in
dotnet/linker@13ea158
and so the `ITextWatcher` interface was not marked anymore.

> Interfaces on a type will no longer be marked simply because the
> type is marked. Instead, marking of interfaces on types is deferred
> until that interface type is marked for some other reason.

Before the new optimization, the linker preserved these
interfaces (like `ITextWatcher`) when marking the types based on them
(like `AbsListView` or `ITextWatcherImplementor` in this case).  That
also resulted in keeping the invoker interfaces in the assembly.  The
above mentioned optimization changed that.

Now they were linked out, because the interfaces implementation
are only accessed (during app runtime) after the invoker type is
loaded by `Type.GetType(string,bool)` in the native members
registration process.  The linker cannot know about that without XA
help during the linking.  The result of `ITextWatcher` being linked
away is that the `ITeextWatcherInvoker` type was linked away too.

It is now fixed by overriding the
`MarkStep.ShouldMarkInterfaceImplementation()` method, which returns
true for registered interfaces.  These interfaces are special for XA
in a way, that it can be, in some cases, reached only from Java side
and thus linker is unable to track the interface usage.  It is
similar to how linker handles the COM interfaces.

The crash in Topeka sample:

	System.TypeLoadException: Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
	  at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool)
	  at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName)
	  at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark)
	  at System.Type.GetType (System.String typeName, System.Boolean throwOnError)
	  at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType jniType, System.Type type, System.String methods)
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len)
	  at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_call_static_object_method_a(intptr,intptr&,intptr,intptr,Java.Interop.JniArgumentValue*)
	  at Java.Interop.JniEnvironment+StaticMethods.CallStaticObjectMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args)
	  at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue* parms)
	  at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue[] parms)
	  at Android.Runtime.JNIEnv.FindClass (System.String classname)
	  at Android.Runtime.JNIEnv.AllocObject (System.String jniClassName)
	  at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue* constructorParameters)
	  at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue[] constructorParameters)
	  at Android.Text.TextWatcherImplementor..ctor (System.Object inst, System.EventHandler`1[TEventArgs] changed_handler, System.EventHandler`1[TEventArgs] before_handler, System.EventHandler`1[TEventArgs] after_handler)
	  at Android.Widget.TextView.add_TextChanged (System.EventHandler`1[TEventArgs] value)
	  at Topeka.Fragments.SignInFragment.InitContentViews (Android.Views.View view)
	  at Topeka.Fragments.SignInFragment.OnViewCreated (Android.Views.View view, Android.OS.Bundle savedInstanceState)
	  at Android.App.Fragment.n_OnViewCreated_Landroid_view_View_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.IntPtr native_savedInstanceState)
	  at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.9(intptr,intptr,intptr,intptr)

The comparison of apk sizes with original and this fix (Topeka sample,
which was also used as repro):

| Version        | Size in bytes |
|----------------|--------------:|
| Before @4d8c28 |      21434941 |
| After  @4d8c28 |      28242405 |
| This fix       |      21638678 |

So the previous fix increased the apk size by around 6.5Mbytes, this
fix adds less than 199kbytes instead.
jonpryor pushed a commit that referenced this issue Aug 21, 2019
Fixes: #3454

Context: #3525 (comment)

Improves fix of #3263

The fix in commit
4d8c28f
increased the apk size more than needed.  It marked all the
`*Invoker` types, because the `MarkJavaObjects` substep iterates over
all `Java.Lang.Object` derived types, before all the linking.

Turned out that the original regression came from the linker
itself. The new linker optimization was introduced in
dotnet/linker@13ea158
and so the `ITextWatcher` interface was not marked anymore.

> Interfaces on a type will no longer be marked simply because the
> type is marked. Instead, marking of interfaces on types is deferred
> until that interface type is marked for some other reason.

Before the new optimization, the linker preserved these
interfaces (like `ITextWatcher`) when marking the types based on them
(like `AbsListView` or `ITextWatcherImplementor` in this case).  That
also resulted in keeping the invoker interfaces in the assembly.  The
above mentioned optimization changed that.

Now they were linked out, because the interfaces implementation
are only accessed (during app runtime) after the invoker type is
loaded by `Type.GetType(string,bool)` in the native members
registration process.  The linker cannot know about that without XA
help during the linking.  The result of `ITextWatcher` being linked
away is that the `ITeextWatcherInvoker` type was linked away too.

It is now fixed by overriding the
`MarkStep.ShouldMarkInterfaceImplementation()` method, which returns
true for registered interfaces.  These interfaces are special for XA
in a way, that it can be, in some cases, reached only from Java side
and thus linker is unable to track the interface usage.  It is
similar to how linker handles the COM interfaces.

The crash in Topeka sample:

	System.TypeLoadException: Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
	  at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool)
	  at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName)
	  at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark)
	  at System.Type.GetType (System.String typeName, System.Boolean throwOnError)
	  at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType jniType, System.Type type, System.String methods)
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len)
	  at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_call_static_object_method_a(intptr,intptr&,intptr,intptr,Java.Interop.JniArgumentValue*)
	  at Java.Interop.JniEnvironment+StaticMethods.CallStaticObjectMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args)
	  at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue* parms)
	  at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue[] parms)
	  at Android.Runtime.JNIEnv.FindClass (System.String classname)
	  at Android.Runtime.JNIEnv.AllocObject (System.String jniClassName)
	  at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue* constructorParameters)
	  at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue[] constructorParameters)
	  at Android.Text.TextWatcherImplementor..ctor (System.Object inst, System.EventHandler`1[TEventArgs] changed_handler, System.EventHandler`1[TEventArgs] before_handler, System.EventHandler`1[TEventArgs] after_handler)
	  at Android.Widget.TextView.add_TextChanged (System.EventHandler`1[TEventArgs] value)
	  at Topeka.Fragments.SignInFragment.InitContentViews (Android.Views.View view)
	  at Topeka.Fragments.SignInFragment.OnViewCreated (Android.Views.View view, Android.OS.Bundle savedInstanceState)
	  at Android.App.Fragment.n_OnViewCreated_Landroid_view_View_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.IntPtr native_savedInstanceState)
	  at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.9(intptr,intptr,intptr,intptr)

The comparison of apk sizes with original and this fix (Topeka sample,
which was also used as repro):

| Version        | Size in bytes |
|----------------|--------------:|
| Before @4d8c28 |      21434941 |
| After  @4d8c28 |      28242405 |
| This fix       |      21638678 |

So the previous fix increased the apk size by around 6.5Mbytes, this
fix adds less than 199kbytes instead.
jonpryor pushed a commit that referenced this issue Aug 28, 2019
Fixes: #3454

Context: #3525 (comment)

Improves fix of #3263

The fix in commit
4d8c28f
increased the apk size more than needed.  It marked all the
`*Invoker` types, because the `MarkJavaObjects` substep iterates over
all `Java.Lang.Object` derived types, before all the linking.

Turned out that the original regression came from the linker
itself. The new linker optimization was introduced in
dotnet/linker@13ea158
and so the `ITextWatcher` interface was not marked anymore.

> Interfaces on a type will no longer be marked simply because the
> type is marked. Instead, marking of interfaces on types is deferred
> until that interface type is marked for some other reason.

Before the new optimization, the linker preserved these
interfaces (like `ITextWatcher`) when marking the types based on them
(like `AbsListView` or `ITextWatcherImplementor` in this case).  That
also resulted in keeping the invoker interfaces in the assembly.  The
above mentioned optimization changed that.

Now they were linked out, because the interfaces implementation
are only accessed (during app runtime) after the invoker type is
loaded by `Type.GetType(string,bool)` in the native members
registration process.  The linker cannot know about that without XA
help during the linking.  The result of `ITextWatcher` being linked
away is that the `ITeextWatcherInvoker` type was linked away too.

It is now fixed by overriding the
`MarkStep.ShouldMarkInterfaceImplementation()` method, which returns
true for registered interfaces.  These interfaces are special for XA
in a way, that it can be, in some cases, reached only from Java side
and thus linker is unable to track the interface usage.  It is
similar to how linker handles the COM interfaces.

The crash in Topeka sample:

	System.TypeLoadException: Could not load type 'Android.Text.ITextWatcherInvoker' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
	  at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool)
	  at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName)
	  at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark)
	  at System.Type.GetType (System.String typeName, System.Boolean throwOnError)
	  at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType jniType, System.Type type, System.String methods)
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len)
	  at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_call_static_object_method_a(intptr,intptr&,intptr,intptr,Java.Interop.JniArgumentValue*)
	  at Java.Interop.JniEnvironment+StaticMethods.CallStaticObjectMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args)
	  at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue* parms)
	  at Android.Runtime.JNIEnv.CallStaticObjectMethod (System.IntPtr jclass, System.IntPtr jmethod, Android.Runtime.JValue[] parms)
	  at Android.Runtime.JNIEnv.FindClass (System.String classname)
	  at Android.Runtime.JNIEnv.AllocObject (System.String jniClassName)
	  at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue* constructorParameters)
	  at Android.Runtime.JNIEnv.StartCreateInstance (System.String jniClassName, System.String jniCtorSignature, Android.Runtime.JValue[] constructorParameters)
	  at Android.Text.TextWatcherImplementor..ctor (System.Object inst, System.EventHandler`1[TEventArgs] changed_handler, System.EventHandler`1[TEventArgs] before_handler, System.EventHandler`1[TEventArgs] after_handler)
	  at Android.Widget.TextView.add_TextChanged (System.EventHandler`1[TEventArgs] value)
	  at Topeka.Fragments.SignInFragment.InitContentViews (Android.Views.View view)
	  at Topeka.Fragments.SignInFragment.OnViewCreated (Android.Views.View view, Android.OS.Bundle savedInstanceState)
	  at Android.App.Fragment.n_OnViewCreated_Landroid_view_View_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.IntPtr native_savedInstanceState)
	  at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.9(intptr,intptr,intptr,intptr)

The comparison of apk sizes with original and this fix (Topeka sample,
which was also used as repro):

| Version        | Size in bytes |
|----------------|--------------:|
| Before @4d8c28 |      21434941 |
| After  @4d8c28 |      28242405 |
| This fix       |      21638678 |

So the previous fix increased the apk size by around 6.5Mbytes, this
fix adds less than 199kbytes instead.
@ghost ghost locked as resolved and limited conversation to collaborators Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App Runtime Issues in `libmonodroid.so`. bug Component does not function as intended. vs-sync For internal use only; creates a VSTS "mirror" issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.