Comparing 35.0.7...35.0.24 · 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 for
IExoPlayerInvoker
.
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 the
Invoker
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 that
IExoPlayerInvoker
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 that
JavaObjectExtensionsTests.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 a
Java.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
, and
Example.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 use
JavaObjectExtensions.JavaCast<T>()
as originally intended.
We could have used any interface type instead of introducing a new
ValueProvider
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.