From 1e1ff5200123761908c997c809dac3ea1b4a86c1 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 13 Mar 2019 19:51:16 -0400 Subject: [PATCH 1/2] Rename file to match our naming convention for partials --- ...Provider.Ide.cs => EditorConfigDocumentOptionsProvider_Ide.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/EditorFeatures/Core.Wpf/Options/{EditorConfigDocumentOptionsProvider.Ide.cs => EditorConfigDocumentOptionsProvider_Ide.cs} (100%) diff --git a/src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProvider.Ide.cs b/src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProvider_Ide.cs similarity index 100% rename from src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProvider.Ide.cs rename to src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProvider_Ide.cs From 8097ff9db4f2ff2f0e460fc70d996473783799a0 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 13 Mar 2019 21:43:52 -0400 Subject: [PATCH 2/2] Workaround a deadlock caused by watching .editorconfigs We use the CodingConventions library from Visual Studio that parses and processes .editorconfig files for us. This library under the covers uses the Visual Studio file watching service for watching files. In Visual Studio 2017 the file watching service made the guarantee that watching files could be done in a free-threaded manner in the background and wouldn't marshal. The .editorconfig library then passed this guarantee along to us. In Dev16, the file watching service was rewritten, and accidentally lost that guarantee in specific cases. Much of our interactions with the file change service also changed so it doesn't matter, but the .editorconfig library's use of the file watching service is causing deadlocks in some scenarios. This change is a workaround to ensure we don't start watching .editorconfig files on the background thread in places where we need to ensure we don't touch the UI thread. Ideally we'd be fixing the file change service itself to restore the guarantee, but that's too risky of a change for our current needs and isn't something we can do quickly enough to avoid customer issues. Since we're rewriting our .editorconfig handling entirely which will cause us to stop using the .editorconfig library (and by extension, it's use of this problematic file change service path), we can do this workaround now and let the problem go away on it's own soon when we simply delete all of this. Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/810494 --- ...torConfigDocumentOptionsProviderFactory.cs | 132 +++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProviderFactory.cs b/src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProviderFactory.cs index a0e8a3abb367d..2ea68b35e5636 100644 --- a/src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProviderFactory.cs +++ b/src/EditorFeatures/Core.Wpf/Options/EditorConfigDocumentOptionsProviderFactory.cs @@ -2,10 +2,13 @@ using System; using System.Composition; +using System.IO; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.VisualStudio.CodingConventions; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Editor.Options { @@ -13,21 +16,148 @@ namespace Microsoft.CodeAnalysis.Editor.Options class EditorConfigDocumentOptionsProviderFactory : IDocumentOptionsProviderFactory { private readonly ICodingConventionsManager _codingConventionsManager; + private readonly IFileWatcher _fileWatcher; private readonly IAsynchronousOperationListenerProvider _asynchronousOperationListenerProvider; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] public EditorConfigDocumentOptionsProviderFactory( ICodingConventionsManager codingConventionsManager, + IFileWatcher fileWatcher, IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider) { _codingConventionsManager = codingConventionsManager; + _fileWatcher = fileWatcher; _asynchronousOperationListenerProvider = asynchronousOperationListenerProvider; } public IDocumentOptionsProvider Create(Workspace workspace) { - return new EditorConfigDocumentOptionsProvider(workspace, _codingConventionsManager, _asynchronousOperationListenerProvider); + ICodingConventionsManager codingConventionsManager; + + if (workspace.Kind == WorkspaceKind.RemoteWorkspace) + { + // If it's the remote workspace, it's our own implementation of the file watcher which is already doesn't have + // UI thread dependencies. + codingConventionsManager = _codingConventionsManager; + } + else + { + // The default file watcher implementation inside Visual Studio accientally depends on the UI thread + // (sometimes!) when trying to add a watch to a file. This can cause us to deadlock, since our assumption is + // consumption of a coding convention can be done freely without having to use a JTF-friendly wait. + // So we'll wrap the standard file watcher with one that defers the file watches until later. + var deferredFileWatcher = new DeferredFileWatcher(_fileWatcher, _asynchronousOperationListenerProvider); + codingConventionsManager = CodingConventionsManagerFactory.CreateCodingConventionsManager(deferredFileWatcher); + } + + return new EditorConfigDocumentOptionsProvider(workspace, codingConventionsManager, _asynchronousOperationListenerProvider); + } + + /// + /// An implementation of that ensures we don't watch for a file synchronously to + /// avoid deadlocks. + /// + internal class DeferredFileWatcher : IFileWatcher + { + private readonly IFileWatcher _fileWatcher; + private readonly SimpleTaskQueue _taskQueue = new SimpleTaskQueue(TaskScheduler.Default); + private readonly IAsynchronousOperationListener _listener; + + public DeferredFileWatcher(IFileWatcher fileWatcher, IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider) + { + _fileWatcher = fileWatcher; + _fileWatcher.ConventionFileChanged += OnConventionFileChanged; + + _listener = asynchronousOperationListenerProvider.GetListener(FeatureAttribute.Workspace); + } + + private Task OnConventionFileChanged(object sender, ConventionsFileChangeEventArgs arg) + { + return ConventionFileChanged?.Invoke(this, arg) ?? Task.CompletedTask; + } + + public event ConventionsFileChangedAsyncEventHandler ConventionFileChanged; + + public event ContextFileMovedAsyncEventHandler ContextFileMoved + { + add + { + _fileWatcher.ContextFileMoved += value; + } + + remove + { + _fileWatcher.ContextFileMoved -= value; + } + } + + public void Dispose() + { + _fileWatcher.ConventionFileChanged -= OnConventionFileChanged; + _fileWatcher.Dispose(); + } + + public void StartWatching(string fileName, string directoryPath) + { + var asyncToken = _listener.BeginAsyncOperation(nameof(DeferredFileWatcher) + "." + nameof(StartWatching)); + + // Read the file time stamp right now; we want to know if it changes between now + // and our ability to get the file watcher in place. + var originalFileTimeStamp = TryGetFileTimeStamp(fileName, directoryPath); + _taskQueue.ScheduleTask(() => + { + _fileWatcher.StartWatching(fileName, directoryPath); + + var newFileTimeStamp = TryGetFileTimeStamp(fileName, directoryPath); + + if (originalFileTimeStamp != newFileTimeStamp) + { + ChangeType changeType; + + if (!originalFileTimeStamp.HasValue && newFileTimeStamp.HasValue) + { + changeType = ChangeType.FileCreated; + } + else if (originalFileTimeStamp.HasValue && !newFileTimeStamp.HasValue) + { + changeType = ChangeType.FileDeleted; + } + else + { + changeType = ChangeType.FileModified; + } + + ConventionFileChanged?.Invoke(this, + new ConventionsFileChangeEventArgs(fileName, directoryPath, changeType)); + } + }).CompletesAsyncOperation(asyncToken); + } + + private static DateTime? TryGetFileTimeStamp(string fileName, string directoryPath) + { + try + { + var fullFilePath = Path.Combine(directoryPath, fileName); + + // Avoid a first-chance exception if the file definitely doesn't exist + if (!File.Exists(fullFilePath)) + { + return null; + } + + return FileUtilities.GetFileTimeStamp(fullFilePath); + } + catch (IOException) + { + return null; + } + } + + public void StopWatching(string fileName, string directoryPath) + { + _taskQueue.ScheduleTask(() => _fileWatcher.StopWatching(fileName, directoryPath)); + } } } }