[Xamarin.Android.Build.Tasks] Invokers after abstracts in typemaps by jonpryor · Pull Request #9545 · dotnet/android (original) (raw)
Fixes: #9535
Context: a017561
Context: 35f41dc
Create a new .NET for Android project (dotnet new android
), add
a @(PackageReference)
to Xamarin.AndroidX.Media3.ExoPlayer,
and add the following code:
var builder = new AndroidX.Media3.ExoPlayer.ExoPlayerBuilder(this);
var player = builder.Build();
Build and run in Debug configuration, and it crashes:
System.ArgumentException: Could not determine Java type corresponding to `AndroidX.Media3.ExoPlayer.IExoPlayerInvoker, Xamarin.AndroidX.Media3.ExoPlayer, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null`. (Parameter 'targetType')
at Java.Interop.TypeManager.CreateInstance(IntPtr handle, JniHandleOwnership transfer, Type targetType) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Java.Interop/TypeManager.cs:line 323
at Java.Lang.Object.GetObject(IntPtr handle, JniHandleOwnership transfer, Type type) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Java.Lang/Object.cs:line 302
at Java.Lang.Object._GetObject[IExoPlayer](IntPtr handle, JniHandleOwnership transfer) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Java.Lang/Object.cs:line 288
at Java.Lang.Object.GetObject[IExoPlayer](IntPtr handle, JniHandleOwnership transfer) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Java.Lang/Object.cs:line 280
at AndroidX.Media3.ExoPlayer.ExoPlayerBuilder.Build() in D:\a\_work\1\s\generated\androidx.media3.media3-exoplayer\obj\Release\net8.0-android\generated\src\AndroidX.Media3.ExoPlayer.IExoPlayer.cs:line 749
at media3.MainActivity.OnCreate(Bundle savedInstanceState) in /Users/moljac/Downloads/1036/media3/MainActivity.cs:line 16
at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/obj/Release/net9.0/android-35/mcw/Android.App.Activity.cs:line 3196
at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs:line 121
☹️
The problem, as often appears to be the case these days, is an
unexpected interaction between different things.
Firstly, the immediate cause of the crash is 35f41dc, which
updated TypeManager.CreateInstance()
to attempt to lookup the Java
type that corresponds to a possible *Invoker
type:
// `type` is `typeof(AndroidX.Media3.ExoPlayer.IExoPlayerInvoker)`
var typeSig = JNIEnvInit.androidRuntime?.TypeManager.GetTypeSignature (type) ?? default;
if (!typeSig.IsValid || typeSig.SimpleReference == null) {
throw new ArgumentException ($"Could not determine Java type corresponding to `{type.AssemblyQualifiedName}`.", nameof (targetType));
}
The underlying cause of the crash, and what 35f41dc uncovered, is
that there is no type mapping for IExoPlayerInvoker
. This is
(not quite) readily seen by examining the generated file:
% grep IExoPlayerInvoker obj/Debug/net*android*/android/typemaps.arm64-v8a.ll
# no match
This is also true on .NET 8: there is no type mapping forIExoPlayerInvoker
.
There are type mappings for other interface types, e.g.IIntSupplier
:
% grep IIntSupplier obj/Debug/net*android*/android/typemaps.arm64-v8a.ll
@.TypeMapEntry.16871_from = private unnamed_addr constant [47 x i8] c"Java.Util.Functions.IIntSupplier, Mono.Android\00", align 1
@.TypeMapEntry.16873_from = private unnamed_addr constant [54 x i8] c"Java.Util.Functions.IIntSupplierInvoker, Mono.Android\00", align 1
A bit more digging around, and the "problem" is as follows: type map
generation had a type ordering requirement, and needed theInvoker
types to follow their corresponding abstract types.
# Mono.Android.dll: IIntSupplier is before IIntSupplierInvoker
% monodis --typedef obj/Debug/net9.0-android/android/assets/arm64-v8a/Mono.Android.dll | grep IIntSupplier
7676: Java.Util.Functions.IIntSupplier (flist=57465, mlist=135759, flags=0x1000a1, extends=0x0)
7677: Java.Util.Functions.IIntSupplierInvoker (flist=57465, mlist=135760, flags=0x100000, extends=0x7c28)
# Xamarin.AndroidX.Media3.ExoPlayer.dll: IExoPlayerInvoker is *before* IExoPlayer
% monodis --typedef obj/Debug/net9.0-android/android/assets/arm64-v8a/Xamarin.AndroidX.Media3.ExoPlayer.dll| grep IExoPlayer
139: AndroidX.Media3.ExoPlayer.IExoPlayerInvoker (flist=20, mlist=567, flags=0x100000, extends=0x85)
…
166: AndroidX.Media3.ExoPlayer.IExoPlayer (flist=598, mlist=1671, flags=0x1000a1, extends=0x0)
Reasonable aside: why is IExoPlayerInvoker
before IExoPlayer
?
Because it's a "real" checked-in copy of generated code, not
generated code that follows the IExoPlayer
generated declaration.
The reason why ordering matters comes from a017561:
Update typemap generation code in
Xamarin.Android.Build.Tasks.dll
so
that all the duplicate Java type names will point to the same managed
type name. Additionally, make sure we select the managed type in the
same fashion the old typemap generator inJava.Interop
worked: prefer
base types to the derived ones if the type is an interface or an
abstract class. The effect of this change is that no matter which
entryEmbeddedAssemblies::binary_search()
ends up selecting it will
always return the same managed type name for all aliased Java types.
The TypeMapGenerator.HandleDebugDuplicates()
method from a017561
implicitly preferred the first TypeDefinition encountered for a given
Java type. When the *Invoker
came first, it attempted to treat it
as a duplicate, but wound up removing it entirely:
oldEntry.TypeDefinition = td;
oldEntry.ManagedName = GetManagedTypeName (td);
oldEntry.TypeDefinition
would refer to IExoPlayerInvoker
, and
setting it to td
would instead set it to IExoPlayer
.IExoPlayerInvoker
is lost.
Fix this by updating HandleDebugDuplicates()
to be clearer about
intent:
// oldEntry == typeof(IExoPlayerInvoker)
// td == typeof(IExoPlayer)
if ((td.IsAbstract || td.IsInterface) &&
!oldEntry.TypeDefinition.IsAbstract &&
!oldEntry.TypeDefinition.IsInterface &&
td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) {
// We found the `Invoker` type *before* the declared type
// Fix things up so the abstract type is first, and the `Invoker` is considered a duplicate.
duplicates.Insert (0, entry);
oldEntry.SkipInJavaToManaged = false;
}
Now, when the "new" TypeDefinition is:
- An abstract class or interface type, and
- The original entry is neither an abstract class nor interface, &
- The old entry can be assigned to the new entry,
then we assume that this is a *Invoker
scenario, ensure things are
consistent with the "expected" { IExoPlayer, IExoPlayerInvoker } order:
- Inserting the new TypeDefinition first in
duplicates
.
This ensures expected ordering. - Setting
oldEntry.SkipInJavaToManaged
tofalse
.
This ensures thatIExoPlayerInvoker
is emitted in the typemaps.
WORKAROUND: Building the app in Release configuration avoids
the crash. (Then cry about inner dev loop performance.)
"But what about tests?" Originally, @jonpryor thought thatJavaObjectExtensionsTests.JavaCast_InterfaceCast()
was a sufficient
test. This was wrong, because it didn't test what it thought it did:
IntPtr g;
using (var n = new Java.Lang.Integer (42)) {
g = JNIEnv.NewGlobalRef (n.Handle);
}
// We want a Java.Lang.Object so that we create an IComparableInvoker
// instead of just getting back the original instance.
using (var o = Java.Lang.Object.GetObject<Java.Lang.Object> (g, JniHandleOwnership.TransferGlobalRef)) {
var c = JavaObjectExtensions.JavaCast<Java.Lang.IComparable> (o);
c.Dispose ();
}
The comment is not quite right: while it says we want aJava.Lang.Object
, that's not the runtime type o.GetType()
.
The runtime type o.GetType()
would be Java.Lang.Integer
(!),
because the first thing that TypeManager.CreateInstance()
tries
to do is see if we have an existing type mapping for the runtime type
of the JNI handle. As the runtime type is java.lang.Integer
, we
do have such a binding, and that bound type is implicitly
convertible to Java.Lang.Object
.
Meaning this test never actually tested an interface cast!
So for starters, we need to fix this to something that ensures we
get an unbound type in the base class hierarchy, so that we can
actually test interface checking behavior.
Enter @(AndroidJavaSource)
, ValueProvider.java
, andExample.java
. Example.getValueProvider()
returns an anonymous
inner class that implements ValueProvider
, ensuring that the only
bound type that will work is Java.Lang.Object
. We then can useJavaObjectExtensions.JavaCast<T>()
as originally intended.
We could have used any interface type instead of introducing a newValueProvider
interface, but introducing a new type is the only way
to attempt to reproduce the TypeDefinition ordering issue, by
adding a new partial
class for IValueProviderInvoker
.
Locally, we get the intended "wrong" ordering of Invoker before
abstract type:
% monodis --typedef tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/obj/Debug/net9.0-android/Mono.Android.NET-Tests.dll | grep IValueProvider
5: Net.Dot.Android.Test.IValueProviderInvoker (flist=10, mlist=20, flags=0x100000, extends=0x89)
7: Net.Dot.Android.Test.IValueProvider (flist=13, mlist=37, flags=0x1000a1, extends=0x0)
and the test passes as desired.