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

[api-merge] add deprecation platform information to API description #824

Merged
merged 1 commit into from
Sep 6, 2017

Conversation

grendello
Copy link
Contributor

While the merged API description file contains information about
deprecation status of a given API, it lacks information on when
the API (which Android platform) was deprecated. This information
will be needed for new Java code generator (currently WIP).

To make gathering the information easier, each platform .xml.in
file gains a platform attribute in its api root element.

While the merged API description file contains information about
deprecation status of a given API, it lacks information on when
the API (which Android platform) was deprecated. This information
will be needed for new Java code generator (currently WIP).

To make gathering the information easier, each platform .xml.in
file gains a `platform` attribute in its `api` root element.
@atsushieno
Copy link
Contributor

I'm not sure, how will "deprecated-since" information be useful?

For removal of API, I had defined "api-until" attribute (to be paired with "api-since") before.
https://github.com/xamarin/java.interop/blob/2986b248f96119771adabe4cf2cf2c71d23c5c7e/tools/generator/ApiFixup.cs#L37

If that's what you actually intended to achieve, it would be useful.

@grendello
Copy link
Contributor Author

grendello commented Sep 6, 2017

@atsushieno it will be used by the new code generator to put #ifdefs in the generated code so that we can generate everything once instead of separately per-api. It will be used together with api-{since,until} for that purpose

@atsushieno atsushieno merged commit 938c349 into dotnet:master Sep 6, 2017
@grendello grendello deleted the api-merge-mods branch September 6, 2017 18:02
atsushieno added a commit to atsushieno/java.interop that referenced this pull request Oct 25, 2017
dotnet/android#824 added some code to
api-merge to handle "platform" attributes on <api> element, but those
attributes on api-*.xml.in were not automatically generated. That means,
every time we regenerate api-*.xml.in from api-xml-adjuster, those files
are invalid and causes build breakages.

Add more code in class-parse and api-xml-adjuster so that they can be
automatically added, depending on the command line.
grendello pushed a commit to dotnet/java-interop that referenced this pull request Oct 25, 2017
…t. (#197)

dotnet/android#824 added some code to
api-merge to handle "platform" attributes on <api> element, but those
attributes on api-*.xml.in were not automatically generated. That means,
every time we regenerate api-*.xml.in from api-xml-adjuster, those files
are invalid and causes build breakages.

Add more code in class-parse and api-xml-adjuster so that they can be
automatically added, depending on the command line.
Redth pushed a commit to Redth/java.interop that referenced this pull request Oct 26, 2017
…t. (dotnet#197)

dotnet/android#824 added some code to
api-merge to handle "platform" attributes on <api> element, but those
attributes on api-*.xml.in were not automatically generated. That means,
every time we regenerate api-*.xml.in from api-xml-adjuster, those files
are invalid and causes build breakages.

Add more code in class-parse and api-xml-adjuster so that they can be
automatically added, depending on the command line.
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.

3 participants