From 5721a0f0b2d4f4a20f2eb0b51cf4e0ba7e7d76e8 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 28 Apr 2026 09:19:42 -0500 Subject: [PATCH 1/2] Fix JavaList.Remove() always-false guard: && -> || The guard condition if (i < 0 && i >= Count) was always false because a value cannot simultaneously be negative and >= Count (which is non-negative). This caused Remove() to call RemoveAt(-1) when an item wasn't found, throwing an exception instead of silently no-op-ing as IList.Remove requires. Fix: change && to || so the guard correctly returns early when IndexOf returns -1 (item not found) or when a race condition shrinks the list between IndexOf and RemoveAt. Add a test to verify Remove() with a non-existent item does not throw. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Mono.Android/Android.Runtime/JavaList.cs | 2 +- .../Mono.Android-Tests/Java.Interop/JavaListTest.cs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Mono.Android/Android.Runtime/JavaList.cs b/src/Mono.Android/Android.Runtime/JavaList.cs index 6d12575963c..a38bbac9e81 100644 --- a/src/Mono.Android/Android.Runtime/JavaList.cs +++ b/src/Mono.Android/Android.Runtime/JavaList.cs @@ -392,7 +392,7 @@ public unsafe void Insert (int index, object? item) public void Remove (object? item) { int i = IndexOf (item); - if (i < 0 && i >= Count) + if (i < 0 || i >= Count) return; RemoveAt (i); } diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaListTest.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaListTest.cs index 3ac3c8220f9..5b096ad8cbc 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaListTest.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaListTest.cs @@ -130,6 +130,14 @@ public void RemoveAt () Assert.AreEqual ("foo", list [1]); } + [Test] + public void RemoveNonExistentItemDoesNotThrow () + { + list.Add ("foo"); + Assert.DoesNotThrow (() => list.Remove ("bar")); + Assert.AreEqual (1, list.Count); + } + [Test] public void Set () { From bbd9896d56cf96e996c5d4ac142264c684b30556 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 28 Apr 2026 16:04:14 -0500 Subject: [PATCH 2/2] Fix remaining && -> || in JavaList.Remove() overloads The original PR only fixed JavaList.Remove(object?), but the same bug exists in JavaList.Remove(Java.Lang.Object?) and JavaList.Remove(T). The test calls list.Remove("bar") which resolves to the Java.Lang.Object? overload via the implicit string->Java.Lang.Object conversion operator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Mono.Android/Android.Runtime/JavaList.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/JavaList.cs b/src/Mono.Android/Android.Runtime/JavaList.cs index a38bbac9e81..5dd329e71cb 100644 --- a/src/Mono.Android/Android.Runtime/JavaList.cs +++ b/src/Mono.Android/Android.Runtime/JavaList.cs @@ -627,7 +627,7 @@ public virtual bool IsEmpty { public virtual bool Remove (Java.Lang.Object? item) { int i = IndexOf (item); - if (i < 0 && i >= Count) + if (i < 0 || i >= Count) return false; RemoveAt (i); return true; @@ -964,7 +964,7 @@ public unsafe void Insert (int index, T item) public bool Remove (T item) { int i = IndexOf (item); - if (i < 0 && i >= Count) + if (i < 0 || i >= Count) return false; RemoveAt (i); return true;