From 0186713274192336b48794cf98d3ac2e8e60c15c Mon Sep 17 00:00:00 2001
From: Marek Habersack <grendel@twistedcode.net>
Date: Thu, 9 Mar 2017 03:20:34 +0100
Subject: [PATCH] [Mono.Android] Make HTTP connection attempt cancellable
 (#472)

The `HttpURLConnection.ConnectAsync` API doesn't accept a `CancellationToken`
instance and, thus, cannot be easily cancelled in a graceful manner. This
limitation resulted in using the `Task.WhenAny` to make sure the connection
attempt is aborted whenever the calling task is cancelled. This, however, led to
a problem of unobserved exceptions should the cancellation occur before the
connection attempt was successful.

This commit wraps the synchronous `HttpURLConnection.Connect` call in a task
that is passed the cancellation token, so that it can be gracefully cancelled
when needed. The cancellation is done by registering a handler for when token is
about to expire as Task.Run will check if cancellation was requested only before
it runs the passed code, the rest is the code's responsibility.
Since `URLConnection.Connect()` is asynchronous we abort it by calling
`Disconnect()` on the instance which is a brutal but effective way to interrupt
the connection attempt.

Additionally, the diff makes sure to properly configure a few tasks for `await`

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=51804
---
 .../AndroidClientHandler.cs                   | 37 ++++++++++---------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs
index 3d4d6fb14de..ad577a49673 100644
--- a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs
+++ b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs
@@ -209,8 +209,8 @@ string EncodeUrl (Uri url)
 			while (true) {
 				URL java_url = new URL (EncodeUrl (redirectState.NewUrl));
 				URLConnection java_connection = java_url.OpenConnection ();
-				HttpURLConnection httpConnection = await SetupRequestInternal (request, java_connection);
-				HttpResponseMessage response = await ProcessRequest (request, java_url, httpConnection, cancellationToken, redirectState);
+				HttpURLConnection httpConnection = await SetupRequestInternal (request, java_connection).ConfigureAwait (continueOnCapturedContext: false);;
+				HttpResponseMessage response = await ProcessRequest (request, java_url, httpConnection, cancellationToken, redirectState).ConfigureAwait (continueOnCapturedContext: false);;
 				if (response != null)
 					return response;
 
@@ -235,6 +235,19 @@ Task DisconnectAsync (HttpURLConnection httpConnection)
 			return Task.Run (() => httpConnection?.Disconnect ());
 		}
 
+		Task ConnectAsync (HttpURLConnection httpConnection, CancellationToken ct)
+		{
+			return Task.Run (() => {
+				try {
+					using (ct.Register (() => httpConnection?.Disconnect ()))
+						httpConnection?.Connect ();
+				} catch {
+					ct.ThrowIfCancellationRequested ();
+					throw;
+				}
+			}, ct);
+		}
+
 		async Task <HttpResponseMessage> DoProcessRequest (HttpRequestMessage request, URL javaUrl, HttpURLConnection httpConnection, CancellationToken cancellationToken, RequestRedirectionState redirectState)
 		{
 			if (Logger.LogNet)
@@ -247,20 +260,11 @@ Task DisconnectAsync (HttpURLConnection httpConnection)
 				cancellationToken.ThrowIfCancellationRequested ();
 			}
 
-			CancellationTokenSource waitHandleSource = new CancellationTokenSource();
 			try {
 				if (Logger.LogNet)
 					Logger.Log (LogLevel.Info, LOG_APP, $"  connecting");
 
-				CancellationToken linkedToken = CancellationTokenSource.CreateLinkedTokenSource(waitHandleSource.Token, cancellationToken).Token;
-				await Task.WhenAny (
-						httpConnection.ConnectAsync (),
-						Task.Run (()=> { 
-							linkedToken.WaitHandle.WaitOne();
-							if (Logger.LogNet)
-								Logger.Log(LogLevel.Info, LOG_APP, $"Wait handle task finished");
-						}))
-					.ConfigureAwait(false);
+				await ConnectAsync (httpConnection, cancellationToken).ConfigureAwait (continueOnCapturedContext: false);
 				if (Logger.LogNet)
 					Logger.Log (LogLevel.Info, LOG_APP, $"  connected");
 			} catch (Java.Net.ConnectException ex) {
@@ -268,9 +272,6 @@ await Task.WhenAny (
 					Logger.Log (LogLevel.Info, LOG_APP, $"Connection exception {ex}");
 				// Wrap it nicely in a "standard" exception so that it's compatible with HttpClientHandler
 				throw new WebException (ex.Message, ex, WebExceptionStatus.ConnectFailure, null);
-			} finally{
-				//If not already cancelled, cancel the WaitOne through the waitHandleSource to prevent an orphaned thread
-				waitHandleSource.Cancel();
 			}
 
 			if (cancellationToken.IsCancellationRequested) {
@@ -541,7 +542,7 @@ void CopyHeaders (HttpURLConnection httpConnection, HttpResponseMessage response
 		/// <param name="conn">Pre-configured connection instance</param>
 		protected virtual Task SetupRequest (HttpRequestMessage request, HttpURLConnection conn)
 		{
-			return Task.Factory.StartNew (AssertSelf);
+			return Task.Run (AssertSelf);
 		}
 
 		/// <summary>
@@ -645,9 +646,9 @@ void AppendEncoding (string encoding, ref List <string> list)
 			}
 			
 			HandlePreAuthentication (httpConnection);
-			await SetupRequest (request, httpConnection);
+			await SetupRequest (request, httpConnection).ConfigureAwait (continueOnCapturedContext: false);;
 			SetupRequestBody (httpConnection, request);
-			
+
 			return httpConnection;
 		}