Some more fixes to workaround the Roslyn deadlock during project close

See dotnet/roslyn#14479 for more details.
mavasani · Oct 18, 2016 · 08f5cfe · 08f5cfe
1 parent ed68cf1
commit 08f5cfe
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 28 deletions.
diff --git a/...ojectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/UnconfiguredProjectHostObject.cs b/...ojectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/UnconfiguredProjectHostObject.cs
@@ -4,7 +4,6 @@
 using System.Collections.Generic;
 using Microsoft.VisualStudio.ProjectSystem.LanguageServices;
 using Microsoft.VisualStudio.Shell.Interop;
-using Task = System.Threading.Tasks.Task;
 
 namespace Microsoft.VisualStudio.ProjectSystem.VS.LanguageServices
 {
@@ -15,18 +14,33 @@ internal sealed class UnconfiguredProjectHostObject : AbstractHostObject, IUncon
     {
         private readonly IVsHierarchy _innerHierarchy;
         private readonly Dictionary<uint, IVsHierarchyEvents> _hierEventSinks;
+        private readonly HashSet<uint> _pendingItemIds;
 
         public UnconfiguredProjectHostObject(UnconfiguredProject unconfiguredProject)
         {
             Requires.NotNull(unconfiguredProject, nameof(unconfiguredProject));
 
             _innerHierarchy = (IVsHierarchy)unconfiguredProject.Services.HostObject;
             _hierEventSinks = new Dictionary<uint, IVsHierarchyEvents>();
+            _pendingItemIds = new HashSet<uint>();
         }
 
         protected override IVsHierarchy InnerHierarchy => _innerHierarchy;
         public IConfiguredProjectHostObject ActiveIntellisenseProjectHostObject { get; set; }
         public override String ActiveIntellisenseProjectDisplayName => ActiveIntellisenseProjectHostObject?.ProjectDisplayName;
+        public bool DisposingConfiguredProjectHostObjects { get; set; }
+
+        public void PushPendingIntellisenseProjectHostObjectUpdates()
+        {
+            Requires.Range(!DisposingConfiguredProjectHostObjects, nameof(DisposingConfiguredProjectHostObjects));
+
+            foreach (var itemId in _pendingItemIds)
+            {
+                OnPropertyChanged(itemId, (int)__VSHPROPID7.VSHPROPID_SharedItemContextHierarchy);
+            }
+
+            _pendingItemIds.Clear();
+        }
 
         #region IVsHierarchy overrides
 
@@ -65,12 +79,8 @@ public override int SetProperty(uint itemid, int propid, Object var)
             switch (propid)
             {
                 case (int)__VSHPROPID7.VSHPROPID_SharedItemContextHierarchy:
-                    var newActiveIntellisenseProjectHostObject = var as IConfiguredProjectHostObject;
-                    if (newActiveIntellisenseProjectHostObject != ActiveIntellisenseProjectHostObject)
-                    {
-                        ActiveIntellisenseProjectHostObject = newActiveIntellisenseProjectHostObject;
-                        OnPropertyChanged(itemid, propid);
-                    }
+                    ActiveIntellisenseProjectHostObject = var as IConfiguredProjectHostObject;
+                    OnPropertyChanged(itemid, propid);
                     return VSConstants.S_OK;
 
                 default:
@@ -86,6 +96,12 @@ public override int UnadviseHierarchyEvents(uint dwCookie)
 
         private void OnPropertyChanged(uint itemid, int propid, uint flags = 0)
         {
+            if (DisposingConfiguredProjectHostObjects)
+            {
+                _pendingItemIds.Add(itemid);
+                return;
+            }
+
             foreach (var eventSinkKvp in _hierEventSinks)
             {
                 eventSinkKvp.Value.OnPropertyChanged(itemid, propid, flags);

diff --git a/....ProjectSystem.Managed/ProjectSystem/LanguageServices/AggregateWorkspaceProjectContext.cs b/....ProjectSystem.Managed/ProjectSystem/LanguageServices/AggregateWorkspaceProjectContext.cs
@@ -126,8 +126,8 @@ public bool HasMatchingTargetFrameworks(string targetFrameworksProperty)
 
         public void Dispose(Func<IWorkspaceProjectContext, bool> shouldDisposeInnerContext)
         {
-            // Dispose the host object.
-            _unconfiguredProjectHostObject.Dispose();
+            // Workaround Roslyn deadlock https://github.com/dotnet/roslyn/issues/14479.
+            _unconfiguredProjectHostObject.DisposingConfiguredProjectHostObjects = true;
 
             // Dispose the inner project contexts.
             var disposedContexts = new List<IWorkspaceProjectContext>();
@@ -144,6 +144,9 @@ public void Dispose(Func<IWorkspaceProjectContext, bool> shouldDisposeInnerConte
             {
                 _disposedConfiguredProjectContexts.AddRange(disposedContexts);
             }
+
+            _unconfiguredProjectHostObject.DisposingConfiguredProjectHostObjects = false;
+            _unconfiguredProjectHostObject.PushPendingIntellisenseProjectHostObjectUpdates();
         }
     }
 }
diff --git a/...io.ProjectSystem.Managed/ProjectSystem/LanguageServices/IUnconfiguredProjectHostObject.cs b/...io.ProjectSystem.Managed/ProjectSystem/LanguageServices/IUnconfiguredProjectHostObject.cs
@@ -13,5 +13,18 @@ internal interface IUnconfiguredProjectHostObject : IDisposable
         /// <see cref="IConfiguredProjectHostObject"/> for the active intellisense project.
         /// </summary>
         IConfiguredProjectHostObject ActiveIntellisenseProjectHostObject { get; set; }
+
+        /// <summary>
+        /// Flag indicating that we are currently disposing inner configured projects.
+        /// </summary>
+        /// <remarks>This property is to workaround Roslyn deadlock https://github.com/dotnet/roslyn/issues/14479. </remarks>
+        bool DisposingConfiguredProjectHostObjects { get; set; }
+
+        /// <summary>
+        /// Pushes all the pending updates for active intellisense host objects while we were
+        /// disposing configured projects.
+        /// </summary>
+        /// <remarks>This method is to workaround Roslyn deadlock https://github.com/dotnet/roslyn/issues/14479. </remarks>
+        void PushPendingIntellisenseProjectHostObjectUpdates();
     }
 }
diff --git a/....VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/LanguageServiceHost.cs b/....VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/LanguageServiceHost.cs
@@ -43,7 +43,7 @@ public LanguageServiceHost(IUnconfiguredProjectCommonServices commonServices,
             Requires.NotNull(contextProvider, nameof(contextProvider));
             Requires.NotNull(tasksService, nameof(tasksService));
             Requires.NotNull(activeConfiguredProjectSubscriptionService, nameof(activeConfiguredProjectSubscriptionService));
-            
+
             _commonServices = commonServices;
             _contextProvider = contextProvider;
             _tasksService = tasksService;
@@ -54,20 +54,9 @@ public LanguageServiceHost(IUnconfiguredProjectCommonServices commonServices,
             _designTimeBuildSubscriptionLinks = new List<IDisposable>();
         }
 
-        private AggregateWorkspaceProjectContext CurrentAggregateProjectContext
-        {
-            get
-            {
-                using (_gate.DisposableWait())
-                {
-                    return _currentAggregateProjectContext;
-                }
-            }
-        }
-
-        public object HostSpecificErrorReporter => CurrentAggregateProjectContext?.HostSpecificErrorReporter;
+        public object HostSpecificErrorReporter => _currentAggregateProjectContext?.HostSpecificErrorReporter;
 
-        public IWorkspaceProjectContext ActiveProjectContext => CurrentAggregateProjectContext?.ActiveProjectContext;
+        public IWorkspaceProjectContext ActiveProjectContext => _currentAggregateProjectContext?.ActiveProjectContext;
 
         [ImportMany]
         public OrderPrecedenceImportCollection<ILanguageServiceRuleHandler> Handlers
@@ -134,7 +123,7 @@ private async Task OnProjectChangedCoreAsync(IProjectVersionedValue<IProjectSubs
 
         private async Task UpdateProjectContextAndSubscriptionsAsync()
         {
-            var previousProjectContext = this.CurrentAggregateProjectContext;
+            var previousProjectContext = _currentAggregateProjectContext;
             var newProjectContext = await UpdateProjectContextAsync().ConfigureAwait(false);
             if (previousProjectContext != newProjectContext)
             {

diff --git a/...rojectSystem.Managed/ProjectSystem/LanguageServices/UnconfiguredProjectContextProvider.cs b/...rojectSystem.Managed/ProjectSystem/LanguageServices/UnconfiguredProjectContextProvider.cs
@@ -27,6 +27,7 @@ internal partial class UnconfiguredProjectContextProvider : OnceInitializedOnceD
         private readonly List<AggregateWorkspaceProjectContext> _contexts = new List<AggregateWorkspaceProjectContext>();
         private readonly IProjectHostProvider _projectHostProvider;
         private readonly ActiveConfiguredProjectsProvider _activeConfiguredProjectsProvider;
+        private readonly IUnconfiguredProjectHostObject _unconfiguredProjectHostObject;
         private readonly Dictionary<ConfiguredProject, IWorkspaceProjectContext> _configuredProjectContextsMap = new Dictionary<ConfiguredProject, IWorkspaceProjectContext>();
         private readonly Dictionary<ConfiguredProject, IConfiguredProjectHostObject> _configuredProjectHostObjectsMap = new Dictionary<ConfiguredProject, IConfiguredProjectHostObject>();
 
@@ -51,6 +52,8 @@ public UnconfiguredProjectContextProvider(IUnconfiguredProjectCommonServices com
             _taskScheduler = taskScheduler;
             _projectHostProvider = projectHostProvider;
             _activeConfiguredProjectsProvider = activeConfiguredProjectsProvider;
+
+            _unconfiguredProjectHostObject = _projectHostProvider.GetUnconfiguredProjectHostObject(_commonServices.Project);
         }
 
         public async Task<AggregateWorkspaceProjectContext> CreateProjectContextAsync()
@@ -135,6 +138,7 @@ protected override void Initialize()
         protected override void Dispose(bool disposing)
         {
             _commonServices.Project.ProjectRenamed -= OnProjectRenamed;
+            _unconfiguredProjectHostObject.Dispose();
         }
 
         private Task OnProjectRenamed(object sender, ProjectRenamedEventArgs args)
@@ -242,7 +246,6 @@ private async Task<AggregateWorkspaceProjectContext> CreateProjectContextAsyncCo
                 var configuredProjectsMap = await _activeConfiguredProjectsProvider.GetActiveConfiguredProjectsMapAsync().ConfigureAwait(true);
 
                 // Get the unconfigured project host object (shared host object).
-                IUnconfiguredProjectHostObject unconfiguredProjectHostObject = _projectHostProvider.GetUnconfiguredProjectHostObject(_commonServices.Project);
                 var configuredProjectsToRemove = new HashSet<ConfiguredProject>(_configuredProjectHostObjectsMap.Keys);
                 var activeProjectConfiguration = _commonServices.ActiveConfiguredProject.ProjectConfiguration;
 
@@ -265,7 +268,7 @@ private async Task<AggregateWorkspaceProjectContext> CreateProjectContextAsyncCo
                         targetPath = (string)await configurationGeneralProperties.TargetPath.GetValueAsync().ConfigureAwait(true);
                         targetPath = NormalizeTargetPath(targetPath, projectData);
                         var displayName = GetDisplayName(configuredProject, projectData, targetFramework);
-                        configuredProjectHostObject = _projectHostProvider.GetConfiguredProjectHostObject(unconfiguredProjectHostObject, displayName);
+                        configuredProjectHostObject = _projectHostProvider.GetConfiguredProjectHostObject(_unconfiguredProjectHostObject, displayName);
 
                         // TODO: https://github.com/dotnet/roslyn-project-system/issues/353
                         await _commonServices.ThreadingService.SwitchToUIThread();
@@ -283,9 +286,9 @@ private async Task<AggregateWorkspaceProjectContext> CreateProjectContextAsyncCo
                     }
                 }
 
-                unconfiguredProjectHostObject.ActiveIntellisenseProjectHostObject = activeIntellisenseProjectHostObject;
+                _unconfiguredProjectHostObject.ActiveIntellisenseProjectHostObject = activeIntellisenseProjectHostObject;
 
-                return new AggregateWorkspaceProjectContext(innerProjectContextsBuilder.ToImmutable(), configuredProjectsMap, activeTargetFramework, unconfiguredProjectHostObject);
+                return new AggregateWorkspaceProjectContext(innerProjectContextsBuilder.ToImmutable(), configuredProjectsMap, activeTargetFramework, _unconfiguredProjectHostObject);
             });
         }