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

[Xamarin.Android.Build.Tasks] _UpdateAndroidResgen is throwing new AP… #823

Merged
merged 1 commit into from
Sep 6, 2017

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Sep 6, 2017

…T0000 errors in d15-4 (#769)

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=58889
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=58890

Commit 0667a2b and ea6b9b4 added support for short path names.
However during the rework we added code to replace

library_project_imports/

with an empty string. Some of the Nuget Support packages seem
to include a __AndroidLibraryProjects__.zip which uses
windows path sepatators rather than unix ones. So the code
to strip off the library_project_imports/ does not work.
We end up with a directory structure like

__library_projects__/[dllname]/library_project_imports/library_project_imports

which a) does not get picked up by our tasks and b) takes up
a bunch of additional characters in our already long path..

The result is the two errors above... one to do with missing resources
the other a PathToLong error.

Because the nuget packages have already shipped and will be in use,
we need to handle BOTH cases.

…T0000 errors in d15-4 (dotnet#769)

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=58889
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=58890

Commit 0667a2b and ea6b9b4 added support for short path names.
However during the rework we added code to replace

	library_project_imports/

with an empty string. Some of the Nuget Support packages seem
to include a `__AndroidLibraryProjects__.zip` which uses
windows path sepatators rather than unix ones. So the code
to strip off the `library_project_imports/` does not work.
We end up with a directory structure like

	__library_projects__/[dllname]/library_project_imports/library_project_imports

which a) does not get picked up by our tasks and b) takes up
a bunch of additional characters in our already long path..

The result is the two errors above... one to do with missing resources
the other a PathToLong error.

Because the nuget packages have already shipped and will be in use,
we need to handle BOTH cases.
@Aguilex Aguilex merged commit 773712f into dotnet:d15-4 Sep 6, 2017
jonpryor pushed a commit that referenced this pull request Apr 22, 2021
Context: dotnet/java-interop@ebd7d76
Context: dotnet/java-interop@f9faaab

Changes: dotnet/java-interop@a3de91e...f9faaab

  * dotnet/java-interop@f9faaaba: [jcw-gen] Skip interface types (#825)
  * dotnet/java-interop@64399900: [Java.Interop.Tools.JavaCallableWrappers] Fix typo.
  * dotnet/java-interop@ebd7d761: [Java.Interop.Tools.JavaCallableWrappers] Include interfaces in type maps (#824)
  * dotnet/java-interop@002dea4a: Bump to xamarin/xamarin-android-tools/main@d92fc3e3 (#823)

Fix a runtime assertion failure within a `Java.Interop.JniPeerMembers`
constructor when `JniRuntime.JniTypeManager.GetTypeSignature()` is
used with a bound interface type:

	var type    = typeof(global::Java.Lang.Iterable);
	var sig     = JniEnvironment.Runtime.TypeManager.GetTypeSignature(type);
	var jniType = sig.JniTypeName;

The expectation is that `jniType` should be `java/lang/Iterable`.

In actuality, `jniType` was the empty string `""`.

When `JniTypeSignature.JniTypeName` didn't match the expected value,
an assertion failed, causing the app to crash:

	---- DEBUG ASSERTION FAILED ----
	---- Assert Short Message ----
	ManagedPeerType <=> JniTypeName Mismatch! javaVM.GetJniTypeInfoForType(typeof(Java.Lang.IIterable)).JniTypeName="" != "java/lang/Iterable"
	---- Assert Long Message ----
	    at System.Diagnostics.DebugProvider.Fail(String message, String detailMessage)
	    at System.Diagnostics.Debug.Fail(String message, String detailMessage)
	    at System.Diagnostics.Debug.Assert(Boolean condition, String message, String detailMessage)
	    at System.Diagnostics.Debug.Assert(Boolean condition, String message)
	    at Java.Interop.JniPeerMembers..ctor(String jniPeerTypeName, Type managedPeerType, Boolean checkManagedPeerType, Boolean isInterface)
	Process terminated due to "ManagedPeerType <=> JniTypeName Mismatch! javaVM.GetJniTypeInfoForType(typeof(Java.Lang.IIterable)).JniTypeName="" != "java/lang/Iterable"
	   …

The "cause" of the crash is that type map files did not contain
interfaces.  There was no actual "need" to store typemap data for
interfaces, as the most common use of `JniPeerMembers` was for bound
types, and our interface bindings (mostly) didn't use `JniPeerMembers`,
so the lack of typemap data for interfaces wasn't a problem.

This "lack of need" changed with dotnet/java-interop@29f97075, when
Java default interface methods were bound as C# Interface Default
Members.  *Now*, interface bindings *can* use `JniPeerMembers`:

	public partial interface IIterable : IJavaObject, IJavaPeerable {
	  private static readonly JniPeerMembers _members = new XAPeerMembers ("java/lang/Iterable", typeof (IIterable), isInterface: true);

	  virtual unsafe void ForEach(Java.Util.Functions.IConsumer action) => …
	}

*However*, the assertion is only reached if a default interface
method is executed, as that's what causes e.g. `IIterable._members`
to be constructed, triggering the assertion.

Related question: why didn't we see this back in Fall 2020?

Two reasons:

 1. Most important, this was a `Debug.Assert()` assertion, which in
    turn requires that the assembly be built with `DEBUG` defined.
    This is *never* the case for assemblies shipped to customers;
    those are built in the Release configuration.  Thus, this could
    only happen to developers/maintainers of Xamarin.Android.

 2. When *Mono* is used, `Debug.Assert()` prints out a message, and
    continues execution.  The process does not crash.

    This changes with .NET 6: `Debug.Assert()` failures will abort
    the process!

Thus, the only "reasonable" way to trigger this assert was to:

 1. Be a maintainer of xamarin-android, and have Debug builds of the
    product.
 
 2. Be running on .NET 6.

 3. "Just happen" to be running an app that makes use of interface
    default members.

    In this particular case, the [HelloMaui][0] sample.

The fix is twofold:

 1. Start including interfaces within typemaps.

 2. Update Java Callable Wrapper generation to *ignore* interfaces.

    This is necessary because our Java Callable Wrapper infrastructure
    doesn't support "exporting" C# interfaces to Java, and will thus
    throw an exception if we attempt to do so.

This allows `HelloMaui` to launch on .NET 6 with Debug builds of
Xamarin.Android without triggering the assertion.

[0]: https://github.com/dotnet/net6-mobile-samples/tree/46129f85d78b55589280e5bb0ea399bb1dd72167/HelloMaui
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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.

4 participants