Merge pull request #18887 from sharwell/dispose-session

Fix failures to dispose PinnedRemotableDataScope

src/Compilers/Core/Portable/Optional.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System;
+
 namespace Microsoft.CodeAnalysis
 {
     /// <summary>
@@ -33,7 +35,14 @@ public bool HasValue
         /// <summary>
         /// Gets the value of the current object.
         /// </summary>
-        /// <returns></returns>
+        /// <remarks>
+        /// <para>Unlike <see cref="Nullable{T}.Value"/>, this property does not throw an exception when
+        /// <see cref="HasValue"/> is <see langword="false"/>.</para>
+        /// </remarks>
+        /// <returns>
+        /// <para>The value if <see cref="HasValue"/> is <see langword="true"/>; otherwise, the default value for type
+        /// <typeparamref name="T"/>.</para>
+        /// </returns>
         public T Value
         {
             get { return _value; }

src/EditorFeatures/TestUtilities/Remote/InProcRemostHostClient.cs

@@ -59,17 +59,17 @@ public static async Task<RemoteHostClient> CreateAsync(Workspace workspace, bool
 
         public AssetStorage AssetStorage => _inprocServices.AssetStorage;
 
-        protected override async Task<Session> TryCreateServiceSessionAsync(string serviceName, PinnedRemotableDataScope snapshot, object callbackTarget, CancellationToken cancellationToken)
+        protected override async Task<Session> TryCreateServiceSessionAsync(string serviceName, Optional<Func<CancellationToken, Task<PinnedRemotableDataScope>>> getSnapshotAsync, object callbackTarget, CancellationToken cancellationToken)
         {
             // get stream from service hub to communicate snapshot/asset related information
             // this is the back channel the system uses to move data between VS and remote host
-            var snapshotStream = await _inprocServices.RequestServiceAsync(WellKnownServiceHubServices.SnapshotService, cancellationToken).ConfigureAwait(false);
+            var snapshotStream = getSnapshotAsync.Value == null ? null : await _inprocServices.RequestServiceAsync(WellKnownServiceHubServices.SnapshotService, cancellationToken).ConfigureAwait(false);
 
             // get stream from service hub to communicate service specific information
             // this is what consumer actually use to communicate information
             var serviceStream = await _inprocServices.RequestServiceAsync(serviceName, cancellationToken).ConfigureAwait(false);
 
-            return await JsonRpcSession.CreateAsync(snapshot, callbackTarget, serviceStream, snapshotStream, cancellationToken).ConfigureAwait(false);
+            return await JsonRpcSession.CreateAsync(getSnapshotAsync, callbackTarget, serviceStream, snapshotStream, cancellationToken).ConfigureAwait(false);
         }
 
         protected override void OnConnected()

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

@@ -7,6 +7,7 @@
 using System.Threading.Tasks;
 using StreamJsonRpc;
 using Microsoft.CodeAnalysis.Remote;
+using Roslyn.Utilities;
 
 namespace Microsoft.VisualStudio.LanguageServices.Remote
 {
@@ -22,6 +23,8 @@ internal class JsonRpcClient : IDisposable
         public JsonRpcClient(
             Stream stream, object callbackTarget, bool useThisAsCallback, CancellationToken cancellationToken)
         {
+            Contract.Requires(stream != null);
+
             var target = useThisAsCallback ? this : callbackTarget;
             _cancellationToken = cancellationToken;
 

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

@@ -31,15 +31,35 @@ internal class JsonRpcSession : RemoteHostClient.Session
         private readonly CancellationTokenRegistration _cancellationRegistration;
 
         public static async Task<JsonRpcSession> CreateAsync(
-            PinnedRemotableDataScope snapshot,
+            Optional<Func<CancellationToken, Task<PinnedRemotableDataScope>>> getSnapshotAsync,
             object callbackTarget,
             Stream serviceStream,
             Stream snapshotStreamOpt,
             CancellationToken cancellationToken)
         {
-            var session = new JsonRpcSession(snapshot, callbackTarget, serviceStream, snapshotStreamOpt, cancellationToken);
+            var snapshot = getSnapshotAsync.Value == null ? null : await getSnapshotAsync.Value(cancellationToken).ConfigureAwait(false);
 
-            await session.InitializeAsync().ConfigureAwait(false);
+            JsonRpcSession session;
+            try
+            {
+                session = new JsonRpcSession(snapshot, callbackTarget, serviceStream, snapshotStreamOpt, cancellationToken);
+            }
+            catch
+            {
+                snapshot?.Dispose();
+                throw;
+            }
+
+            try
+            {
+                await session.InitializeAsync().ConfigureAwait(false);
+            }
+            catch when (!cancellationToken.IsCancellationRequested)
+            {
+                // The session disposes of itself when cancellation is requested.
+                session.Dispose();
+                throw;
+            }
 
             return session;
         }
@@ -52,6 +72,8 @@ internal class JsonRpcSession : RemoteHostClient.Session
             CancellationToken cancellationToken) :
             base(snapshot, cancellationToken)
         {
+            Contract.Requires((snapshot == null) == (snapshotStreamOpt == null));
+
             // get session id
             _currentSessionId = Interlocked.Increment(ref s_sessionId);
 

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

@@ -5,6 +5,7 @@
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Execution;
@@ -114,17 +115,17 @@ private static async Task RegisterWorkspaceHostAsync(Workspace workspace, Remote
             _rpc.StartListening();
         }
 
-        protected override async Task<Session> TryCreateServiceSessionAsync(string serviceName, PinnedRemotableDataScope snapshot, object callbackTarget, CancellationToken cancellationToken)
+        protected override async Task<Session> TryCreateServiceSessionAsync(string serviceName, Optional<Func<CancellationToken, Task<PinnedRemotableDataScope>>> getSnapshotAsync, object callbackTarget, CancellationToken cancellationToken)
         {
             // get stream from service hub to communicate snapshot/asset related information
             // this is the back channel the system uses to move data between VS and remote host for solution related information
-            var snapshotStream = snapshot == null ? null : await RequestServiceAsync(_hubClient, WellKnownServiceHubServices.SnapshotService, _hostGroup, _timeout, cancellationToken).ConfigureAwait(false);
+            var snapshotStream = getSnapshotAsync.Value == null ? null : await RequestServiceAsync(_hubClient, WellKnownServiceHubServices.SnapshotService, _hostGroup, _timeout, cancellationToken).ConfigureAwait(false);
 
             // 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 await JsonRpcSession.CreateAsync(snapshot, callbackTarget, serviceStream, snapshotStream, cancellationToken).ConfigureAwait(false);
+            return await JsonRpcSession.CreateAsync(getSnapshotAsync, callbackTarget, serviceStream, snapshotStream, cancellationToken).ConfigureAwait(false);
         }
 
         protected override void OnConnected()

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpEncapsulateField.cs

@@ -30,7 +30,7 @@ static void Main(string[] args)
     }
 }";
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/18879"), Trait(Traits.Feature, Traits.Features.EncapsulateField)]
+        [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)]
         public void EncapsulateThroughCommand()
         {
             SetUpEditor(TestSource);

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpNavigateTo.cs

@@ -20,7 +20,7 @@ public CSharpNavigateTo(VisualStudioInstanceFactory instanceFactory)
         {
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/18870"), Trait(Traits.Feature, Traits.Features.SignatureHelp)]
+        [Fact, Trait(Traits.Feature, Traits.Features.SignatureHelp)]
         public void NavigateTo()
         {
             var project = new ProjectUtils.Project(ProjectName);

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpReplIntellisense.cs

@@ -61,7 +61,7 @@ public void VerifySharpLoadCompletionList()
             VisualStudio.InteractiveWindow.Verify.CompletionItemsExist("C:");
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/18877")]
+        [Fact]
         public void VerifyNoCrashOnEnter()
         {
             VisualStudio.Workspace.SetUseSuggestionMode(false);

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpSendToInteractive.cs

@@ -214,7 +214,7 @@ public void AddAssemblyReferenceAndTypesToInteractive()
             VisualStudio.Workspace.WaitForAsyncOperations(FeatureAttribute.SolutionCrawler);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/18880")]
+        [Fact]
         public void ResetInteractiveFromProjectAndVerify()
         {
             var assembly = new ProjectUtils.AssemblyReference("System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089");

src/VisualStudio/IntegrationTest/IntegrationTests/VisualBasic/BasicNavigateTo.cs

@@ -18,7 +18,7 @@ public BasicNavigateTo(VisualStudioInstanceFactory instanceFactory)
         {
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/18870"), Trait(Traits.Feature, Traits.Features.SignatureHelp)]
+        [Fact, Trait(Traits.Feature, Traits.Features.SignatureHelp)]
         public void NavigateTo()
         {
             var project = new ProjectUtils.Project(ProjectName);

src/Workspaces/Core/Portable/Execution/PinnedRemotableDataScope.cs

@@ -12,10 +12,11 @@ namespace Microsoft.CodeAnalysis.Execution
     /// <summary>
     /// checksum scope that one can use to pin assets in memory while working on remote host
     /// </summary>
-    internal class PinnedRemotableDataScope : IDisposable
+    internal sealed class PinnedRemotableDataScope : IDisposable
     {
         private readonly AssetStorages _storages;
         private readonly AssetStorages.Storage _storage;
+        private bool _disposed;
 
         public readonly Checksum SolutionChecksum;
 
@@ -63,7 +64,12 @@ public RemotableData GetRemotableData(Checksum checksum, CancellationToken cance
 
         public void Dispose()
         {
-            _storages.UnregisterSnapshot(this);
+            if (!_disposed)
+            {
+                _disposed = true;
+                _storages.UnregisterSnapshot(this);
+            }
+
             GC.SuppressFinalize(this);
         }
 

src/Workspaces/Core/Portable/FindSymbols/Declarations/DeclarationFinder_AllDeclarations.cs

@@ -93,18 +93,20 @@ internal static partial class DeclarationFinder
         private static async Task<(bool, ImmutableArray<SymbolAndProjectId>)> TryFindAllDeclarationsWithNormalQueryInRemoteProcessAsync(
             Project project, SearchQuery query, SymbolFilter criteria, CancellationToken cancellationToken)
         {
-            var session = await SymbolFinder.TryGetRemoteSessionAsync(
-                project.Solution, cancellationToken).ConfigureAwait(false);
-            if (session != null)
+            using (var session = await SymbolFinder.TryGetRemoteSessionAsync(
+                project.Solution, cancellationToken).ConfigureAwait(false))
             {
-                var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
-                    nameof(IRemoteSymbolFinder.FindAllDeclarationsWithNormalQueryAsync),
-                    project.Id, query.Name, query.Kind, criteria).ConfigureAwait(false);
+                if (session != null)
+                {
+                    var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
+                        nameof(IRemoteSymbolFinder.FindAllDeclarationsWithNormalQueryAsync),
+                        project.Id, query.Name, query.Kind, criteria).ConfigureAwait(false);
 
-                var rehydrated = await RehydrateAsync(
-                    project.Solution, result, cancellationToken).ConfigureAwait(false);
+                    var rehydrated = await RehydrateAsync(
+                        project.Solution, result, cancellationToken).ConfigureAwait(false);
 
-                return (true, rehydrated);
+                    return (true, rehydrated);
+                }
             }
 
             return (false, ImmutableArray<SymbolAndProjectId>.Empty);

src/Workspaces/Core/Portable/FindSymbols/Declarations/DeclarationFinder_SourceDeclarations.cs

@@ -118,17 +118,19 @@ internal static partial class DeclarationFinder
         private static async Task<(bool, ImmutableArray<SymbolAndProjectId>)> TryFindSourceDeclarationsWithNormalQueryInRemoteProcessAsync(
             Solution solution, string name, bool ignoreCase, SymbolFilter criteria, CancellationToken cancellationToken)
         {
-            var session = await SymbolFinder.TryGetRemoteSessionAsync(solution, cancellationToken).ConfigureAwait(false);
-            if (session != null)
+            using (var session = await SymbolFinder.TryGetRemoteSessionAsync(solution, cancellationToken).ConfigureAwait(false))
             {
-                var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
-                    nameof(IRemoteSymbolFinder.FindSolutionSourceDeclarationsWithNormalQueryAsync),
-                    name, ignoreCase, criteria).ConfigureAwait(false);
+                if (session != null)
+                {
+                    var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
+                        nameof(IRemoteSymbolFinder.FindSolutionSourceDeclarationsWithNormalQueryAsync),
+                        name, ignoreCase, criteria).ConfigureAwait(false);
 
-                var rehydrated = await RehydrateAsync(
-                    solution, result, cancellationToken).ConfigureAwait(false);
+                    var rehydrated = await RehydrateAsync(
+                        solution, result, cancellationToken).ConfigureAwait(false);
 
-                return (true, rehydrated);
+                    return (true, rehydrated);
+                }
             }
 
             return (false, ImmutableArray<SymbolAndProjectId>.Empty);
@@ -137,17 +139,19 @@ internal static partial class DeclarationFinder
         private static async Task<(bool, ImmutableArray<SymbolAndProjectId>)> TryFindSourceDeclarationsWithNormalQueryInRemoteProcessAsync(
             Project project, string name, bool ignoreCase, SymbolFilter criteria, CancellationToken cancellationToken)
         {
-            var session = await SymbolFinder.TryGetRemoteSessionAsync(project.Solution, cancellationToken).ConfigureAwait(false);
-            if (session != null)
+            using (var session = await SymbolFinder.TryGetRemoteSessionAsync(project.Solution, cancellationToken).ConfigureAwait(false))
             {
-                var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
-                    nameof(IRemoteSymbolFinder.FindProjectSourceDeclarationsWithNormalQueryAsync),
-                    project.Id, name, ignoreCase, criteria).ConfigureAwait(false);
+                if (session != null)
+                {
+                    var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
+                        nameof(IRemoteSymbolFinder.FindProjectSourceDeclarationsWithNormalQueryAsync),
+                        project.Id, name, ignoreCase, criteria).ConfigureAwait(false);
 
-                var rehydrated = await RehydrateAsync(
-                    project.Solution, result, cancellationToken).ConfigureAwait(false);
+                    var rehydrated = await RehydrateAsync(
+                        project.Solution, result, cancellationToken).ConfigureAwait(false);
 
-                return (true, rehydrated);
+                    return (true, rehydrated);
+                }
             }
 
             return (false, ImmutableArray<SymbolAndProjectId>.Empty);
@@ -156,17 +160,19 @@ internal static partial class DeclarationFinder
         private static async Task<(bool, ImmutableArray<SymbolAndProjectId>)> TryFindSourceDeclarationsWithPatternInRemoteProcessAsync(
             Project project, string pattern, SymbolFilter criteria, CancellationToken cancellationToken)
         {
-            var session = await SymbolFinder.TryGetRemoteSessionAsync(project.Solution, cancellationToken).ConfigureAwait(false);
-            if (session != null)
+            using (var session = await SymbolFinder.TryGetRemoteSessionAsync(project.Solution, cancellationToken).ConfigureAwait(false))
             {
-                var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
-                    nameof(IRemoteSymbolFinder.FindProjectSourceDeclarationsWithPatternAsync),
-                    project.Id, pattern, criteria).ConfigureAwait(false);
+                if (session != null)
+                {
+                    var result = await session.InvokeAsync<SerializableSymbolAndProjectId[]>(
+                        nameof(IRemoteSymbolFinder.FindProjectSourceDeclarationsWithPatternAsync),
+                        project.Id, pattern, criteria).ConfigureAwait(false);
 
-                var rehydrated = await RehydrateAsync(
-                    project.Solution, result, cancellationToken).ConfigureAwait(false);
+                    var rehydrated = await RehydrateAsync(
+                        project.Solution, result, cancellationToken).ConfigureAwait(false);
 
-                return (true, rehydrated);
+                    return (true, rehydrated);
+                }
             }
 
             return (false, ImmutableArray<SymbolAndProjectId>.Empty);

src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs

@@ -63,7 +63,7 @@ internal sealed partial class PatternMatcher : IDisposable
             {
                 // PERF: Avoid string.Split allocations when the pattern doesn't contain a dot.
                 _dotSeparatedPatternSegments = pattern.Length > 0
-                    ? new PatternSegment[1] { _fullPatternSegment }
+                    ? new PatternSegment[1] { new PatternSegment(pattern.Trim(), allowFuzzyMatching) }
                     : Array.Empty<PatternSegment>();
             }
             else

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

@@ -24,30 +24,6 @@ protected RemoteHostClient(Workspace workspace)
 
         public event EventHandler<bool> ConnectionChanged;
 
-        [Obsolete("use TryCreateServiceSessionAsync instead")]
-        public Task<Session> CreateServiceSessionAsync(string serviceName, CancellationToken cancellationToken)
-        {
-            return CreateServiceSessionAsync(serviceName, callbackTarget: null, cancellationToken: cancellationToken);
-        }
-
-        [Obsolete("use TryCreateServiceSessionAsync instead")]
-        public Task<Session> CreateServiceSessionAsync(string serviceName, object callbackTarget, CancellationToken cancellationToken)
-        {
-            return TryCreateServiceSessionAsync(serviceName, snapshot: null, callbackTarget: callbackTarget, cancellationToken: cancellationToken);
-        }
-
-        [Obsolete("use TryCreateServiceSessionAsync instead")]
-        public Task<Session> CreateServiceSessionAsync(string serviceName, Solution solution, CancellationToken cancellationToken)
-        {
-            return CreateServiceSessionAsync(serviceName, solution, callbackTarget: null, cancellationToken: cancellationToken);
-        }
-
-        [Obsolete("use TryCreateServiceSessionAsync instead")]
-        public Task<Session> CreateServiceSessionAsync(string serviceName, Solution solution, object callbackTarget, CancellationToken cancellationToken)
-        {
-            return TryCreateServiceSessionAsync(serviceName, solution, callbackTarget, cancellationToken);
-        }
-
         /// <summary>
         /// Create <see cref="RemoteHostClient.Session"/> for the <paramref name="serviceName"/> if possible.
         /// otherwise, return null.
@@ -69,7 +45,7 @@ public Task<Session> TryCreateServiceSessionAsync(string serviceName, Cancellati
         /// </summary>
         public Task<Session> TryCreateServiceSessionAsync(string serviceName, object callbackTarget, CancellationToken cancellationToken)
         {
-            return TryCreateServiceSessionAsync(serviceName, snapshot: null, callbackTarget: callbackTarget, cancellationToken: cancellationToken);
+            return TryCreateServiceSessionAsync(serviceName, getSnapshotAsync: null, callbackTarget: callbackTarget, cancellationToken: cancellationToken);
         }
 
         /// <summary>
@@ -93,26 +69,15 @@ public Task<Session> TryCreateServiceSessionAsync(string serviceName, Solution s
         /// </summary>
         public async Task<Session> TryCreateServiceSessionAsync(string serviceName, Solution solution, object callbackTarget, CancellationToken cancellationToken)
         {
-            var snapshot = await GetPinnedScopeAsync(solution, cancellationToken).ConfigureAwait(false);
-            return await TryCreateServiceSessionAsync(serviceName, snapshot, callbackTarget, cancellationToken).ConfigureAwait(false);
+            Func<CancellationToken, Task<PinnedRemotableDataScope>> getSnapshotAsync = ct => GetPinnedScopeAsync(solution, ct);
+            return await TryCreateServiceSessionAsync(serviceName, getSnapshotAsync, callbackTarget, cancellationToken).ConfigureAwait(false);
         }
 
         protected abstract void OnConnected();
 
         protected abstract void OnDisconnected();
 
-        [Obsolete]
-        protected virtual Task<Session> CreateServiceSessionAsync(string serviceName, PinnedRemotableDataScope snapshot, object callbackTarget, CancellationToken cancellationToken)
-        {
-            return SpecializedTasks.Default<Session>();
-        }
-
-        protected virtual Task<Session> TryCreateServiceSessionAsync(string serviceName, PinnedRemotableDataScope snapshot, object callbackTarget, CancellationToken cancellationToken)
-        {
-#pragma warning disable CS0612 // leave it for now to not break backward compatibility
-            return CreateServiceSessionAsync(serviceName, snapshot, callbackTarget, cancellationToken);
-#pragma warning restore CS0612 // Type or member is obsolete
-        }
+        protected abstract Task<Session> TryCreateServiceSessionAsync(string serviceName, Optional<Func<CancellationToken, Task<PinnedRemotableDataScope>>> getSnapshotAsync, object callbackTarget, CancellationToken cancellationToken);
 
         internal void Shutdown()
         {
@@ -210,7 +175,7 @@ public class NoOpClient : RemoteHostClient
             }
 
             protected override Task<Session> TryCreateServiceSessionAsync(
-                string serviceName, PinnedRemotableDataScope snapshot, object callbackTarget, CancellationToken cancellationToken)
+                string serviceName, Optional<Func<CancellationToken, Task<PinnedRemotableDataScope>>> getSnapshotAsync, object callbackTarget, CancellationToken cancellationToken)
             {
                 return SpecializedTasks.Default<Session>();
             }