Perf: significant resource optimization around garbage collection, memory and connection utilization

libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs

@@ -751,9 +751,11 @@ public async Task<ConversationsResult> GetConversationsAsync(string serviceUrl,
                 throw new ArgumentNullException(nameof(credentials));
             }
 
-            var connectorClient = CreateConnectorClient(serviceUrl, credentials);
-            var results = await connectorClient.Conversations.GetConversationsAsync(continuationToken, cancellationToken).ConfigureAwait(false);
-            return results;
+            using (var connectorClient = CreateConnectorClient(serviceUrl, credentials))
+            {
+                var results = await connectorClient.Conversations.GetConversationsAsync(continuationToken, cancellationToken).ConfigureAwait(false);
+                return results;
+            }
         }
 
         /// <summary>
@@ -1602,14 +1604,14 @@ private IConnectorClient CreateConnectorClient(string serviceUrl, AppCredentials
                 ConnectorClient connectorClient;
                 if (appCredentials != null)
                 {
-                    connectorClient = new ConnectorClient(new Uri(serviceUrl), appCredentials, customHttpClient: _httpClient);
+                    connectorClient = new ConnectorClient(new Uri(serviceUrl), appCredentials, customHttpClient: _httpClient, disposeHttpClient: _httpClient == null);
                 }
                 else
                 {
                     var emptyCredentials = (ChannelProvider != null && ChannelProvider.IsGovernment()) ?
                         MicrosoftGovernmentAppCredentials.Empty :
                         MicrosoftAppCredentials.Empty;
-                    connectorClient = new ConnectorClient(new Uri(serviceUrl), emptyCredentials, customHttpClient: _httpClient);
+                    connectorClient = new ConnectorClient(new Uri(serviceUrl), emptyCredentials, customHttpClient: _httpClient, disposeHttpClient: _httpClient == null);
                 }
 
                 if (_connectorClientRetryPolicy != null)

libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs

@@ -243,13 +243,17 @@ protected async Task ProcessProactiveAsync(ClaimsIdentity claimsIdentity, Conver
             var credentials = await _botFrameworkAuthentication.GetProactiveCredentialsAsync(claimsIdentity, audience, cancellationToken).ConfigureAwait(false);
 
             // Create the connector client to use for outbound requests.
-            var connectorClient = new ConnectorClient(new Uri(reference.ServiceUrl), credentials, _httpClient);
-
-            // Create a turn context and run the pipeline.
-            using (var context = CreateTurnContext(reference.GetContinuationActivity(), claimsIdentity, audience, connectorClient, callback))
+            using (var connectorClient = new ConnectorClient(new Uri(reference.ServiceUrl), credentials, _httpClient, disposeHttpClient: _httpClient == null))
             {
-                // Run the pipeline.
-                await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false);
+                // Create a turn context and run the pipeline.
+                using (var context = CreateTurnContext(reference.GetContinuationActivity(), claimsIdentity, audience, connectorClient, callback))
+                {
+                    // Run the pipeline.
+                    await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false);
+
+                    // Cleanup disposable resources in case other code kept a reference to it.
+                    context.TurnState.Set<IConnectorClient>(null);
+                }
             }
         }
 
@@ -270,16 +274,20 @@ protected async Task<InvokeResponse> ProcessActivityAsync(string authHeader, Act
             activity.CallerId = authenticateRequestResult.CallerId;
 
             // Create the connector client to use for outbound requests.
-            var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), authenticateRequestResult.Credentials, _httpClient);
-
-            // Create a turn context and run the pipeline.
-            using (var context = CreateTurnContext(activity, authenticateRequestResult.ClaimsIdentity, authenticateRequestResult.Scope, connectorClient, callback))
+            using (var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), authenticateRequestResult.Credentials, _httpClient, disposeHttpClient: _httpClient == null))
             {
-                // Run the pipeline.
-                await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false);
+                // Create a turn context and run the pipeline.
+                using (var context = CreateTurnContext(activity, authenticateRequestResult.ClaimsIdentity, authenticateRequestResult.Scope, connectorClient, callback))
+                {
+                    // Run the pipeline.
+                    await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false);
 
-                // If there are any results they will have been left on the TurnContext. 
-                return ProcessTurnResults(context);
+                    // Cleanup disposable resources in case other code kept a reference to it.
+                    context.TurnState.Set<IConnectorClient>(null);
+
+                    // If there are any results they will have been left on the TurnContext. 
+                    return ProcessTurnResults(context);
+                }
             }
         }
 

libraries/Microsoft.Bot.Builder/Inspection/InspectionSession.cs

@@ -22,7 +22,7 @@ public InspectionSession(ConversationReference conversationReference, MicrosoftA
         {
             _conversationReference = conversationReference;
             _logger = logger;
-            _connectorClient = new ConnectorClient(new Uri(_conversationReference.ServiceUrl), credentials, httpClient);
+            _connectorClient = new ConnectorClient(new Uri(_conversationReference.ServiceUrl), credentials, httpClient, disposeHttpClient: httpClient == null);
         }
 
         public async Task<bool> SendAsync(Activity activity, CancellationToken cancellationToken)

libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs

@@ -143,11 +143,17 @@ public async Task<InvokeResponse> ProcessStreamingActivityAsync(Activity activit
                 {
                     context.TurnState.Add<IIdentity>(BotIdentityKey, ClaimsIdentity);
                 }
-
-                var connectorClient = CreateStreamingConnectorClient(activity, requestHandler);
-                context.TurnState.Add(connectorClient);
 
-                await RunPipelineAsync(context, callbackHandler, cancellationToken).ConfigureAwait(false);
+                using (var connectorClient = CreateStreamingConnectorClient(activity, requestHandler))
+                {
+                    // Add connector client to be used throughout the turn
+                    context.TurnState.Add(connectorClient);
+
+                    await RunPipelineAsync(context, callbackHandler, cancellationToken).ConfigureAwait(false);
+
+                    // Cleanup connector client 
+                    context.TurnState.Set<IConnectorClient>(null);
+                }
 
                 if (activity.Type == ActivityTypes.Invoke)
                 {
@@ -308,7 +314,7 @@ private IConnectorClient CreateStreamingConnectorClient(Activity activity, Strea
 #pragma warning disable CA2000 // Dispose objects before losing scope (We need to make ConnectorClient disposable to fix this, ignoring it for now)
             var streamingClient = new StreamingHttpClient(requestHandler, Logger);
 #pragma warning restore CA2000 // Dispose objects before losing scope
-            var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), emptyCredentials, customHttpClient: streamingClient);
+            var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), emptyCredentials, customHttpClient: streamingClient, disposeHttpClient: false);
             return connectorClient;
         }
 

libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs

@@ -225,7 +225,7 @@ public override async Task<StreamingResponse> ProcessRequestAsync(ReceiveRequest
             string body;
             try
             {
-                body = request.ReadBodyAsString();
+                body = await request.ReadBodyAsStringAsync().ConfigureAwait(false);
             }
 #pragma warning disable CA1031 // Do not catch general exception types (we log the exception and continue execution)
             catch (Exception ex)

libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs

@@ -62,6 +62,34 @@ public ConnectorClient(Uri baseUri, MicrosoftAppCredentials credentials, HttpCli
         {
         }
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ConnectorClient"/> class.
+        /// </summary>
+        /// <param name="baseUri">Base URI for the Bot Connector service.</param>
+        /// <param name="credentials">Credentials for the Bot Connector service.</param>
+        /// <param name="customHttpClient">The HTTP client to use for this connector client.</param>
+        /// <param name="disposeHttpClient">Whether to dispose the <see cref="HttpClient"/>.</param>
+        /// <remarks>Constructor specifically designed to be the one that allows control of the disposing of the custom <see cref="HttpClient"/>.
+        /// <see cref="ServiceClient{T}"/> only has one constructor that accepts control of the disposing of the <see cref="HttpClient"/>, so we call that overload here.
+        /// All other overloads of <see cref="ConnectorClient"/> will not control this parameter and it will default to true, resulting on disposal of the provided <see cref="HttpClient"/> when the <see cref="ConnectorClient"/> is disposed.
+        /// When reusing <see cref="HttpClient"/> instances across connectors, pass 'false' for <paramref name="disposeHttpClient"/> to avoid <see cref="ObjectDisposedException"/>.</remarks>
+#pragma warning disable CA1801 // Review unused parameters (we can't change this without breaking binary compat)
+        public ConnectorClient(Uri baseUri, ServiceClientCredentials credentials, HttpClient customHttpClient, bool disposeHttpClient)
+#pragma warning restore CA1801 // Review unused parameters
+            : base(customHttpClient, disposeHttpClient)
+        {
+            this.Credentials = credentials;
+
+            if (baseUri == null)
+            {
+                throw new ArgumentNullException(nameof(baseUri));
+            }
+
+            Initialize();
+
+            BaseUri = baseUri;
+        }
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ConnectorClient"/> class.
         /// </summary>

libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs

@@ -5,6 +5,7 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using System.Threading.Tasks;
 using Newtonsoft.Json;
 
 namespace Microsoft.Bot.Streaming
@@ -23,29 +24,37 @@ public static class ReceiveRequestExtensions
         /// Otherwise a default instance of type T.
         /// </returns>
         public static T ReadBodyAsJson<T>(this ReceiveRequest request)
+        {
+            return request.ReadBodyAsJsonAsync<T>().GetAwaiter().GetResult();
+        }
+
+        /// <summary>
+        /// Serializes the body of this <see cref="ReceiveRequest"/> as JSON.
+        /// </summary>
+        /// <typeparam name="T">The type to attempt to deserialize the contents of this <see cref="ReceiveRequest"/>'s body into.</typeparam>
+        /// <param name="request">The current instance of <see cref="ReceiveRequest"/>.</param>
+        /// <returns>On success, an object of type T populated with data serialized from the <see cref="ReceiveRequest"/> body.
+        /// Otherwise a default instance of type T.
+        /// </returns>
+        public static async Task<T> ReadBodyAsJsonAsync<T>(this ReceiveRequest request)
         {
             // The first stream attached to a ReceiveRequest is always the ReceiveRequest body.
             // Any additional streams must be defined within the body or they will not be
             // attached properly when processing activities.
             var contentStream = request.Streams.FirstOrDefault();
 
             /* If the response had no body we have to return a compatible
-                 * but empty object to avoid throwing exceptions upstream anytime
-                 * an empty response is received.
-                 */
+            * but empty object to avoid throwing exceptions upstream anytime
+            * an empty response is received.
+            */
             if (contentStream == null)
             {
                 return default;
             }
 
-            using (var reader = new StreamReader(contentStream.Stream, Encoding.UTF8))
-            {
-                using (var jsonReader = new JsonTextReader(reader))
-                {
-                    var serializer = JsonSerializer.Create(SerializationSettings.DefaultDeserializationSettings);
-                    return serializer.Deserialize<T>(jsonReader);
-                }
-            }
+            var bodyString = await request.ReadBodyAsStringAsync().ConfigureAwait(false);
+
+            return JsonConvert.DeserializeObject<T>(bodyString, SerializationSettings.DefaultDeserializationSettings);
         }
 
         /// <summary>
@@ -56,17 +65,29 @@ public static T ReadBodyAsJson<T>(this ReceiveRequest request)
         /// Otherwise null.
         /// </returns>
         public static string ReadBodyAsString(this ReceiveRequest request)
+        {
+            return request.ReadBodyAsStringAsync().GetAwaiter().GetResult();
+        }
+
+        /// <summary>
+        /// Reads the body of this <see cref="ReceiveRequest"/> as a string.
+        /// </summary>
+        /// <param name="request">The current instance of <see cref="ReceiveRequest"/>.</param>
+        /// <returns>On success, a string populated with data read from the <see cref="ReceiveRequest"/> body.
+        /// Otherwise null.
+        /// </returns>
+        public static Task<string> ReadBodyAsStringAsync(this ReceiveRequest request)
         {
             var contentStream = request.Streams.FirstOrDefault();
 
             if (contentStream == null)
             {
-                return string.Empty;
+                return Task.FromResult(string.Empty);
             }
 
             using (var reader = new StreamReader(contentStream.Stream, Encoding.UTF8))
             {
-                return reader.ReadToEnd();
+                return reader.ReadToEndAsync();
             }
         }
     }

libraries/Microsoft.Bot.Streaming/ReceiveResponseExtensions.cs

@@ -5,6 +5,7 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using System.Threading.Tasks;
 using Newtonsoft.Json;
 
 namespace Microsoft.Bot.Streaming
@@ -52,12 +53,23 @@ public static T ReadBodyAsJson<T>(this ReceiveResponse response)
         /// <returns>On success, an <see cref="string"/> of the data from the <see cref="ReceiveResponse"/> body.
         /// </returns>
         public static string ReadBodyAsString(this ReceiveResponse response)
+        {
+            return response.ReadBodyAsStringAsync().GetAwaiter().GetResult();
+        }
+
+        /// <summary>
+        /// Serializes the body of this <see cref="ReceiveResponse"/> as a <see cref="string"/>.
+        /// </summary>
+        /// <param name="response">The current instance of <see cref="ReceiveResponse"/>.</param>
+        /// <returns>On success, an <see cref="string"/> of the data from the <see cref="ReceiveResponse"/> body.
+        /// </returns>
+        public static async Task<string> ReadBodyAsStringAsync(this ReceiveResponse response)
         {
             var contentStream = response.Streams.FirstOrDefault();
 
             if (contentStream != null)
             {
-                return contentStream.Stream.ReadAsUtf8String();
+                return await contentStream.Stream.ReadAsUtf8StringAsync().ConfigureAwait(false);
             }
 
             return string.Empty;

tests/Microsoft.Bot.Connector.Tests/ConnectorClientTest.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Net.Http;
+using System.Threading.Tasks;
 using Microsoft.Bot.Connector.Authentication;
 using Microsoft.Bot.Schema;
 using Xunit;
@@ -12,7 +13,7 @@ namespace Microsoft.Bot.Connector.Tests
     public class ConnectorClientTest : BaseTest
     {
         [Fact]
-        public void ConnectorClientWithCustomHttpClientAndMicrosoftCredentials()
+        public void ConnectorClient_CustomHttpClient_AndMicrosoftCredentials()
         {
             var baseUri = new Uri("https://test.coffee");
             var customHttpClient = new HttpClient();
@@ -23,5 +24,45 @@ public void ConnectorClientWithCustomHttpClientAndMicrosoftCredentials()
 
             Assert.Equal(connector.HttpClient.BaseAddress, baseUri);
         }
+
+        [Fact]
+        public async Task ConnectorClient_CustomHttpClientAndCredConstructor_HttpClientDisposed()
+        {
+            var baseUri = new Uri("https://test.coffee");
+            var customHttpClient = new HttpClient();
+
+            using (var connector = new ConnectorClient(new Uri("http://localhost/"), new MicrosoftAppCredentials(string.Empty, string.Empty), customHttpClient))
+            { 
+                // Use the connector
+            }
+
+            await Assert.ThrowsAsync<ObjectDisposedException>(() => customHttpClient.GetAsync("http://bing.com"));
+        }
+
+        [Fact]
+        public async Task ConnectorClient_CustomHttpClientAndDisposeFalse_HttpClientNotDisposed()
+        {
+            var baseUri = new Uri("https://test.coffee");
+            var customHttpClient = new HttpClient();
+
+            using (var connector = new ConnectorClient(new Uri("http://localhost/"), new MicrosoftAppCredentials(string.Empty, string.Empty), customHttpClient, disposeHttpClient: customHttpClient == null))
+            {
+                // Use the connector
+            }
+
+            // If the HttpClient were disposed, this would throw ObjectDisposedException
+            await customHttpClient.GetAsync("http://bing.com");
+        }
+
+        [Fact]
+        public void ConnectorClient_CustomHttpClientNull_Works()
+        {
+            var baseUri = new Uri("https://test.coffee");
+
+            using (var connector = new ConnectorClient(new Uri("http://localhost/"), new MicrosoftAppCredentials(string.Empty, string.Empty), null, disposeHttpClient: true))
+            {
+                // Use the connector
+            }
+        }
     }
 }