From b3223c442069169fe016c9d290cc3d2779aff451 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 17 Apr 2026 10:32:29 +0200 Subject: [PATCH 1/2] [Foundation] Clean up pre-net10.0 code from NSUrlSessionHandler. NET10_0_OR_GREATER is always set now, so remove other code paths. --- src/Foundation/NSUrlSessionHandler.cs | 115 -------------------------- 1 file changed, 115 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 610af3a33997..f655aa3efd58 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -124,10 +124,6 @@ public partial class NSUrlSessionHandler : HttpMessageHandler { readonly Dictionary inflightRequests; readonly object inflightRequestsLock = new object (); readonly NSUrlSessionConfiguration.SessionConfigurationType sessionType; -#if !MONOMAC && !NET8_0 && !NET10_0_OR_GREATER - NSObject? notificationToken; // needed to make sure we do not hang if not using a background session - readonly object notificationTokenLock = new object (); // need to make sure that threads do no step on each other with a dispose and a remove inflight data -#endif X509ChainPolicy? policy; static NSUrlSessionConfiguration CreateConfig () @@ -161,89 +157,10 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) allowsCellularAccess = configuration.AllowsCellularAccess; AllowAutoRedirect = true; -#if !NET10_0_OR_GREATER -#pragma warning disable SYSLIB0014 - // SYSLIB0014: 'ServicePointManager' is obsolete: 'WebRequest, HttpWebRequest, ServicePoint, and WebClient are obsolete. Use HttpClient instead. Settings on ServicePointManager no longer affect SslStream or HttpClient.' (https://aka.ms/dotnet-warnings/SYSLIB0014) - // https://github.com/dotnet/macios/issues/20764 - var sp = ServicePointManager.SecurityProtocol; -#pragma warning restore SYSLIB0014 - - // The analyzer has a bug where SupportedOSPlatformGuard attributes don't work correctly (https://github.com/dotnet/roslyn-analyzers/issues/7665#issuecomment-2898275765), so ignore CA1416/CA1422 here - // warning CA1422: This call site is reachable on: 'ios' 12.2 and later, 'maccatalyst' 12.2 and later, 'macOS/OSX' 12.0 and later, 'tvos' 12.2 and later. 'NSUrlSessionConfiguration.[...]' is obsoleted on: 'ios' 13.0 and later (Use '...' instead.), 'maccatalyst' 13.0 and later (Use '...' instead.), 'macOS/OSX' 10.15 and later (Use '...' instead.). - // warning CA1416: This call site is reachable on: 'ios' 12.2 and later, 'maccatalyst' 12.2 and later, 'macOS/OSX' 10.15 and later, 'tvos' 12.2 and later. 'NSUrlSessionConfiguration.[...]' is only supported on: 'ios' 13.0 and later, 'tvos' 13.0 and later -#pragma warning disable CA1416 -#pragma warning disable CA1422 - if (SystemVersion.IsAtLeastXcode11) { - if ((sp & SecurityProtocolType.Ssl3) != 0) { - // no equivalent - } else if ((sp & SecurityProtocolType.Tls) != 0) { - configuration.TlsMinimumSupportedProtocolVersion = TlsProtocolVersion.Tls10; - } else if ((sp & SecurityProtocolType.Tls11) != 0) { - configuration.TlsMinimumSupportedProtocolVersion = TlsProtocolVersion.Tls11; - } else if ((sp & SecurityProtocolType.Tls12) != 0) { - configuration.TlsMinimumSupportedProtocolVersion = TlsProtocolVersion.Tls12; - } else if ((sp & SecurityProtocolType.Tls13) != 0) { - configuration.TlsMinimumSupportedProtocolVersion = TlsProtocolVersion.Tls13; - } - } else { - if ((sp & SecurityProtocolType.Ssl3) != 0) - configuration.TLSMinimumSupportedProtocol = SslProtocol.Ssl_3_0; - else if ((sp & SecurityProtocolType.Tls) != 0) - configuration.TLSMinimumSupportedProtocol = SslProtocol.Tls_1_0; - else if ((sp & SecurityProtocolType.Tls11) != 0) - configuration.TLSMinimumSupportedProtocol = SslProtocol.Tls_1_1; - else if ((sp & SecurityProtocolType.Tls12) != 0) - configuration.TLSMinimumSupportedProtocol = SslProtocol.Tls_1_2; - else if ((sp & SecurityProtocolType.Tls13) != 0) - configuration.TLSMinimumSupportedProtocol = SslProtocol.Tls_1_3; - } -#pragma warning restore CA1422 -#pragma warning restore CA1416 -#endif // NET10_0_OR_GREATER - session = NSUrlSession.FromConfiguration (configuration, (INSUrlSessionDelegate) new NSUrlSessionHandlerDelegate (this), null); inflightRequests = new Dictionary (); } -#if !MONOMAC && !NET8_0 && !NET10_0_OR_GREATER - - void AddNotification () - { - lock (notificationTokenLock) { - if (!bypassBackgroundCheck && sessionType != NSUrlSessionConfiguration.SessionConfigurationType.Background && notificationToken is null) - notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (UIApplication.WillResignActiveNotification, BackgroundNotificationCb); - } // lock - } - - void RemoveNotification () - { - NSObject? localNotificationToken; - lock (notificationTokenLock) { - localNotificationToken = notificationToken; - notificationToken = null; - } - if (localNotificationToken is not null) - NSNotificationCenter.DefaultCenter.RemoveObserver (localNotificationToken); - } - - void BackgroundNotificationCb (NSNotification obj) - { - // the cancelation task of each of the sources will clean the different resources. Each removal is done - // inside a lock, but of course, the .Values collection will not like that because it is modified during the - // iteration. We split the operation in two, get all the diff cancelation sources, then try to cancel each of them - // which will do the correct lock dance. Note that we could be tempted to do a RemoveAll, that will yield the same - // runtime issue, this is dull but safe. - List> sources; - lock (inflightRequestsLock) { // just lock when we iterate - sources = new List> (inflightRequests.Count); - foreach (var r in inflightRequests.Values) { - sources.Add (r.CompletionSource); - } - } - sources.ForEach (source => { source.TrySetCanceled (); }); - } -#endif - /// The maximum amount of content to load into memory when sending content with a request. /// The maximum size of content to load into memory. /// @@ -264,11 +181,6 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) data.CancellationTokenSource.Cancel (); inflightRequests.Remove (task); } -#if !MONOMAC && !NET8_0 && !NET10_0_OR_GREATER - // do we need to be notified? If we have not inflightData, we do not - if (inflightRequests.Count == 0) - RemoveNotification (); -#endif } if (cancel) @@ -283,10 +195,6 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) protected override void Dispose (bool disposing) { lock (inflightRequestsLock) { -#if !MONOMAC && !NET8_0 && !NET10_0_OR_GREATER - // remove the notification if present, method checks against null - RemoveNotification (); -#endif foreach (var pair in inflightRequests) { pair.Key?.Cancel (); pair.Key?.Dispose (); @@ -366,34 +274,15 @@ public NSUrlSessionHandlerTrustOverrideForUrlCallback? TrustOverrideForUrl { trustOverrideForUrl = value; } } -#if !NET8_0 && !NET10_0_OR_GREATER - // we do check if a user does a request and the application goes to the background, but - // in certain cases the user does that on purpose (BeingBackgroundTask) and wants to be able - // to use the network. In those cases, which are few, we want the developer to explicitly - // bypass the check when there are not request in flight - bool bypassBackgroundCheck = true; -#endif #if !XAMCORE_5_0 [EditorBrowsable (EditorBrowsableState.Never)] -#if NET8_0 || NET10_0_OR_GREATER [Obsolete ("This property is ignored.")] -#else - [Obsolete ("This property will be ignored in .NET 10+.")] -#endif public bool BypassBackgroundSessionCheck { get { -#if NET8_0 || NET10_0_OR_GREATER return true; -#else - return bypassBackgroundCheck; -#endif } set { -#if !NET8_0 && !NET10_0_OR_GREATER - EnsureModifiability (); - bypassBackgroundCheck = value; -#endif } } #endif // !XAMCORE_5_0 @@ -567,10 +456,6 @@ protected override async Task SendAsync (HttpRequestMessage var inflightData = new InflightData (request.RequestUri?.AbsoluteUri!, cancellationToken, request); lock (inflightRequestsLock) { -#if !MONOMAC && !NET8_0 && !NET10_0_OR_GREATER - // Add the notification whenever needed - AddNotification (); -#endif inflightRequests.Add (dataTask, inflightData); } From 2b8d57860069d543fd31ff96a07b4333535bc0f5 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 17 Apr 2026 10:34:40 +0200 Subject: [PATCH 2/2] [Foundation] Improve potential reentrancy problems in NSUrlSessionHandler. 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 https://github.com/dotnet/macios/issues/20345 (this solution was tested by a customer and apparently worked). Might fix https://github.com/dotnet/macios/issues/20345. --- src/Foundation/NSUrlSessionHandler.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index f655aa3efd58..7ea4ff34bda6 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -176,10 +176,9 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { lock (inflightRequestsLock) { - if (inflightRequests.TryGetValue (task, out var data)) { + if (inflightRequests.Remove (task, out var data)) { if (cancel) data.CancellationTokenSource.Cancel (); - inflightRequests.Remove (task); } } @@ -194,14 +193,16 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) /// To be added. protected override void Dispose (bool disposing) { + var tasks = new List (); 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 (); + } + session.InvalidateAndCancel (); base.Dispose (disposing); }