From 5706a6bdcaeefaf94fcc4719ff530f0537054041 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Wed, 22 Mar 2017 14:02:07 +0100 Subject: [PATCH 1/2] [foundation] Honor the cancellation token win the async requests in NSUrlSessionHandler. Fixes #51423 (#1888) Do pass the cancelation token in the NSUrlSessionHandler to ensure that if the request is cancelled we do not hang for ever. https://bugzilla.xamarin.com/show_bug.cgi?id=51423 --- src/Foundation/NSUrlSessionHandler.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 3d348d71decf..0fdc21e24cce 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -223,15 +223,24 @@ public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataT var inflight = GetInflightData (dataTask); try { + // ensure that we did not cancel the request, if we did, do cancel the task + if (inflight.CancellationToken.IsCancellationRequested) { + dataTask.Cancel (); + } + var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode; var content = new NSUrlSessionDataTaskStreamContent (inflight.Stream, () => { + if (!inflight.Completed) { + dataTask.Cancel (); + } + inflight.Disposed = true; inflight.Stream.TrySetException (new ObjectDisposedException ("The content stream was disposed.")); sessionHandler.RemoveInflightData (dataTask); - }); + }, inflight.CancellationToken); // NB: The double cast is because of a Xamarin compiler bug var httpResponse = new HttpResponseMessage ((HttpStatusCode)status) { @@ -278,7 +287,7 @@ public override void DidCompleteWithError (NSUrlSession session, NSUrlSessionTas { var inflight = GetInflightData (task); - // this can happen if the HTTP request times out and it is removed as part of the cancelation process + // this can happen if the HTTP request times out and it is removed as part of the cancellation process if (inflight != null) { // set the stream as finished inflight.Stream.TrySetReceivedAllData (); @@ -396,7 +405,7 @@ class NSUrlSessionDataTaskStreamContent : StreamContent { Action disposed; - public NSUrlSessionDataTaskStreamContent (NSUrlSessionDataTaskStream source, Action onDisposed) : base (source) + public NSUrlSessionDataTaskStreamContent (NSUrlSessionDataTaskStream source, Action onDisposed, CancellationToken token) : base (source, token) { disposed = onDisposed; } From 11258ee4ef2d7937b9d850531df75a849c65d1d9 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 23 Mar 2017 22:42:04 +0100 Subject: [PATCH 2/2] [Foundation] Ensure that if a request is canceled we cancel the NSUrlSessionTask (#1900) If the request is canceled, the inflight data is cleaned up. That allows us to know that the NSUrlSessionTask should be canceled, which will execute the DidCompleteWithError that will take care of the cleanup of resources and will ensure that everything is back to a known situation. * Do check for managed cancelations in the GetInflightData so that it is check in all handlers. --- src/Foundation/NSUrlSessionHandler.cs | 28 +++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 0fdc21e24cce..e25f6ced2c98 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -212,9 +212,15 @@ InflightData GetInflightData (NSUrlSessionTask task) var inflight = default (InflightData); lock (sessionHandler.inflightRequestsLock) - if (sessionHandler.inflightRequests.TryGetValue (task, out inflight)) + if (sessionHandler.inflightRequests.TryGetValue (task, out inflight)) { + // ensure that we did not cancel the request, if we did, do cancel the task + if (inflight.CancellationToken.IsCancellationRequested) + task?.Cancel (); return inflight; + } + // if we did not manage to get the inflight data, we either got an error or have been canceled, lets cancel the task, that will execute DidCompleteWithError + task?.Cancel (); return null; } @@ -222,12 +228,10 @@ public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataT { var inflight = GetInflightData (dataTask); - try { - // ensure that we did not cancel the request, if we did, do cancel the task - if (inflight.CancellationToken.IsCancellationRequested) { - dataTask.Cancel (); - } + if (inflight == null) + return; + try { var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode; @@ -279,6 +283,9 @@ public override void DidReceiveData (NSUrlSession session, NSUrlSessionDataTask { var inflight = GetInflightData (dataTask); + if (inflight == null) + return; + inflight.Stream.Add (data); SetResponse (inflight); } @@ -338,6 +345,11 @@ public override void WillPerformHttpRedirection (NSUrlSession session, NSUrlSess public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask task, NSUrlAuthenticationChallenge challenge, Action completionHandler) { + var inflight = GetInflightData (task); + + if (inflight == null) + return; + // case for the basic auth failing up front. As per apple documentation: // The URL Loading System is designed to handle various aspects of the HTTP protocol for you. As a result, you should not modify the following headers using // the addValue(_:forHTTPHeaderField:) or setValue(_:forHTTPHeaderField:) methods: @@ -352,7 +364,7 @@ public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask // header, it means that we do not have the correct credentials, in any other case just do what it is expected. if (challenge.PreviousFailureCount == 0) { - var authHeader = GetInflightData (task)?.Request?.Headers?.Authorization; + var authHeader = inflight.Request?.Headers?.Authorization; if (!(string.IsNullOrEmpty (authHeader?.Scheme) && string.IsNullOrEmpty (authHeader?.Parameter))) { completionHandler (NSUrlSessionAuthChallengeDisposition.RejectProtectionSpace, null); return; @@ -363,7 +375,7 @@ public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask if (sessionHandler.Credentials != null) { var credentialsToUse = sessionHandler.Credentials as NetworkCredential; if (credentialsToUse == null) { - var uri = GetInflightData (task).Request.RequestUri; + var uri = inflight.Request.RequestUri; credentialsToUse = sessionHandler.Credentials.GetCredential (uri, "NTLM"); } var credential = new NSUrlCredential (credentialsToUse.UserName, credentialsToUse.Password, NSUrlCredentialPersistence.ForSession);