[Foundation] Improve potential reentrancy problems in NSUrlSessionHandler. by rolfbjarne · Pull Request #25183 · dotnet/macios

rolfbjarne · 2026-04-17T17:21:09Z

C# locks are re-entrant, so try to avoid re-entrancy issues by:

In RemoveInflightData: fetch and remove the inflight data in a single
Dictionary.Remove call. This is also faster than doing two dictionary
lookups.
In Dispose: collect all tasks to cancel and dispose first (inside the
lock), and then we cancel and dispose after the lock. This hopefully fixes
Crash in networking stack #20345 (this solution was tested by a
customer and apparently worked).

Might fix #20345.

NET10_0_OR_GREATER is always set now, so remove other code paths.

…dler. C# locks are re-entrant, so try to avoid re-entrancy issues by: * In `RemoveInflightData`: fetch and remove the inflight data in a single `Dictionary.Remove` call. This is also faster than doing two dictionary lookups. * In `Dispose`: collect all tasks to cancel and dispose first (inside the lock), and then we cancel and dispose after the lock. This hopefully fixes #20345 (this solution was tested by a customer and apparently worked). Might fix #20345.

Copilot

Pull request overview

This PR updates NSUrlSessionHandler to reduce re-entrancy hazards during inflight request cleanup and handler disposal, aiming to improve stability in long-running/high-volume networking scenarios (potentially addressing #20345).

Changes:

Simplifies inflight request removal by using Dictionary.Remove(key, out value) to fetch+remove in one operation.
Refactors Dispose(bool) to snapshot inflight tasks under lock, clear the dictionary, then cancel/dispose tasks outside the lock to avoid re-entrant lock usage.

Copilot · 2026-04-17T17:25:23Z

+			var tasks = new List<NSUrlSessionTask> ();
 			lock (inflightRequestsLock) {
-				foreach (var pair in inflightRequests) {
-					pair.Key?.Cancel ();
-					pair.Key?.Dispose ();
-				}
-
+				tasks.AddRange (inflightRequests.Keys);
 				inflightRequests.Clear ();
 			}
+			foreach (var task in tasks) {
+				task.Cancel ();
+				task.Dispose ();
+			}


In Dispose, inflightRequests.Clear () happens before canceling tasks, so any subsequent DidCompleteWithError / DidReceiveData delegate callbacks won’t be able to retrieve the corresponding InflightData from inflightRequests. Since InflightData.CompletionSource is only completed when GetInflightData finds an entry, this can leave callers awaiting SendAsync permanently blocked if the handler is disposed while requests are in-flight. Consider capturing the InflightData instances (not just the tasks) while holding the lock and explicitly completing/canceling their CompletionSource (and canceling InflightData.CancellationTokenSource / faulting the stream) as part of Dispose, instead of relying on delegate callbacks after the dictionary is cleared.

…-safer-dispose-20345

vs-mobiletools-engineering-service2 · 2026-04-20T16:28:44Z

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

Pipeline on Agent
Hash: 9764620da39250b2d45717679a6259a2ba2079d8 [PR build]

vs-mobiletools-engineering-service2 · 2026-04-20T16:30:49Z

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

Pipeline on Agent
Hash: 9764620da39250b2d45717679a6259a2ba2079d8 [PR build]

vs-mobiletools-engineering-service2 · 2026-04-20T16:43:26Z

✅ API diff for current PR / commit

NET (empty diffs)

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

✅ API diff vs stable

NET (empty diffs)

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

ℹ️ Generator diff

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

Pipeline on Agent
Hash: 9764620da39250b2d45717679a6259a2ba2079d8 [PR build]

vs-mobiletools-engineering-service2 · 2026-04-20T16:55:47Z

✅ [CI Build #9764620] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 9764620da39250b2d45717679a6259a2ba2079d8 [PR build]

vs-mobiletools-engineering-service2 · 2026-04-21T07:30:01Z

🔥 [CI Build #9764620] Test results 🔥

Test results

❌ Tests failed on VSTS: test results

0 tests crashed, 1 tests failed, 155 tests passed.

Failures

❌ Tests on macOS Monterey (12) tests

1 tests failed, 4 tests passed.

Failed tests

monotouch-test: Failed (exit code 2)
- TlsDefaults : Connection is ready

Html Report (VSDrops) Download

Successes

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 6 tests passed. Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 15 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 11 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Ventura (13): All 5 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

✅ Linux build succeeded

Pipeline on Agent
Hash: 9764620da39250b2d45717679a6259a2ba2079d8 [PR build]

rolfbjarne added 2 commits April 17, 2026 16:48

[Foundation] Clean up pre-net10.0 code from NSUrlSessionHandler.

b3223c4

NET10_0_OR_GREATER is always set now, so remove other code paths.

rolfbjarne requested a review from Copilot April 17, 2026 17:21

Copilot started reviewing on behalf of rolfbjarne April 17, 2026 17:21 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rolfbjarne changed the base branch from dev/rolf/nsurlsession-cleanup to main April 20, 2026 16:03

Merge remote-tracking branch 'origin/main' into dev/rolf/nsurlsession…

9764620

…-safer-dispose-20345

rolfbjarne marked this pull request as ready for review April 20, 2026 16:06

This comment has been minimized.

Sign in to view

dalexsoto approved these changes Apr 22, 2026

View reviewed changes

Merge branch 'main' into dev/rolf/nsurlsession-safer-dispose-20345

af39f1f

This comment has been minimized.

Sign in to view

rolfbjarne merged commit c11dc4a into main Apr 23, 2026
10 checks passed

Conversation

rolfbjarne commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Apr 20, 2026

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

Uh oh!

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Apr 20, 2026

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

Uh oh!

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Apr 20, 2026

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

Uh oh!

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Apr 20, 2026

✅ [CI Build #9764620] Build passed (Build macOS tests) ✅

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Apr 21, 2026

🔥 [CI Build #9764620] Test results 🔥

Test results

Failures

❌ Tests on macOS Monterey (12) tests

Failed tests

Successes

macOS tests

Linux Build Verification

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants