Skip to content

Fix GCHandle leak in NetworkReachability.SetNotification#24146

Closed
Copilot wants to merge 2 commits intomainfrom
copilot/fix-gchandle-leak-notification
Closed

Fix GCHandle leak in NetworkReachability.SetNotification#24146
Copilot wants to merge 2 commits intomainfrom
copilot/fix-gchandle-leak-notification

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 31, 2025

NetworkReachability.SetNotification allocates a GCHandle to keep the object alive during native callbacks but never frees it, causing a memory leak.

Changes

  • Track allocation state: Added gchAllocated bool field
  • Free on removal: Free GCHandle when SetNotification(null) is called
  • Free on dispose: Override Dispose to free handle if object is disposed without explicitly removing notification
  • Add tests: Added 4 tests covering set/remove, dispose, multiple cycles, and callback updates

Example

// Before: GCHandle leaks when notification is removed or object is disposed
using var nr = new NetworkReachability(IPAddress.Loopback);
nr.SetNotification(flags => Console.WriteLine(flags));
nr.SetNotification(null);  // GCHandle leaked here
// GCHandle also leaked on dispose

// After: GCHandle properly freed in both cases
Original prompt

This section details on the original issue you should resolve

<issue_title>NetworkReachability.SetNotification leaks a GCHandle</issue_title>
<issue_description>The GCHandle allocated here:

https://github.com/xamarin/xamarin-macios/blob/2930c052b32318af63c1f3b90f6447f6907548b8/src/SystemConfiguration/NetworkReachability.cs#L252

is never freed.

Also we don't have tests for NetworkReachability.SetNotification.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: rolfbjarne <249268+rolfbjarne@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix GCHandle leak in NetworkReachability.SetNotification Fix GCHandle leak in NetworkReachability.SetNotification Oct 31, 2025
Copilot AI requested a review from rolfbjarne October 31, 2025 14:25
@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [CI Build #4834577] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 48345778f6685c49978be336dc77c74cde1ecc19 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [PR Build #4834577] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 48345778f6685c49978be336dc77c74cde1ecc19 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)

✅ API diff vs stable

NET (empty diffs)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 48345778f6685c49978be336dc77c74cde1ecc19 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

🔥 [CI Build #4834577] Build failed (Build macOS tests) 🔥

Build failed for the job 'Build macOS tests' (with job status 'Failed')

Pipeline on Agent
Hash: 48345778f6685c49978be336dc77c74cde1ecc19 [PR build]

@rolfbjarne
Copy link
Copy Markdown
Member

Closing in favor of #24147.

@rolfbjarne rolfbjarne closed this Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NetworkReachability.SetNotification leaks a GCHandle

3 participants