From 60cc9719c45aa3907d75fccf00f24bfff76a67a5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 13 May 2024 20:49:51 -0700 Subject: [PATCH 1/2] Properly dispose the SymbolSearchEngine --- .../SymbolSearchUpdateNoOpEngine.cs | 5 ++++ .../Windows/SymbolSearchUpdateEngine.cs | 5 ++++ .../VisualStudioSymbolSearchService.cs | 25 +++++++++++++------ .../SymbolSearch/ISymbolSearchUpdateEngine.cs | 3 ++- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Features/Core/Portable/SymbolSearch/SymbolSearchUpdateNoOpEngine.cs b/src/Features/Core/Portable/SymbolSearch/SymbolSearchUpdateNoOpEngine.cs index 1646c549e47cb..ea2a0b7184685 100644 --- a/src/Features/Core/Portable/SymbolSearch/SymbolSearchUpdateNoOpEngine.cs +++ b/src/Features/Core/Portable/SymbolSearch/SymbolSearchUpdateNoOpEngine.cs @@ -13,6 +13,11 @@ internal sealed class SymbolSearchUpdateNoOpEngine : ISymbolSearchUpdateEngine { public static readonly SymbolSearchUpdateNoOpEngine Instance = new(); + public void Dispose() + { + // Nothing to do for the no-op version. + } + public ValueTask> FindPackagesWithAssemblyAsync(string source, string assemblyName, CancellationToken cancellationToken) => ValueTaskFactory.FromResult(ImmutableArray.Empty); diff --git a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.cs b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.cs index d827000bfa327..e5db69773fdd4 100644 --- a/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.cs +++ b/src/Features/Core/Portable/SymbolSearch/Windows/SymbolSearchUpdateEngine.cs @@ -65,6 +65,11 @@ internal SymbolSearchUpdateEngine( _reportAndSwallowExceptionUnlessCanceled = reportAndSwallowExceptionUnlessCanceled; } + public void Dispose() + { + // Nothing to do for the core symbol search engine. + } + public ValueTask> FindPackagesWithTypeAsync( string source, string name, int arity, CancellationToken cancellationToken) { diff --git a/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs b/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs index a658f1d2e748b..be065a81685d3 100644 --- a/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs +++ b/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs @@ -2,13 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; -using System.IO; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -25,7 +22,6 @@ using Microsoft.VisualStudio.LanguageServices.Storage; using Microsoft.VisualStudio.Settings; using Microsoft.VisualStudio.Shell; -using Microsoft.VisualStudio.Shell.Interop; using Microsoft.VisualStudio.Shell.Settings; using Roslyn.Utilities; using VSShell = Microsoft.VisualStudio.Shell; @@ -40,18 +36,18 @@ namespace Microsoft.VisualStudio.LanguageServices.SymbolSearch; /// date by downloading patches on a daily basis. /// [ExportWorkspaceService(typeof(ISymbolSearchService), ServiceLayer.Host), Shared] -internal partial class VisualStudioSymbolSearchService : AbstractDelayStartedService, ISymbolSearchService +internal partial class VisualStudioSymbolSearchService : AbstractDelayStartedService, ISymbolSearchService, IDisposable { private readonly SemaphoreSlim _gate = new(initialCount: 1); // Note: A remote engine is disposable as it maintains a connection with ServiceHub, // but we want to keep it alive until the VS is closed, so we don't dispose it. - private ISymbolSearchUpdateEngine _lazyUpdateEngine; + private ISymbolSearchUpdateEngine? _lazyUpdateEngine; private readonly SVsServiceProvider _serviceProvider; private readonly IPackageInstallerService _installerService; - private string _localSettingsDirectory; + private string? _localSettingsDirectory; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] @@ -69,7 +65,20 @@ public VisualStudioSymbolSearchService( [SymbolSearchOptionsStorage.SearchReferenceAssemblies, SymbolSearchOptionsStorage.SearchNuGetPackages]) { _serviceProvider = serviceProvider; - _installerService = workspace.Services.GetService(); + _installerService = workspace.Services.GetRequiredService(); + } + + public void Dispose() + { + ISymbolSearchUpdateEngine? updateEngine; + using (_gate.DisposableWait()) + { + // Once we're disposed, swap out our engine with a no-op one so we don't try to do any more work. + updateEngine = _lazyUpdateEngine; + _lazyUpdateEngine = SymbolSearchUpdateNoOpEngine.Instance; + } + + updateEngine?.Dispose(); } protected override async Task EnableServiceAsync(CancellationToken cancellationToken) diff --git a/src/Workspaces/Core/Portable/SymbolSearch/ISymbolSearchUpdateEngine.cs b/src/Workspaces/Core/Portable/SymbolSearch/ISymbolSearchUpdateEngine.cs index 33a083cc0f12f..2c5f5529dff5d 100644 --- a/src/Workspaces/Core/Portable/SymbolSearch/ISymbolSearchUpdateEngine.cs +++ b/src/Workspaces/Core/Portable/SymbolSearch/ISymbolSearchUpdateEngine.cs @@ -4,6 +4,7 @@ #nullable disable +using System; using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; @@ -14,7 +15,7 @@ namespace Microsoft.CodeAnalysis.SymbolSearch; /// Service that allows you to query the SymbolSearch database and which keeps /// the database up to date. /// -internal interface ISymbolSearchUpdateEngine +internal interface ISymbolSearchUpdateEngine : IDisposable { ValueTask UpdateContinuouslyAsync(string sourceName, string localSettingsDirectory, CancellationToken cancellationToken); From 1a2f73fe0cd6b020d7aae0233f7d77316eb61284 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 14 May 2024 11:08:13 -0700 Subject: [PATCH 2/2] Move off UI thread --- .../VisualStudioSymbolSearchService.cs | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs b/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs index be065a81685d3..b29accb7d3325 100644 --- a/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs +++ b/src/VisualStudio/Core/Def/SymbolSearch/VisualStudioSymbolSearchService.cs @@ -23,6 +23,7 @@ using Microsoft.VisualStudio.Settings; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Settings; +using Microsoft.VisualStudio.Threading; using Roslyn.Utilities; using VSShell = Microsoft.VisualStudio.Shell; @@ -70,15 +71,27 @@ public VisualStudioSymbolSearchService( public void Dispose() { - ISymbolSearchUpdateEngine? updateEngine; - using (_gate.DisposableWait()) + // Once we're disposed, swap out our engine with a no-op one so we don't try to do any more work, and dispose of + // our connection to the OOP server so it can be cleaned up. + // + // Kick off a Task for this so we don't block MEF from proceeding (as it will be calling us on the UI thread). + _ = DisposeAsync(); + return; + + async Task DisposeAsync() { - // Once we're disposed, swap out our engine with a no-op one so we don't try to do any more work. - updateEngine = _lazyUpdateEngine; - _lazyUpdateEngine = SymbolSearchUpdateNoOpEngine.Instance; - } + // Make sure we get off the UI thread so that Dispose can return immediately. + await TaskScheduler.Default; - updateEngine?.Dispose(); + ISymbolSearchUpdateEngine? updateEngine; + using (await _gate.DisposableWaitAsync().ConfigureAwait(false)) + { + updateEngine = _lazyUpdateEngine; + _lazyUpdateEngine = SymbolSearchUpdateNoOpEngine.Instance; + } + + updateEngine?.Dispose(); + } } protected override async Task EnableServiceAsync(CancellationToken cancellationToken)