-
Notifications
You must be signed in to change notification settings - Fork 532
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
[Media3] Attempting to create ExoPlayer instance crashes #9535
Comments
I have the exact same issue. I've tried many variations, including a clean MAUI project running .NET 9 (full version, not pre-release). |
Repro Exception
|
Here is an example of how to use media 3 with Maui. CommunityToolkit/Maui#2076
Above is how you create an new instance of the player |
@ne0rrmatrix Have you tested this with .NET 9? I cannot get it working, just get the exception. |
I have tested against dotnet 8 over the last few months. I am currently unable to test using either dotnet 8 or 9 and can't provide any feedback. After updating to dotnet 9 I cannot build anything. Once I get my mac's and pc's working with new dotnet I will see if any changes are needed. |
Let me create repro sample for both |
Repro: dotnet build net8.0/media3/media3.csproj -t:run -f:net8.0-android OK! dotnet build net9.0/media3/media3.csproj -t:run -f:net9.0-android Crashes!!! |
Sample builds fine in Debug or Release mode when using current 9.0 version of dotnet from VS 2022 install. edit: Sample build fine but does crash when app loads. |
Yes. Builds fine, but crashes during running. |
I believe that this was broken by: 35f41dc What I don't understand is why this wasn't caught by existing unit tests, e.g.
|
Does your app crash during running [loading]? Just to be certain |
App crashes after launching activity. It also only crashes when exoplayer build() instruction is executed |
To be specific it crashes only when executing
|
Thanks a lot. Did you use repro sample provided in this issue? Just asking, so other can use it for debugging. thx |
Yes I tried the sample app. It has exact same behavior. I also tested by modifying it like above to try and test if |
Moving this to |
The answer is that before we try to do anything with android/src/Mono.Android/Java.Interop/TypeManager.cs Lines 246 to 264 in 4559426
Because the runtime type is (The test wasn't testing what I thought it was testing!) |
For me, media3.zip works on .NET 9 when you build in Release configuration. |
I can confirm it works in Release mode in .NET 9. It does not work in Debug mode though. |
…which brings us to the question: What's Happening™? Answer: type maps. (Borderline "duh.") Which differ in structure and content between Debug and Release builds. For reasons I'm still digging into, the Debug config type maps also differ between
It does not contain
though oddly the
Open Questions:
|
Still digging, but this appears to be an ordering issue: duplicate detection appears to want the abstract class/interface declaration to appear before the % monodis --typedef obj/Debug/net9.0-android/android/assets/arm64-v8a/Mono.Android.dll
…
7676: Java.Util.Functions.IIntSupplier (flist=57464, mlist=135763, flags=0x1000a1, extends=0x0)
7677: Java.Util.Functions.IIntSupplierInvoker (flist=57464, mlist=135764, flags=0x100000, extends=0x7d58) This isn't the case with % 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) Note that the type definition for This presumably happens because there is an in-tree (non-generated) (Aside: why is all this generated code checked in?) The cause of the bug is somewhere within
to
Note in particular the last two lines, which come from
With |
The bug appears to be within android/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs Lines 301 to 317 in 78f8863
@@ -309,8 +335,11 @@ namespace Xamarin.Android.Tasks
TypeMapDebugEntry oldEntry = duplicates[0];
if (td.IsAbstract || td.IsInterface || oldEntry.TypeDefinition.IsAbstract || oldEntry.TypeDefinition.IsInterface) {
if (td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) {
+ log.LogDebugMessage ($"# jonp: !!! {td.FullName} IsAssignableFrom {oldEntry.TypeDefinition.FullName}");
+ log.LogDebugMessage ($"# jonp: !!! entry={oldEntry}");
oldEntry.TypeDefinition = td;
oldEntry.ManagedName = GetManagedTypeName (td);
+ log.LogDebugMessage ($"# jonp: !!! updated entry={oldEntry}");
}
}
} prints the following log messages:
Within the repro, there are two other types that trigger the above message::
Unsurprisingly, those also have |
@grendello: what's the rationale for this code? android/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs Lines 309 to 315 in 78f8863
If I remove that code -- such that the |
Commit a017561 appears to have added the
|
It might not crash, but it's also not right either. From
The first entry should be |
@grendello: my current attempted fix is: diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs
index a6dbce277..91b552af3 100644
--- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs
+++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs
@@ -77,6 +77,11 @@ namespace Xamarin.Android.Tasks
public TypeDefinition TypeDefinition;
public bool SkipInJavaToManaged;
public TypeMapDebugEntry DuplicateForJavaToManaged;
+
+ public override string ToString ()
+ {
+ return $"TypeMapDebugEntry{{JavaName={JavaName}, ManagedName={ManagedName}, JavaIndex={JavaIndex}, ManagedIndex={ManagedIndex}, SkipInJavaToManaged={SkipInJavaToManaged}, DuplicateForJavaToManaged={DuplicateForJavaToManaged}}}";
+ }
}
// Widths include the terminating nul character but not the padding!
@@ -170,6 +175,7 @@ namespace Xamarin.Android.Tasks
bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, string outputDirectory, bool generateNativeAssembly)
{
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebug: generateNativeAssembly: {generateNativeAssembly}");
if (generateNativeAssembly) {
return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, outputDirectory);
}
@@ -192,11 +198,13 @@ namespace Xamarin.Android.Tasks
var javaDuplicates = new Dictionary<string, List<TypeMapDebugEntry>> (StringComparer.Ordinal);
foreach (TypeDefinition td in state.AllJavaTypes) {
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugFiles: td={td.FullName}");
UpdateApplicationConfig (td);
string moduleName = td.Module.Assembly.Name.Name;
ModuleDebugData module;
if (!modules.TryGetValue (moduleName, out module)) {
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugFiles: adding new module: {moduleName}");
string outputFileName = $"{moduleName}{TypemapExtension}";
module = new ModuleDebugData {
EntryCount = 0,
@@ -219,6 +227,8 @@ namespace Xamarin.Android.Tasks
}
TypeMapDebugEntry entry = GetDebugEntry (td, state.TypeCache);
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugFiles: entry={{JavaName={entry.JavaName}, ManagedName={entry.ManagedName}, SkipInJavaToManaged={entry.SkipInJavaToManaged}}}");
+
HandleDebugDuplicates (javaDuplicates, entry, td, state.TypeCache);
if (entry.JavaName.Length > module.JavaNameWidth)
module.JavaNameWidth = (uint)entry.JavaName.Length + 1;
@@ -230,6 +240,13 @@ namespace Xamarin.Android.Tasks
module.ManagedToJavaMap.Add (entry);
}
SyncDebugDuplicates (javaDuplicates);
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugFiles: javaDuplicates:");
+ foreach (var e in javaDuplicates) {
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugFiles: {e.Key}:");
+ foreach (var entry in e.Value) {
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugFiles: {entry}");
+ }
+ }
foreach (ModuleDebugData module in modules.Values) {
PrepareDebugMaps (module);
@@ -251,6 +268,7 @@ namespace Xamarin.Android.Tasks
bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, string outputDirectory)
{
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugNativeAssembly");
var javaToManaged = new List<TypeMapDebugEntry> ();
var managedToJava = new List<TypeMapDebugEntry> ();
@@ -259,12 +277,20 @@ namespace Xamarin.Android.Tasks
UpdateApplicationConfig (td);
TypeMapDebugEntry entry = GetDebugEntry (td, state.TypeCache);
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugNativeAssembly: entry={{JavaName={entry.JavaName}, ManagedName={entry.ManagedName}, SkipInJavaToManaged={entry.SkipInJavaToManaged}}}");
HandleDebugDuplicates (javaDuplicates, entry, td, state.TypeCache);
javaToManaged.Add (entry);
managedToJava.Add (entry);
}
SyncDebugDuplicates (javaDuplicates);
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugNativeAssembly: javaDuplicates:");
+ foreach (var e in javaDuplicates) {
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugNativeAssembly: Java type: {e.Key}:");
+ foreach (var entry in e.Value) {
+ log.LogDebugMessage ($"# jonp: TypeMapGenerator.GenerateDebugNativeAssembly: {entry}");
+ }
+ }
var data = new ModuleDebugData {
EntryCount = (uint)javaToManaged.Count,
@@ -305,14 +331,30 @@ namespace Xamarin.Android.Tasks
if (!javaDuplicates.TryGetValue (entry.JavaName, out duplicates)) {
javaDuplicates.Add (entry.JavaName, new List<TypeMapDebugEntry> { entry });
} else {
- duplicates.Add (entry);
TypeMapDebugEntry oldEntry = duplicates[0];
+ 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;
+ } else {
+ // ¯\_(ツ)_/¯
+ duplicates.Add (entry);
+ }
+ #if false
if (td.IsAbstract || td.IsInterface || oldEntry.TypeDefinition.IsAbstract || oldEntry.TypeDefinition.IsInterface) {
if (td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) {
+ log.LogDebugMessage ($"# jonp: !!! {td.FullName} IsAssignableFrom {oldEntry.TypeDefinition.FullName}");
+ log.LogDebugMessage ($"# jonp: !!! entry={oldEntry}");
oldEntry.TypeDefinition = td;
oldEntry.ManagedName = GetManagedTypeName (td);
+ log.LogDebugMessage ($"# jonp: !!! updated entry={oldEntry}");
}
}
+ #endif
}
}
The good news is that this appears to work. The bad news is that despite working, the resulting
The The But this appears to be what .NET 9 GA does with |
Fixes: #9535 Context: a017561 Context: 35f41dc Create a new .NET for Android project (`dotnet new android`), add a `@(PackageReference)` to [Xamarin.AndroidX.Media3.ExoPlayer][0], 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][1], 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 in `Java.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 > entry `EmbeddedAssemblies::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: 1. An abstract class or interface type, and 2. The original entry is *neither* an abstract class nor interface, & 3. 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: 1. Inserting the new TypeDefinition *first* in `duplicates`. This ensures expected ordering. 2. Setting `oldEntry.SkipInJavaToManaged` to `false`. 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. [0]: https://www.nuget.org/packages/Xamarin.AndroidX.Media3.ExoPlayer/1.4.1 [1]: https://github.com/dotnet/android-libraries/blob/3c21f4643c5dfe74e64fec54e3679baf4dc8d067/source/androidx.media3/media3-exoplayer/Additions/AndroidX.Media3.ExoPlayer.IExoPlayer.cs#L10
Android framework version
net9.0-android
Affected platform version
.NET 9
Description
The code throw an exception when trying to create an instance of
ExoPlayer
Steps to Reproduce
Example project:
media3.zip
Given other developers seem to already be using the media3 bindings, I wonder if I am doing anything wrong...?
Did you find any workaround?
No response
Relevant log output
No response
The text was updated successfully, but these errors were encountered: