Merge remote-tracking branch 'upstream/master' into navToPerfWork4

docs/Language Feature Status.md

@@ -35,4 +35,4 @@ efforts behind them.
 # FAQ
 
 - **Is target version a guarantee?**: No.  It's explicitly not a guarantee.  This is just the planned and ongoing work to the best of our knowledge at this time.
-- **Where are these State values defined?**: Take a look at the [Developing a Language Feature](contributing/Developing a Language Feature.md) document.
+- **Where are these State values defined?**: Take a look at the [Developing a Language Feature](contributing/Developing%20a%20Language%20Feature.md) document.

src/Features/Core/Portable/TodoComments/IRemoteTodoCommentService.cs

@@ -4,6 +4,7 @@
 using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Execution;
 
 namespace Microsoft.CodeAnalysis.TodoComments
 {
@@ -12,6 +13,6 @@ namespace Microsoft.CodeAnalysis.TodoComments
     /// </summary>
     internal interface IRemoteTodoCommentService
     {
-        Task<IList<TodoComment>> GetTodoCommentsAsync(DocumentId documentId, ImmutableArray<TodoCommentDescriptor> commentDescriptors, CancellationToken cancellationToken);
+        Task<IList<TodoComment>> GetTodoCommentsAsync(PinnedSolutionInfo solutionInfo, DocumentId documentId, ImmutableArray<TodoCommentDescriptor> commentDescriptors, CancellationToken cancellationToken);
     }
 }

src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs

@@ -119,11 +119,21 @@ private static async Task RegisterWorkspaceHostAsync(Workspace workspace, Remote
 
         public override async Task<Connection> TryCreateConnectionAsync(string serviceName, object callbackTarget, CancellationToken cancellationToken)
         {
+            var dataRpc = _remotableDataRpc.TryAddReference();
+            if (dataRpc == null)
+            {
+                // dataRpc is disposed. this can happen if someone killed remote host process while there is
+                // no other one holding the data connection.
+                // in those error case, don't crash but return null. this method is TryCreate since caller expects it to return null
+                // on such error situation.
+                return null;
+            }
+
             // get stream from service hub to communicate service specific information
             // this is what consumer actually use to communicate information
             var serviceStream = await RequestServiceAsync(_hubClient, serviceName, _hostGroup, _timeout, cancellationToken).ConfigureAwait(false);
 
-            return new JsonRpcConnection(callbackTarget, serviceStream, _remotableDataRpc.TryAddReference());
+            return new JsonRpcConnection(callbackTarget, serviceStream, dataRpc);
         }
 
         protected override void OnStarted()

src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs

@@ -73,11 +73,16 @@ public async Task TestTodoComments()
 
                 var solution = workspace.CurrentSolution;
 
-                var comments = await client.TryRunCodeAnalysisRemoteAsync<IList<TodoComment>>(
-                    solution, nameof(IRemoteTodoCommentService.GetTodoCommentsAsync),
-                    new object[] { solution.Projects.First().DocumentIds.First(), ImmutableArray.Create(new TodoCommentDescriptor("TODO", 0)) }, CancellationToken.None);
+                var keepAliveSession = await client.TryCreateCodeAnalysisKeepAliveSessionAsync(CancellationToken.None);
+                var comments = await keepAliveSession.TryInvokeAsync<IList<TodoComment>>(
+                    nameof(IRemoteTodoCommentService.GetTodoCommentsAsync),
+                    solution,
+                    new object[] { solution.Projects.First().DocumentIds.First(), ImmutableArray.Create(new TodoCommentDescriptor("TODO", 0)) },
+                    CancellationToken.None);
 
                 Assert.Equal(comments.Count, 1);
+
+                keepAliveSession.Shutdown(CancellationToken.None);
             }
         }
 

src/Workspaces/Core/Portable/Remote/RemoteHostSessionHelpers.cs

@@ -69,153 +69,157 @@ public Task<T> InvokeAsync<T>(string targetName, IReadOnlyList<object> arguments
     /// This will let one to hold onto <see cref="RemoteHostClient.Connection"/> for a while.
     /// this helper will let you not care about remote host being gone while you hold onto the connection if that ever happen
     /// 
-    /// and also make sure state is correct even if multiple threads call TryInvokeAsync at the same time. but this 
-    /// is not optimized to handle highly concurrent usage. if highly concurrent usage is required, either using
-    /// <see cref="RemoteHostClient.Connection"/> direclty or using <see cref="SessionWithSolution"/> would be better choice
+    /// when this is used, solution must be explicitly passed around between client (VS) and remote host (OOP)
     /// </summary>
     internal sealed class KeepAliveSession
     {
-        private readonly SemaphoreSlim _gate;
+        private readonly object _gate;
         private readonly IRemoteHostClientService _remoteHostClientService;
 
         private readonly string _serviceName;
         private readonly object _callbackTarget;
 
         private RemoteHostClient _client;
-        private RemoteHostClient.Connection _connection;
+        private ReferenceCountedDisposable<RemoteHostClient.Connection> _connectionDoNotAccessDirectly;
 
         public KeepAliveSession(RemoteHostClient client, RemoteHostClient.Connection connection, string serviceName, object callbackTarget)
         {
-            Initialize_NoLock(client, connection);
+            _gate = new object();
 
-            _gate = new SemaphoreSlim(initialCount: 1);
-            _remoteHostClientService = client.Workspace.Services.GetService<IRemoteHostClientService>();
+            Initialize(client, connection);
 
+            _remoteHostClientService = client.Workspace.Services.GetService<IRemoteHostClientService>();
             _serviceName = serviceName;
             _callbackTarget = callbackTarget;
         }
 
         public void Shutdown(CancellationToken cancellationToken)
         {
-            using (_gate.DisposableWait(cancellationToken))
+            ReferenceCountedDisposable<RemoteHostClient.Connection> connection;
+
+            lock (_gate)
             {
                 if (_client != null)
                 {
                     _client.StatusChanged -= OnStatusChanged;
                 }
 
-                _connection?.Dispose();
+                connection = _connectionDoNotAccessDirectly;
 
                 _client = null;
-                _connection = null;
+                _connectionDoNotAccessDirectly = null;
             }
+
+            connection?.Dispose();
         }
 
         public async Task<bool> TryInvokeAsync(string targetName, IReadOnlyList<object> arguments, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return false;
                 }
 
-                await connection.InvokeAsync(targetName, arguments, cancellationToken).ConfigureAwait(false);
+                await connection.Target.InvokeAsync(targetName, arguments, cancellationToken).ConfigureAwait(false);
                 return true;
             }
         }
 
         public async Task<T> TryInvokeAsync<T>(string targetName, IReadOnlyList<object> arguments, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return default;
                 }
 
-                return await connection.InvokeAsync<T>(targetName, arguments, cancellationToken).ConfigureAwait(false);
+                return await connection.Target.InvokeAsync<T>(targetName, arguments, cancellationToken).ConfigureAwait(false);
             }
         }
 
         public async Task<bool> TryInvokeAsync(string targetName, IReadOnlyList<object> arguments, Func<Stream, CancellationToken, Task> funcWithDirectStreamAsync, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return false;
                 }
 
-                await connection.InvokeAsync(targetName, arguments, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
+                await connection.Target.InvokeAsync(targetName, arguments, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
                 return true;
             }
         }
 
         public async Task<T> TryInvokeAsync<T>(string targetName, IReadOnlyList<object> arguments, Func<Stream, CancellationToken, Task<T>> funcWithDirectStreamAsync, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return default;
                 }
 
-                return await connection.InvokeAsync(targetName, arguments, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
+                return await connection.Target.InvokeAsync(targetName, arguments, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
             }
         }
 
         public async Task<bool> TryInvokeAsync(string targetName, Solution solution, IReadOnlyList<object> arguments, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var pooledObject = SharedPools.Default<List<object>>().GetPooledObject())
             using (var scope = await solution.GetPinnedScopeAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return false;
                 }
 
-                await connection.RegisterPinnedRemotableDataScopeAsync(scope).ConfigureAwait(false);
-                await connection.InvokeAsync(targetName, arguments, cancellationToken).ConfigureAwait(false);
+                pooledObject.Object.Add(scope.SolutionInfo);
+                pooledObject.Object.AddRange(arguments);
+
+                await connection.Target.InvokeAsync(targetName, pooledObject.Object, cancellationToken).ConfigureAwait(false);
                 return true;
             }
         }
 
         public async Task<T> TryInvokeAsync<T>(string targetName, Solution solution, IReadOnlyList<object> arguments, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var pooledObject = SharedPools.Default<List<object>>().GetPooledObject())
             using (var scope = await solution.GetPinnedScopeAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return default;
                 }
 
-                await connection.RegisterPinnedRemotableDataScopeAsync(scope).ConfigureAwait(false);
-                return await connection.InvokeAsync<T>(targetName, arguments, cancellationToken).ConfigureAwait(false);
+                pooledObject.Object.Add(scope.SolutionInfo);
+                pooledObject.Object.AddRange(arguments);
+
+                return await connection.Target.InvokeAsync<T>(targetName, pooledObject.Object, cancellationToken).ConfigureAwait(false);
             }
         }
 
         public async Task<bool> TryInvokeAsync(
             string targetName, Solution solution, IReadOnlyList<object> arguments, Func<Stream, CancellationToken, Task> funcWithDirectStreamAsync, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var pooledObject = SharedPools.Default<List<object>>().GetPooledObject())
             using (var scope = await solution.GetPinnedScopeAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return false;
                 }
 
-                await connection.RegisterPinnedRemotableDataScopeAsync(scope).ConfigureAwait(false);
-                await connection.InvokeAsync(targetName, arguments, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
+                pooledObject.Object.Add(scope.SolutionInfo);
+                pooledObject.Object.AddRange(arguments);
+
+                await connection.Target.InvokeAsync(targetName, pooledObject.Object, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
                 return true;
             }
         }
@@ -223,25 +227,30 @@ public async Task<T> TryInvokeAsync<T>(string targetName, Solution solution, IRe
         public async Task<T> TryInvokeAsync<T>(
             string targetName, Solution solution, IReadOnlyList<object> arguments, Func<Stream, CancellationToken, Task<T>> funcWithDirectStreamAsync, CancellationToken cancellationToken)
         {
-            using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
+            using (var pooledObject = SharedPools.Default<List<object>>().GetPooledObject())
             using (var scope = await solution.GetPinnedScopeAsync(cancellationToken).ConfigureAwait(false))
+            using (var connection = await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false))
             {
-                var connection = await TryGetConnection_NoLockAsync(cancellationToken).ConfigureAwait(false);
                 if (connection == null)
                 {
                     return default;
                 }
 
-                await connection.RegisterPinnedRemotableDataScopeAsync(scope).ConfigureAwait(false);
-                return await connection.InvokeAsync(targetName, arguments, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
+                pooledObject.Object.Add(scope.SolutionInfo);
+                pooledObject.Object.AddRange(arguments);
+
+                return await connection.Target.InvokeAsync(targetName, pooledObject.Object, funcWithDirectStreamAsync, cancellationToken).ConfigureAwait(false);
             }
         }
 
-        private async Task<RemoteHostClient.Connection> TryGetConnection_NoLockAsync(CancellationToken cancellationToken)
+        private async Task<ReferenceCountedDisposable<RemoteHostClient.Connection>> TryGetConnectionAsync(CancellationToken cancellationToken)
         {
-            if (_connection != null)
+            lock (_gate)
             {
-                return _connection;
+                if (_connectionDoNotAccessDirectly != null)
+                {
+                    return _connectionDoNotAccessDirectly.TryAddReference();
+                }
             }
 
             var client = await _remoteHostClientService.TryGetRemoteHostClientAsync(cancellationToken).ConfigureAwait(false);
@@ -250,15 +259,15 @@ private async Task<RemoteHostClient.Connection> TryGetConnection_NoLockAsync(Can
                 return null;
             }
 
-            var session = await client.TryCreateConnectionAsync(_serviceName, _callbackTarget, cancellationToken).ConfigureAwait(false);
-            if (session == null)
+            var connection = await client.TryCreateConnectionAsync(_serviceName, _callbackTarget, cancellationToken).ConfigureAwait(false);
+            if (connection == null)
             {
                 return null;
             }
 
-            Initialize_NoLock(client, session);
+            Initialize(client, connection);
 
-            return _connection;
+            return await TryGetConnectionAsync(cancellationToken).ConfigureAwait(false);
         }
 
         private void OnStatusChanged(object sender, bool connection)
@@ -271,15 +280,28 @@ private void OnStatusChanged(object sender, bool connection)
             Shutdown(CancellationToken.None);
         }
 
-        private void Initialize_NoLock(RemoteHostClient client, RemoteHostClient.Connection connection)
+        private void Initialize(RemoteHostClient client, RemoteHostClient.Connection connection)
         {
             Contract.ThrowIfNull(client);
             Contract.ThrowIfNull(connection);
 
-            _client = client;
-            _client.StatusChanged += OnStatusChanged;
+            lock (_gate)
+            {
+                if (_client != null)
+                {
+                    Contract.ThrowIfNull(_connectionDoNotAccessDirectly);
 
-            _connection = connection;
+                    // someone else beat us and set the connection. 
+                    // let this connection closed.
+                    connection.Dispose();
+                    return;
+                }
+
+                _client = client;
+                _client.StatusChanged += OnStatusChanged;
+
+                _connectionDoNotAccessDirectly = new ReferenceCountedDisposable<RemoteHostClient.Connection>(connection);
+            }
         }
     }
 }

src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_TodoComments.cs

@@ -6,6 +6,7 @@
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Execution;
 using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.TodoComments;
@@ -22,11 +23,11 @@ internal partial class CodeAnalysisService : IRemoteTodoCommentService
         /// 
         /// This will be called by ServiceHub/JsonRpc framework
         /// </summary>
-        public async Task<IList<TodoComment>> GetTodoCommentsAsync(DocumentId documentId, ImmutableArray<TodoCommentDescriptor> tokens, CancellationToken cancellationToken)
+        public async Task<IList<TodoComment>> GetTodoCommentsAsync(PinnedSolutionInfo solutionInfo, DocumentId documentId, ImmutableArray<TodoCommentDescriptor> tokens, CancellationToken cancellationToken)
         {
             using (RoslynLogger.LogBlock(FunctionId.CodeAnalysisService_GetTodoCommentsAsync, documentId.ProjectId.DebugName, cancellationToken))
             {
-                var solution = await GetSolutionAsync(cancellationToken).ConfigureAwait(false);
+                var solution = await GetSolutionAsync(solutionInfo, cancellationToken).ConfigureAwait(false);
                 var document = solution.GetDocument(documentId);
 
                 var service = document.GetLanguageService<ITodoCommentService>();

src/Workspaces/Remote/ServiceHub/Shared/ServiceHubServiceBase.cs

@@ -102,8 +102,13 @@ protected Task<Solution> GetSolutionAsync(CancellationToken cancellationToken)
         {
             Contract.ThrowIfNull(_solutionInfo);
 
-            var solutionController = (ISolutionController)RoslynServices.SolutionService;
-            return solutionController.GetSolutionAsync(_solutionInfo.SolutionChecksum, _solutionInfo.FromPrimaryBranch, cancellationToken);
+            return GetSolutionAsync(RoslynServices, _solutionInfo, cancellationToken);
+        }
+
+        protected Task<Solution> GetSolutionAsync(PinnedSolutionInfo solutionInfo, CancellationToken cancellationToken)
+        {
+            var localRoslynService = new RoslynServices(solutionInfo.ScopeId, AssetStorage);
+            return GetSolutionAsync(localRoslynService, solutionInfo, cancellationToken);
         }
 
         protected virtual void Dispose(bool disposing)
@@ -158,5 +163,11 @@ private void OnRpcDisconnected(object sender, JsonRpcDisconnectedEventArgs e)
                 Log(TraceEventType.Warning, $"Client stream disconnected unexpectedly: {e.Exception?.GetType().Name} {e.Exception?.Message}");
             }
         }
+
+        private static Task<Solution> GetSolutionAsync(RoslynServices roslynService, PinnedSolutionInfo solutionInfo, CancellationToken cancellationToken)
+        {
+            var solutionController = (ISolutionController)roslynService.SolutionService;
+            return solutionController.GetSolutionAsync(solutionInfo.SolutionChecksum, solutionInfo.FromPrimaryBranch, cancellationToken);
+        }
     }
 }