IWorkspaceProjectContextHost.Loaded -> PublishAsync

This fixes a deadlock when the UI thread is blocked on Loaded, such as during PrioritizedProjectLoad. This occured because MultiLifetimeComponent.Loaded was backed by a TaskCompletionSource that did not have a JoinableTask relationship to the code that completed it (MultiLifetimeComponent.LoadAsync). I sat down with Andrew Arnott came up with a design that creates the relationship. I've used the name Publish (mimicing ITreeService, etc) as this method is basically the precursor to dotnet#3425.
davkean · Nov 14, 2018 · fdd18d5 · fdd18d5
1 parent 98983ea
commit fdd18d5
Show file tree

Hide file tree

Showing 9 changed files with 156 additions and 105 deletions.
diff --git a/...sualStudio.ProjectSystem.Managed.UnitTests/Mocks/AbstractMultiLifetimeComponentFactory.cs b/...sualStudio.ProjectSystem.Managed.UnitTests/Mocks/AbstractMultiLifetimeComponentFactory.cs
@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.Threading;
 
@@ -14,14 +15,14 @@ public static MultiLifetimeComponent Create()
             return new MultiLifetimeComponent(joinableTaskContextNode);
         }
 
-        public class MultiLifetimeComponent : AbstractMultiLifetimeComponent<IMultiLifetimeInstance>
+        public class MultiLifetimeComponent : AbstractMultiLifetimeComponent<MultiLifetimeComponent.MultiLifetimeInstance>
         {
             public MultiLifetimeComponent(JoinableTaskContextNode joinableTaskContextNode) 
                 : base(joinableTaskContextNode)
             {
             }
 
-            protected override IMultiLifetimeInstance CreateInstance()
+            protected override MultiLifetimeInstance CreateInstance()
             {
                 return new MultiLifetimeInstance();
             }
@@ -31,6 +32,11 @@ protected override IMultiLifetimeInstance CreateInstance()
                 get { return base.IsInitialized; }
             }
 
+            public new Task<MultiLifetimeInstance> PublishInstanceAsync(CancellationToken cancellationToken = default)
+            {
+                return base.PublishInstanceAsync(cancellationToken);
+            }
+
             public class MultiLifetimeInstance : IMultiLifetimeInstance
             {
                 public bool IsInitialized

diff --git a/...udio.ProjectSystem.Managed.UnitTests/ProjectSystem/AbstractMultiLifetimeComponentTests.cs b/...udio.ProjectSystem.Managed.UnitTests/ProjectSystem/AbstractMultiLifetimeComponentTests.cs
@@ -3,64 +3,73 @@
 using System.Threading.Tasks;
 
 using Xunit;
-using static Microsoft.VisualStudio.ProjectSystem.AbstractMultiLifetimeComponentFactory.MultiLifetimeComponent;
 
 namespace Microsoft.VisualStudio.ProjectSystem
 {
     public class AbstractMultiLifetimeComponentTests
     {
         [Fact]
-        public void Loaded_WhenNotUnloaded_ReturnsNonCompletedTask()
+        public void PublishInstanceAsync_WhenNotUnPublishInstanceAsync_ReturnsNonCompletedTask()
         {
             var component = CreateInstance();
 
-            Assert.False(component.Loaded.IsCanceled);
-            Assert.False(component.Loaded.IsCompleted);
-            Assert.False(component.Loaded.IsFaulted);
+            var result = component.PublishInstanceAsync();
+
+            Assert.False(result.IsCanceled);
+            Assert.False(result.IsCompleted);
+            Assert.False(result.IsFaulted);
         }
 
         [Fact]
-        public async Task Loaded_WhenLoaded_ReturnsCompletedTask()
+        public async Task PublishInstanceAsync_WhenLoaded_ReturnsCompletedTask()
         {
             var component = CreateInstance();
 
             await component.LoadAsync();
 
-            Assert.True(component.Loaded.IsCompleted);
+            var result = component.PublishInstanceAsync();
+
+            Assert.True(result.IsCompleted);
         }
 
         [Fact]
-        public async Task Loaded_WhenUnloaded_ReturnsNonCompletedTask()
+        public async Task PublishInstanceAsync_WhenUnloaded_ReturnsNonCompletedTask()
         {
             var component = CreateInstance();
 
             await component.LoadAsync();
             await component.UnloadAsync();
 
-            Assert.False(component.Loaded.IsCanceled);
-            Assert.False(component.Loaded.IsCompleted);
-            Assert.False(component.Loaded.IsFaulted);
+            var result = component.PublishInstanceAsync();
+
+            Assert.False(result.IsCanceled);
+            Assert.False(result.IsCompleted);
+            Assert.False(result.IsFaulted);
         }
 
         [Fact]
-        public async Task Loaded_DisposedWhenUnloaded_ReturnsCancelledTask()
+        public async Task PublishInstanceAsync_DisposedWhenUnloaded_ReturnsCancelledTask()
         {
             var component = CreateInstance();
 
             await component.DisposeAsync();
 
-            Assert.True(component.Loaded.IsCanceled);
+            var result = component.PublishInstanceAsync();
+
+            Assert.True(result.IsCanceled);
         }
 
         [Fact]
-        public async Task Loaded_DisposedWhenLoaded_ReturnsCancelledTask()
+        public async Task PublishInstanceAsync_DisposedWhenLoaded_ReturnsCancelledTask()
         {
             var component = CreateInstance();
 
             await component.LoadAsync();
             await component.DisposeAsync();
 
-            Assert.True(component.Loaded.IsCanceled);
+            var result = component.PublishInstanceAsync();
+
+            Assert.True(result.IsCanceled);
         }
 
         [Fact]
@@ -80,7 +89,7 @@ public async Task LoadAsync_InitializesUnderlyingInstance()
 
             await component.LoadAsync();
 
-            var result = (MultiLifetimeInstance)component.Instance;
+            var result = await component.PublishInstanceAsync();
 
             Assert.True(result.IsInitialized);
         }
@@ -92,11 +101,13 @@ public async Task LoadAsync_WhenAlreadyLoaded_DoesNotCreateNewInstance()
 
             await component.LoadAsync();
 
-            var instance = component.Instance;
+            var instance = await component.PublishInstanceAsync();
 
             await component.LoadAsync();
 
-            Assert.Same(instance, component.Instance);
+            var result = await component.PublishInstanceAsync();
+
+            Assert.Same(instance, result);
         }
 
         [Fact]
@@ -106,14 +117,16 @@ public async Task LoadAsync_WhenUnloaded_CreatesNewInstance()
 
             await component.LoadAsync();
 
-            var instance = component.Instance;
+            var instance = await component.PublishInstanceAsync();
 
             await component.UnloadAsync();
 
             // We should create a new instance here
             await component.LoadAsync();
 
-            Assert.NotSame(instance, component.Instance);
+            var result = await component.PublishInstanceAsync();
+
+            Assert.NotSame(instance, result);
         }
 
         [Fact]
@@ -122,12 +135,11 @@ public async Task UnloadAsync_WhenLoaded_DisposesUnderlyingInstance()
             var component = CreateInstance();
 
             await component.LoadAsync();
-            var instance = component.Instance;
+            var result = await component.PublishInstanceAsync();
 
             await component.UnloadAsync();
 
-            Assert.Null(component.Instance);
-            Assert.True(((MultiLifetimeInstance)instance).IsDisposed);
+            Assert.True(result.IsDisposed);
         }
 
         [Fact]
@@ -136,8 +148,6 @@ public async Task UnloadAsync_WhenNotLoaded_DoesNothing()
             var component = CreateInstance();
 
             await component.UnloadAsync();
-
-            Assert.Null(component.Instance);
         }
 
         [Fact]
@@ -165,11 +175,11 @@ public async Task Dispose_WhenLoaded_DisposesUnderlyingInstance()
             var component = CreateInstance();
 
             await component.LoadAsync();
-            var instance = component.Instance;
+            var instance = await component.PublishInstanceAsync();
 
             component.Dispose();
 
-            Assert.True(((MultiLifetimeInstance)instance).IsDisposed);
+            Assert.True(instance.IsDisposed);
         }
 
         private static AbstractMultiLifetimeComponentFactory.MultiLifetimeComponent CreateInstance()

diff --git a/...ojectSystem.Managed.UnitTests/ProjectSystem/LanguageServices/WorkspaceContextHostTests.cs b/...ojectSystem.Managed.UnitTests/ProjectSystem/LanguageServices/WorkspaceContextHostTests.cs
@@ -11,57 +11,67 @@ namespace Microsoft.VisualStudio.ProjectSystem.LanguageServices
     public class WorkspaceContextHostTests
     {
         [Fact]
-        public void Loaded_WhenNotActivated_ReturnsNonCompletedTask()
+        public void PublishAsync_WhenNotActivated_ReturnsNonCompletedTask()
         {
             var component = CreateInstance();
 
-            Assert.False(component.Loaded.IsCanceled);
-            Assert.False(component.Loaded.IsCompleted);
-            Assert.False(component.Loaded.IsFaulted);
+            var result = component.PublishAsync();
+
+            Assert.False(result.IsCanceled);
+            Assert.False(result.IsCompleted);
+            Assert.False(result.IsFaulted);
         }
 
         [Fact]
-        public async Task Loaded_WhenActivated_ReturnsCompletedTask()
+        public async Task PublishAsync_WhenActivated_ReturnsCompletedTask()
         {
             var component = CreateInstance();
 
             await component.ActivateAsync();
 
-            Assert.True(component.Loaded.IsCompleted);
+            var result = component.PublishAsync();
+
+            Assert.True(result.IsCompleted);
         }
 
         [Fact]
-        public async Task Loaded_WhenDeactivated_ReturnsNonCompletedTask()
+        public async Task PublishAsync_WhenDeactivated_ReturnsNonCompletedTask()
         {
             var component = CreateInstance();
 
             await component.ActivateAsync();
             await component.UnloadAsync();
 
-            Assert.False(component.Loaded.IsCanceled);
-            Assert.False(component.Loaded.IsCompleted);
-            Assert.False(component.Loaded.IsFaulted);
+            var result = component.PublishAsync();
+
+            Assert.False(result.IsCanceled);
+            Assert.False(result.IsCompleted);
+            Assert.False(result.IsFaulted);
         }
 
         [Fact]
-        public async Task Loaded_DisposedWhenNotActivated_ReturnsCancelledTask()
+        public async Task PublishAsync_DisposedWhenNotActivated_ReturnsCancelledTask()
         {
             var component = CreateInstance();
 
             await component.DisposeAsync();
 
-            Assert.True(component.Loaded.IsCanceled);
+            var result = component.PublishAsync();
+
+            Assert.True(result.IsCanceled);
         }
 
         [Fact]
-        public async Task Loaded_DisposedWhenActivated_ReturnsCancelledTask()
+        public async Task PublishAsync_DisposedWhenActivated_ReturnsCancelledTask()
         {
             var component = CreateInstance();
 
             await component.ActivateAsync();
             await component.DisposeAsync();
 
-            Assert.True(component.Loaded.IsCanceled);
+            var result = component.PublishAsync();
+
+            Assert.True(result.IsCanceled);
         }
 
         [Fact]

diff --git a/...dio.ProjectSystem.Managed.VS.UnitTests/Mocks/IActiveWorkspaceProjectContextHostFactory.cs b/...dio.ProjectSystem.Managed.VS.UnitTests/Mocks/IActiveWorkspaceProjectContextHostFactory.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using System.Threading;
 using System.Threading.Tasks;
 using Moq;
 
@@ -29,9 +30,9 @@ public ActiveWorkspaceProjectContextHost(IWorkspaceProjectContextAccessor access
                 _accessor = accessor;
             }
 
-            public Task Loaded
+            public Task PublishAsync(CancellationToken cancellationToken = default)
             {
-                get { return Task.CompletedTask; }
+                return Task.CompletedTask;
             }
 
             public Task OpenContextForWriteAsync(Func<IWorkspaceProjectContextAccessor, Task> action)
@@ -43,6 +44,8 @@ public Task<T> OpenContextForWriteAsync<T>(Func<IWorkspaceProjectContextAccessor
             {
                 return action(_accessor);
             }
+
+
         }
     }
 }
diff --git a/...stem.Managed.VS/ProjectSystem/VS/LanguageServices/VsContainedLanguageComponentsFactory.cs b/...stem.Managed.VS/ProjectSystem/VS/LanguageServices/VsContainedLanguageComponentsFactory.cs
@@ -54,7 +54,7 @@ public int GetContainedLanguageFactoryForFile(string filePath,
 
         private async Task<(HierarchyId itemid, IVsHierarchy hierarchy, IVsContainedLanguageFactory containedLanguageFactory)> GetContainedLanguageFactoryForFileAsync(string filePath)
         {
-            await _projectContextHost.Loaded;
+            await _projectContextHost.PublishAsync();
 
             await _projectVsServices.ThreadingService.SwitchToUIThread();