Remove GetService(...) calls from VS MEF importing constructors (#10168)

We observed a hang during solution load because
`VisualStudioEditorDocumentManager` retrieves the
`IVsRunningDocumentTable` service in its constructor. When this
constructor is called on a background thread, it results in an implicit
marshal to the UI thread in order to retrieve the service and QI for its
COM interface. This change fixes that issue delaying the request for
`IVsRunningDocumentTable` until it is needed and running on the UI
thread.

In addition, `VisualStudioFileChangeTrackerFactory` had a similar issue
but with `IVsAsyncFileChangeEx`. Because `IFileChangeTrackerFactory` is
imported by `VisualStudioEditorDocumentManager`, I've fixed this service
as well.
This commit is contained in:
Dustin Campbell 2024-03-26 12:20:56 -07:00 коммит произвёл GitHub
Родитель cd71979cc4 7a202966e1
Коммит a7c635f6de
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
2 изменённых файлов: 21 добавлений и 9 удалений

Просмотреть файл

@ -4,6 +4,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.ComponentModel.Composition; using System.ComponentModel.Composition;
using System.Diagnostics.CodeAnalysis;
using System.Linq; using System.Linq;
using System.Runtime.InteropServices; using System.Runtime.InteropServices;
using Microsoft.AspNetCore.Razor; using Microsoft.AspNetCore.Razor;
@ -28,11 +29,12 @@ internal sealed class VisualStudioEditorDocumentManager(
ProjectSnapshotManagerDispatcher dispatcher, ProjectSnapshotManagerDispatcher dispatcher,
JoinableTaskContext joinableTaskContext) : EditorDocumentManager(fileChangeTrackerFactory, dispatcher, joinableTaskContext) JoinableTaskContext joinableTaskContext) : EditorDocumentManager(fileChangeTrackerFactory, dispatcher, joinableTaskContext)
{ {
private readonly IVsRunningDocumentTable4 _runningDocumentTable = serviceProvider.GetService<SVsRunningDocumentTable, IVsRunningDocumentTable4>(throwOnFailure: true).AssumeNotNull(); private readonly IServiceProvider _serviceProvider = serviceProvider;
private readonly IVsEditorAdaptersFactoryService _editorAdaptersFactory = editorAdaptersFactory; private readonly IVsEditorAdaptersFactoryService _editorAdaptersFactory = editorAdaptersFactory;
private readonly Dictionary<uint, List<DocumentKey>> _documentsByCookie = []; private readonly Dictionary<uint, List<DocumentKey>> _documentsByCookie = [];
private readonly Dictionary<DocumentKey, uint> _cookiesByDocument = []; private readonly Dictionary<DocumentKey, uint> _cookiesByDocument = [];
private IVsRunningDocumentTable4? _runningDocumentTable;
private bool _advised; private bool _advised;
protected override ITextBuffer? GetTextBufferForOpenDocument(string filePath) protected override ITextBuffer? GetTextBufferForOpenDocument(string filePath)
@ -219,10 +221,15 @@ internal sealed class VisualStudioEditorDocumentManager(
} }
} }
[MemberNotNull(nameof(_runningDocumentTable))]
private void EnsureDocumentTableAdvised() private void EnsureDocumentTableAdvised()
{ {
JoinableTaskContext.AssertUIThread(); JoinableTaskContext.AssertUIThread();
// Note: Because it is a COM interface, we defer retrieving IVsRunningDocumentTable until
// now to avoid implicitly marshalling to the UI thread, which can deadlock.
_runningDocumentTable ??= _serviceProvider.GetService<SVsRunningDocumentTable, IVsRunningDocumentTable4>(throwOnFailure: true).AssumeNotNull();
if (!_advised) if (!_advised)
{ {
_advised = true; _advised = true;

Просмотреть файл

@ -13,22 +13,24 @@ namespace Microsoft.VisualStudio.Editor.Razor.Documents;
[Export(typeof(IFileChangeTrackerFactory))] [Export(typeof(IFileChangeTrackerFactory))]
internal class VisualStudioFileChangeTrackerFactory : IFileChangeTrackerFactory internal class VisualStudioFileChangeTrackerFactory : IFileChangeTrackerFactory
{ {
private readonly IErrorReporter _errorReporter; private readonly ProjectSnapshotManagerDispatcher _dispatcher;
private readonly IVsAsyncFileChangeEx _fileChangeService;
private readonly ProjectSnapshotManagerDispatcher _projectSnapshotManagerDispatcher;
private readonly JoinableTaskContext _joinableTaskContext; private readonly JoinableTaskContext _joinableTaskContext;
private readonly IErrorReporter _errorReporter;
private readonly JoinableTask<IVsAsyncFileChangeEx> _getFileChangeServiceTask;
[ImportingConstructor] [ImportingConstructor]
public VisualStudioFileChangeTrackerFactory( public VisualStudioFileChangeTrackerFactory(
SVsServiceProvider serviceProvider, [Import(typeof(SAsyncServiceProvider))] IAsyncServiceProvider serviceProvider,
JoinableTaskContext joinableTaskContext, JoinableTaskContext joinableTaskContext,
ProjectSnapshotManagerDispatcher dispatcher, ProjectSnapshotManagerDispatcher dispatcher,
IErrorReporter errorReporter) IErrorReporter errorReporter)
{ {
_errorReporter = errorReporter; _dispatcher = dispatcher;
_fileChangeService = (IVsAsyncFileChangeEx)serviceProvider.GetService(typeof(SVsFileChangeEx));
_projectSnapshotManagerDispatcher = dispatcher;
_joinableTaskContext = joinableTaskContext; _joinableTaskContext = joinableTaskContext;
_errorReporter = errorReporter;
var jtf = _joinableTaskContext.Factory;
_getFileChangeServiceTask = jtf.RunAsync(serviceProvider.GetServiceAsync<SVsFileChangeEx, IVsAsyncFileChangeEx>);
} }
public IFileChangeTracker Create(string filePath) public IFileChangeTracker Create(string filePath)
@ -38,6 +40,9 @@ internal class VisualStudioFileChangeTrackerFactory : IFileChangeTrackerFactory
throw new ArgumentException(SR.ArgumentCannotBeNullOrEmpty, nameof(filePath)); throw new ArgumentException(SR.ArgumentCannotBeNullOrEmpty, nameof(filePath));
} }
return new VisualStudioFileChangeTracker(filePath, _errorReporter, _fileChangeService, _projectSnapshotManagerDispatcher, _joinableTaskContext); // TODO: Make IFileChangeTrackerFactory.Create(...) asynchronous to avoid blocking here.
var fileChangeService = _getFileChangeServiceTask.Join();
return new VisualStudioFileChangeTracker(filePath, _errorReporter, fileChangeService, _dispatcher, _joinableTaskContext);
} }
} }