From 4e2082a37a9f3af0faa2ee44e8b3187a4f7926b7 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 31 Oct 2025 15:49:12 +0100 Subject: [PATCH 1/5] Fix GCHandle memory leak in NetworkReachability.SetNotification - Free GCHandle when callback is removed via SetNotification(null) - Free GCHandle if SCNetworkReachabilitySetCallback fails - Override Dispose to ensure GCHandle is always freed Fixes #12962 --- src/SystemConfiguration/NetworkReachability.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/SystemConfiguration/NetworkReachability.cs b/src/SystemConfiguration/NetworkReachability.cs index 3fd3a4ad48f5..67d5b84d621f 100644 --- a/src/SystemConfiguration/NetworkReachability.cs +++ b/src/SystemConfiguration/NetworkReachability.cs @@ -523,12 +523,18 @@ public StatusCode SetNotification (Notification? callback) unsafe { rv = SCNetworkReachabilitySetCallback (Handle, &Callback, &ctx) != 0; } + if (!rv) { + gch.Free (); + return StatusCodeError.SCError (); + } } else { if (callback is null) { this.notification = null; unsafe { rv = SCNetworkReachabilitySetCallback (Handle, null, null) != 0; } + if (gch.IsAllocated) + gch.Free (); if (!rv) return StatusCodeError.SCError (); @@ -678,5 +684,12 @@ public bool SetDispatchQueue (DispatchQueue? queue) GC.KeepAlive (queue); return result; } + + protected override void Dispose (bool disposing) + { + if (gch.IsAllocated) + gch.Free (); + base.Dispose (disposing); + } } } From 4b46e0e2b15a48a07ead653a5cd88f3653e77526 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 31 Oct 2025 15:49:59 +0100 Subject: [PATCH 2/5] Add test for NetworkReachability.SetNotification Test verifies that SetNotification can be called multiple times and that clearing the notification works properly. Related to #12962 --- .../NetworkReachabilityTest.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs b/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs index 368f42839548..ba3e3b2206ab 100644 --- a/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs +++ b/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs @@ -142,5 +142,28 @@ public void Schedule () Assert.IsTrue (defaultRouteReachability.Schedule (CFRunLoop.Main, CFRunLoop.ModeDefault), "Schedule"); Assert.IsTrue (defaultRouteReachability.Unschedule (CFRunLoop.Main, CFRunLoop.ModeDefault), "Unschedule"); } + + [Test] + public void SetNotification () + { + var ip = new IPAddress (0); + using var reachability = new NetworkReachability (ip); + + // Test setting a notification + var statusCode = reachability.SetNotification ((flags) => { + }); + Assert.AreEqual (StatusCode.OK, statusCode, "SetNotification should succeed"); + + // Test clearing the notification (this should free the GCHandle) + statusCode = reachability.SetNotification (null); + Assert.AreEqual (StatusCode.OK, statusCode, "SetNotification(null) should succeed"); + + // Test setting notification again after clearing + statusCode = reachability.SetNotification ((flags) => { + }); + Assert.AreEqual (StatusCode.OK, statusCode, "SetNotification should succeed again"); + + // Test that disposing also works (should free the GCHandle in Dispose) + } } } From f964894a945b1fe0e92492e9149f15f28a0cf9fd Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 31 Oct 2025 16:48:42 +0100 Subject: [PATCH 3/5] Improve SetNotification test to verify GCHandle is properly freed Add SetNotification_GCHandleFreed test that: - Creates 10 NetworkReachability instances on a background thread - Sets notifications to allocate GCHandles - Disposes the instances to free the GCHandles - Waits for background thread on main thread - Forces garbage collection - Asserts that at least one instance was collected by GC This test verifies that the GCHandle leak fix works correctly and objects can be garbage collected after disposal. --- .../NetworkReachabilityTest.cs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs b/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs index ba3e3b2206ab..a652713ec43c 100644 --- a/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs +++ b/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs @@ -12,6 +12,8 @@ #endif using SystemConfiguration; using System.Net; +using System.Threading; +using System.Threading.Tasks; namespace MonoTouchFixtures.SystemConfiguration { @@ -165,5 +167,48 @@ public void SetNotification () // Test that disposing also works (should free the GCHandle in Dispose) } + + [Test] + public void SetNotification_GCHandleFreed () + { + // Create weak references to track GC collection + var weakRefs = new WeakReference [10]; + + // Create NetworkReachability instances on a background thread + var thread = new Thread (() => { + for (int i = 0; i < 10; i++) { + var ip = new IPAddress (0); + var reachability = new NetworkReachability (ip); + + // Set a notification to allocate the GCHandle + reachability.SetNotification ((flags) => { + }); + + // Store weak reference to track if object is collected + weakRefs [i] = new WeakReference (reachability); + + // Dispose to ensure GCHandle is freed + reachability.Dispose (); + } + }); + + thread.Start (); + thread.Join (); + + // Force garbage collection + GC.Collect (); + GC.WaitForPendingFinalizers (); + GC.Collect (); + + // Assert that at least one NetworkReachability instance has been collected + var collectedCount = 0; + for (int i = 0; i < weakRefs.Length; i++) { + if (!weakRefs [i].IsAlive) { + collectedCount++; + } + } + + Assert.IsTrue (collectedCount > 0, $"Expected at least one NetworkReachability instance to be collected, but {collectedCount} were collected"); + } } } From 5f35b2492d7136ab3411c1ce399ea48f7c997f36 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 31 Oct 2025 17:01:53 +0100 Subject: [PATCH 4/5] Add test to verify GCHandle is freed with SetNotification(null) Add SetNotification_GCHandleFreedWithNull test that: - Creates 10 NetworkReachability instances on a background thread - Sets notifications to allocate GCHandles - Calls SetNotification(null) to clear and free the GCHandles - Waits for background thread on main thread - Forces garbage collection - Asserts that at least one instance was collected by GC This test specifically validates that calling SetNotification(null) properly frees the GCHandle, allowing objects to be garbage collected. --- .../NetworkReachabilityTest.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs b/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs index a652713ec43c..e4da98fe4040 100644 --- a/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs +++ b/tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs @@ -210,5 +210,48 @@ public void SetNotification_GCHandleFreed () Assert.IsTrue (collectedCount > 0, $"Expected at least one NetworkReachability instance to be collected, but {collectedCount} were collected"); } + + [Test] + public void SetNotification_GCHandleFreedWithNull () + { + // Create weak references to track GC collection + var weakRefs = new WeakReference [10]; + + // Create NetworkReachability instances on a background thread + var thread = new Thread (() => { + for (int i = 0; i < 10; i++) { + var ip = new IPAddress (0); + var reachability = new NetworkReachability (ip); + + // Set a notification to allocate the GCHandle + reachability.SetNotification ((flags) => { + }); + + // Clear notification to free the GCHandle + reachability.SetNotification (null); + + // Store weak reference to track if object is collected + weakRefs [i] = new WeakReference (reachability); + } + }); + + thread.Start (); + thread.Join (); + + // Force garbage collection + GC.Collect (); + GC.WaitForPendingFinalizers (); + GC.Collect (); + + // Assert that at least one NetworkReachability instance has been collected + var collectedCount = 0; + for (int i = 0; i < weakRefs.Length; i++) { + if (!weakRefs [i].IsAlive) { + collectedCount++; + } + } + + Assert.IsTrue (collectedCount > 0, $"Expected at least one NetworkReachability instance to be collected, but {collectedCount} were collected"); + } } } From 85b600d7d623b0f3e6a172caabd0e5d2766fa543 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Mon, 3 Nov 2025 11:02:31 +0100 Subject: [PATCH 5/5] Update tests. --- tests/cecil-tests/Documentation.KnownFailures.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cecil-tests/Documentation.KnownFailures.txt b/tests/cecil-tests/Documentation.KnownFailures.txt index 97f6b0bd7cf5..4ea9a35e62de 100644 --- a/tests/cecil-tests/Documentation.KnownFailures.txt +++ b/tests/cecil-tests/Documentation.KnownFailures.txt @@ -16350,6 +16350,7 @@ M:StoreKit.SKStoreProductViewController.LoadProduct(StoreKit.StoreProductParamet M:StoreKit.SKStoreProductViewController.LoadProductAsync(Foundation.NSDictionary,StoreKit.SKAdImpression) M:StoreKit.SKStoreProductViewController.LoadProductAsync(StoreKit.StoreProductParameters,StoreKit.SKAdImpression) M:StoreKit.SKStoreProductViewController.remove_Finished(System.EventHandler) +M:SystemConfiguration.NetworkReachability.Dispose(System.Boolean) M:ThreadNetwork.THClient.CheckPreferredNetworkAsync(Foundation.NSData) M:ThreadNetwork.THClient.DeleteCredentialsForBorderAgentAsync(Foundation.NSData) M:ThreadNetwork.THClient.IsPreferredNetworkAvailableAsync