From 0168d27cd7e0e3a703236ab02ec5c3f74f5c0f51 Mon Sep 17 00:00:00 2001
From: Jonathan Peppers <jonathan.peppers@microsoft.com>
Date: Thu, 3 Apr 2025 11:36:29 -0500
Subject: [PATCH] [Mono.Android] fix "replaceable" objects in
 `ManagedValueManager`

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:

* https://github.com/dotnet/java-interop/commit/d3d3a1bf8200cbcc545ac438c47abcd158b55a1e

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)
---
 .../Microsoft.Android.Runtime/ManagedValueManager.cs           | 3 ---
 .../Mono.Android-Tests/Java.Lang/ObjectTest.cs                 | 2 --
 2 files changed, 5 deletions(-)

diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs
index c9fb18464cb..c58cfbab431 100644
--- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs
+++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs
@@ -67,9 +67,6 @@ public override void AddPeer (IJavaPeerable value)
 		var r = value.PeerReference;
 		if (!r.IsValid)
 			throw new ObjectDisposedException (value.GetType ().FullName);
-		var o = PeekPeer (value.PeerReference);
-		if (o != null)
-			return;
 
 		if (r.Type != JniObjectReferenceType.Global) {
 			value.SetPeerReference (r.NewGlobalRef ());
diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs
index 875641156d2..e6a051b170a 100644
--- a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs
+++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs
@@ -66,8 +66,6 @@ static MethodInfo MakeGenericMethod (MethodInfo method, Type type) =>
 		}
 
 		[Test]
-		[Category ("CoreCLRIgnore")] //TODO: https://github.com/dotnet/android/issues/10069
-		[Category ("NativeAOTIgnore")] //TODO: https://github.com/dotnet/android/issues/10079
 		public void JnienvCreateInstance_RegistersMultipleInstances ()
 		{
 			using (var adapter = new CreateInstance_OverrideAbsListView_Adapter (Application.Context)) {