Skip to content

Commit

Permalink
Remove GetService(...) calls from VS MEF importing constructors (#10168)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DustinCampbell authored Mar 26, 2024
2 parents cd71979 + 7a20296 commit a7c635f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.Composition;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Runtime.InteropServices;
using Microsoft.AspNetCore.Razor;
Expand All @@ -28,11 +29,12 @@ internal sealed class VisualStudioEditorDocumentManager(
ProjectSnapshotManagerDispatcher dispatcher,
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 Dictionary<uint, List<DocumentKey>> _documentsByCookie = [];
private readonly Dictionary<DocumentKey, uint> _cookiesByDocument = [];
private IVsRunningDocumentTable4? _runningDocumentTable;
private bool _advised;

protected override ITextBuffer? GetTextBufferForOpenDocument(string filePath)
Expand Down Expand Up @@ -219,10 +221,15 @@ public void DocumentRenamed(uint cookie, string fromFilePath, string toFilePath)
}
}

[MemberNotNull(nameof(_runningDocumentTable))]
private void EnsureDocumentTableAdvised()
{
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)
{
_advised = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,24 @@ namespace Microsoft.VisualStudio.Editor.Razor.Documents;
[Export(typeof(IFileChangeTrackerFactory))]
internal class VisualStudioFileChangeTrackerFactory : IFileChangeTrackerFactory
{
private readonly IErrorReporter _errorReporter;
private readonly IVsAsyncFileChangeEx _fileChangeService;
private readonly ProjectSnapshotManagerDispatcher _projectSnapshotManagerDispatcher;
private readonly ProjectSnapshotManagerDispatcher _dispatcher;
private readonly JoinableTaskContext _joinableTaskContext;
private readonly IErrorReporter _errorReporter;
private readonly JoinableTask<IVsAsyncFileChangeEx> _getFileChangeServiceTask;

[ImportingConstructor]
public VisualStudioFileChangeTrackerFactory(
SVsServiceProvider serviceProvider,
[Import(typeof(SAsyncServiceProvider))] IAsyncServiceProvider serviceProvider,
JoinableTaskContext joinableTaskContext,
ProjectSnapshotManagerDispatcher dispatcher,
IErrorReporter errorReporter)
{
_errorReporter = errorReporter;
_fileChangeService = (IVsAsyncFileChangeEx)serviceProvider.GetService(typeof(SVsFileChangeEx));
_projectSnapshotManagerDispatcher = dispatcher;
_dispatcher = dispatcher;
_joinableTaskContext = joinableTaskContext;
_errorReporter = errorReporter;

var jtf = _joinableTaskContext.Factory;
_getFileChangeServiceTask = jtf.RunAsync(serviceProvider.GetServiceAsync<SVsFileChangeEx, IVsAsyncFileChangeEx>);
}

public IFileChangeTracker Create(string filePath)
Expand All @@ -38,6 +40,9 @@ public IFileChangeTracker Create(string 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);
}
}

0 comments on commit a7c635f

Please sign in to comment.