From bffc66381770888b4c6215311ab93b0442da8c04 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Wed, 23 Jan 2019 13:08:57 +0100 Subject: [PATCH 1/7] [Foundation] Ensure that we do not block when in the backgorund without a backgorund session. The mono threadpool gets into an unknown state when the application goes into the background. This fix allows the task that are inflight to be canceled when the app goes to the background allowing the application not to hang and letting the developer retyr the request. If a developer needs to work with the app in the background, he should be using a background session, this fix just ensures that we are left in a known state but does not mean that developers should use this kind of sessions. The MessageHandler class is just used in Mac OS X and does not have the idea of the app going to the background, therefore the fix is not needd in that handler. --- src/Foundation/NSUrlSessionHandler.cs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 159c081e1a10..9b388aedf4fc 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -51,6 +51,10 @@ using nuint = System.UInt32; #endif +#if !MONOMAC +using UIKit; +#endif + #if SYSTEM_NET_HTTP namespace System.Net.Http { #else @@ -120,6 +124,9 @@ public partial class NSUrlSessionHandler : HttpMessageHandler readonly NSUrlSession session; readonly Dictionary inflightRequests; readonly object inflightRequestsLock = new object (); +#if !MONOMAC + readonly NSObject notificationToken; // needed to make sure we do not hang if not using a background session +#endif static NSUrlSessionConfiguration CreateConfig () { @@ -142,6 +149,14 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) if (configuration == null) throw new ArgumentNullException (nameof (configuration)); +#if !MONOMAC + // if the configuration has an identifier, we are dealing with a background session, + // therefore, we do not have to listen to the notifications. + if (string.IsNullOrEmpty (configuration.Identifier)) { + notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (UIApplication.WillResignActiveNotification, BackgoundNotificationCb); + } +#endif + AllowAutoRedirect = true; // we cannot do a bitmask but we can set the minimum based on ServicePointManager.SecurityProtocol minimum @@ -159,6 +174,15 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) inflightRequests = new Dictionary (); } + void BackgoundNotificationCb (NSNotification obj) + { + // we do not need to call the lock, we call cancel on the source, that will trigger all the needed code to + // clean the resources and such + foreach (var pair in inflightRequests) { + pair.Value.CompletionSource.TrySetCanceled (); + } + } + public long MaxInputInMemory { get; set; } = long.MaxValue; void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) From f34899f7fd102b0fa7394825144417dfda7464e7 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Wed, 23 Jan 2019 13:44:05 +0100 Subject: [PATCH 2/7] Do not use property since it is not present at that compilation point. --- src/Foundation/NSUrlSessionHandler.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 9b388aedf4fc..2530a81e19b0 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -153,7 +153,8 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) // if the configuration has an identifier, we are dealing with a background session, // therefore, we do not have to listen to the notifications. if (string.IsNullOrEmpty (configuration.Identifier)) { - notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (UIApplication.WillResignActiveNotification, BackgoundNotificationCb); + using (var notification = new NSString ("UIApplicationWillResignActiveNotification")) + notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (notification, BackgoundNotificationCb); } #endif From 05f95c8ddc6472d9a9c65351a986f67afee5b3ec Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Wed, 23 Jan 2019 14:03:06 +0100 Subject: [PATCH 3/7] Clean code a little. --- src/Foundation/NSUrlSessionHandler.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 2530a81e19b0..f1e95a8385ed 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -51,10 +51,6 @@ using nuint = System.UInt32; #endif -#if !MONOMAC -using UIKit; -#endif - #if SYSTEM_NET_HTTP namespace System.Net.Http { #else @@ -175,6 +171,7 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) inflightRequests = new Dictionary (); } +#if !MONOMAC void BackgoundNotificationCb (NSNotification obj) { // we do not need to call the lock, we call cancel on the source, that will trigger all the needed code to @@ -183,6 +180,7 @@ void BackgoundNotificationCb (NSNotification obj) pair.Value.CompletionSource.TrySetCanceled (); } } +#endif public long MaxInputInMemory { get; set; } = long.MaxValue; From 97131a5af981b1ff6ff6b83ff69e9952ae4f536d Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 24 Jan 2019 13:22:08 +0100 Subject: [PATCH 4/7] Address reviews. --- src/Foundation/NSUrlSessionHandler.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index f1e95a8385ed..e1afbfeecac0 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -150,7 +150,7 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) // therefore, we do not have to listen to the notifications. if (string.IsNullOrEmpty (configuration.Identifier)) { using (var notification = new NSString ("UIApplicationWillResignActiveNotification")) - notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (notification, BackgoundNotificationCb); + notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (notification, BackgroundNotificationCb); } #endif @@ -172,12 +172,12 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) } #if !MONOMAC - void BackgoundNotificationCb (NSNotification obj) + void BackgroundNotificationCb (NSNotification obj) { // we do not need to call the lock, we call cancel on the source, that will trigger all the needed code to // clean the resources and such - foreach (var pair in inflightRequests) { - pair.Value.CompletionSource.TrySetCanceled (); + foreach (var r in inflightRequests.Values) { + r.CompletionSource.TrySetCanceled (); } } #endif @@ -197,6 +197,9 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) protected override void Dispose (bool disposing) { +#if !MONOMAC + NSNotificationCenter.DefaultCenter.RemoveObserver (notificationToken); +#endif lock (inflightRequestsLock) { foreach (var pair in inflightRequests) { pair.Key?.Cancel (); @@ -205,7 +208,6 @@ protected override void Dispose (bool disposing) inflightRequests.Clear (); } - base.Dispose (disposing); } From 944bf2716348ac408e3b9439232a03fb80b06d58 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Fri, 25 Jan 2019 12:29:59 +0100 Subject: [PATCH 5/7] Remove the notification if the inflight data is 0. --- src/Foundation/NSUrlSessionHandler.cs | 52 ++++++++++++++++++++------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index e1afbfeecac0..fbbeeaacd89c 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -51,6 +51,10 @@ using nuint = System.UInt32; #endif +#if !MONOMAC && !WATCH +using UIKit; +#endif + #if SYSTEM_NET_HTTP namespace System.Net.Http { #else @@ -120,8 +124,9 @@ public partial class NSUrlSessionHandler : HttpMessageHandler readonly NSUrlSession session; readonly Dictionary inflightRequests; readonly object inflightRequestsLock = new object (); -#if !MONOMAC - readonly NSObject notificationToken; // needed to make sure we do not hang if not using a background session +#if !MONOMAC && !WATCH + readonly bool isBackgroundSession = false; + NSObject notificationToken; // needed to make sure we do not hang if not using a background session #endif static NSUrlSessionConfiguration CreateConfig () @@ -145,13 +150,10 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) if (configuration == null) throw new ArgumentNullException (nameof (configuration)); -#if !MONOMAC +#if !MONOMAC && !WATCH // if the configuration has an identifier, we are dealing with a background session, // therefore, we do not have to listen to the notifications. - if (string.IsNullOrEmpty (configuration.Identifier)) { - using (var notification = new NSString ("UIApplicationWillResignActiveNotification")) - notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (notification, BackgroundNotificationCb); - } + isBackgroundSession = !string.IsNullOrEmpty (configuration.Identifier); #endif AllowAutoRedirect = true; @@ -171,7 +173,21 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) inflightRequests = new Dictionary (); } -#if !MONOMAC +#if !MONOMAC && !WATCH + + void AddNotification () + { + if (!isBackgroundSession && notificationToken == null) + using (var notification = new NSString ("UIApplicationWillResignActiveNotification")) + notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (notification, BackgroundNotificationCb); + } + + void RemoveNotification () + { + if (notificationToken != null) + NSNotificationCenter.DefaultCenter.RemoveObserver (notificationToken); + } + void BackgroundNotificationCb (NSNotification obj) { // we do not need to call the lock, we call cancel on the source, that will trigger all the needed code to @@ -186,8 +202,14 @@ void BackgroundNotificationCb (NSNotification obj) void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { - lock (inflightRequestsLock) + lock (inflightRequestsLock) { inflightRequests.Remove (task); +#if !MONOMAC && !WATCH + // do we need to be notified? If we have not inflightData, we do not + if (inflightRequests.Count == 0) + RemoveNotification (); +#endif + } if (cancel) task?.Cancel (); @@ -197,8 +219,9 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) protected override void Dispose (bool disposing) { -#if !MONOMAC - NSNotificationCenter.DefaultCenter.RemoveObserver (notificationToken); +#if !MONOMAC && !WATCH + // remove the notification if present, method checks against null + RemoveNotification (); #endif lock (inflightRequestsLock) { foreach (var pair in inflightRequests) { @@ -279,7 +302,11 @@ protected override async Task SendAsync (HttpRequestMessage tcs.TrySetCanceled (); }); - lock (inflightRequestsLock) + lock (inflightRequestsLock) { +#if !MONOMAC && !WATCH + // Add the notification whenever needed + AddNotification (); +#endif inflightRequests.Add (dataTask, new InflightData { RequestUrl = request.RequestUri.AbsoluteUri, CompletionSource = tcs, @@ -287,6 +314,7 @@ protected override async Task SendAsync (HttpRequestMessage Stream = new NSUrlSessionDataTaskStream (), Request = request }); + } if (dataTask.State == NSUrlSessionTaskState.Suspended) dataTask.Resume (); From 9bd39d3f1f33818f82bbe8693d5cda59f6455bb3 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Fri, 25 Jan 2019 12:37:11 +0100 Subject: [PATCH 6/7] Ensure the token is set to null when removed. --- src/Foundation/NSUrlSessionHandler.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index fbbeeaacd89c..e926499d3db7 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -184,8 +184,10 @@ void AddNotification () void RemoveNotification () { - if (notificationToken != null) + if (notificationToken != null) { NSNotificationCenter.DefaultCenter.RemoveObserver (notificationToken); + notificationToken = null; + } } void BackgroundNotificationCb (NSNotification obj) From 50c92a2fa85d9d6b9cc0a8338ad79c5957988ee6 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Wed, 6 Feb 2019 14:08:06 +0100 Subject: [PATCH 7/7] Move to use the property rather than a NSString. --- src/Foundation/NSUrlSessionHandler.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index e926499d3db7..8264ded84d85 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -51,7 +51,7 @@ using nuint = System.UInt32; #endif -#if !MONOMAC && !WATCH +#if !MONOMAC using UIKit; #endif @@ -124,7 +124,7 @@ public partial class NSUrlSessionHandler : HttpMessageHandler readonly NSUrlSession session; readonly Dictionary inflightRequests; readonly object inflightRequestsLock = new object (); -#if !MONOMAC && !WATCH +#if !MONOMAC && !MONOTOUCH_WATCH readonly bool isBackgroundSession = false; NSObject notificationToken; // needed to make sure we do not hang if not using a background session #endif @@ -150,7 +150,7 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) if (configuration == null) throw new ArgumentNullException (nameof (configuration)); -#if !MONOMAC && !WATCH +#if !MONOMAC && !MONOTOUCH_WATCH // if the configuration has an identifier, we are dealing with a background session, // therefore, we do not have to listen to the notifications. isBackgroundSession = !string.IsNullOrEmpty (configuration.Identifier); @@ -173,13 +173,12 @@ public NSUrlSessionHandler (NSUrlSessionConfiguration configuration) inflightRequests = new Dictionary (); } -#if !MONOMAC && !WATCH +#if !MONOMAC && !MONOTOUCH_WATCH void AddNotification () { if (!isBackgroundSession && notificationToken == null) - using (var notification = new NSString ("UIApplicationWillResignActiveNotification")) - notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (notification, BackgroundNotificationCb); + notificationToken = NSNotificationCenter.DefaultCenter.AddObserver (UIApplication.WillResignActiveNotification, BackgroundNotificationCb); } void RemoveNotification () @@ -206,7 +205,7 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { lock (inflightRequestsLock) { inflightRequests.Remove (task); -#if !MONOMAC && !WATCH +#if !MONOMAC && !MONOTOUCH_WATCH // do we need to be notified? If we have not inflightData, we do not if (inflightRequests.Count == 0) RemoveNotification (); @@ -221,7 +220,7 @@ void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) protected override void Dispose (bool disposing) { -#if !MONOMAC && !WATCH +#if !MONOMAC && !MONOTOUCH_WATCH // remove the notification if present, method checks against null RemoveNotification (); #endif @@ -305,7 +304,7 @@ protected override async Task SendAsync (HttpRequestMessage }); lock (inflightRequestsLock) { -#if !MONOMAC && !WATCH +#if !MONOMAC && !MONOTOUCH_WATCH // Add the notification whenever needed AddNotification (); #endif