Skip to content

[Mono.Android] fix "replaceable" objects in ManagedValueManager - #10107

Merged
jonpryor merged 1 commit into
mainfrom
dev/peppers/ManagedValueManagerPeekPeer
May 6, 2025
Merged

[Mono.Android] fix "replaceable" objects in ManagedValueManager#10107
jonpryor merged 1 commit into
mainfrom
dev/peppers/ManagedValueManagerPeekPeer

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

The following test is failing on NativeAOT as well as any case we'd use ManagedValueManager:

[Test]
public void JnienvCreateInstance_RegistersMultipleInstances ()
{
using (var adapter = new CreateInstance_OverrideAbsListView_Adapter (Application.Context)) {
var intermediate = CreateInstance_OverrideAbsListView_Adapter.Intermediate;
var registered = Java.Lang.Object.GetObject<CreateInstance_OverrideAbsListView_Adapter>(adapter.Handle, JniHandleOwnership.DoNotTransfer);
Assert.AreNotSame (adapter, intermediate); // Passes
Assert.AreSame (adapter, registered); // Fails!
}
}

With the assertion:

Expected: same as <com.xamarin.android.runtimetests.CreateInstance_OverrideAbsListView_Adapter{cbd0e5a V.ED.VC.. ......I. 0,0-0,0}>
But was: <com.xamarin.android.runtimetests.CreateInstance_OverrideAbsListView_Adapter{cbd0e5a V.ED.VC.. ......I. 0,0-0,0}>

The second assertion fails because registered is the same instance as intermediate. In this example, this is a code path where intermediate should be "replaced" with adapter.

After lots of debugging, I found the problem are these lines in the ManagedValueManager.AddPeer() method:

var o = PeekPeer (value.PeerReference);
if (o != null)
return;

If we PeekPeer() in the middle of AddPeer() and a type is "replaceable", it would find an instance and not replace it! I did not find equivalent code in AndroidValueManager.AddPeer(), which is what is used in Mono & production today.

This was also addressed in Java.Interop's ManagedValueManager here:

This also solves a test failure in Java.Interop-Tests:

Mono.Android.NET_Tests, Java.InteropTests.InvokeVirtualFromConstructorTests.CreateManagedInstanceFirst_WithNewObject / Release
Error message
Expected t and registered to be the same instance; t=20dfddd, registered=325d330
Expected: same as net.dot.jni.test.CallVirtualFromConstructorDerived@564f0f8
But was: net.dot.jni.test.CallVirtualFromConstructorDerived@564f0f8
Stack trace
at Java.InteropTests.InvokeVirtualFromConstructorTests.CreateManagedInstanceFirst_WithNewObject()
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack, Void**, ObjectHandleOnStack, BOOL, ObjectHandleOnStack)
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack, Void**, ObjectHandleOnStack, BOOL, ObjectHandleOnStack)
at System.RuntimeMethodHandle.InvokeMethod(Object, Void**, Signature, Boolean)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object, BindingFlags)

Loading
Sign up for freeto 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.

2 participants

@jonathanpeppers@jonpryor