Synchronise language service project updates

Ensures a workspace will not be disposed during an update by wrapping the update in `ExecuteUnderLockAsync`.

Author: drewnoakes

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs

@@ -156,66 +156,65 @@ public void ChainDisposal(IDisposable disposable)
     /// both evaluation and build updates may arrive in any order, so long as values
     /// of each type are ordered correctly.
     /// </para>
-    /// <para>
-    /// Calls must not overlap. This method is not thread-safe. This method is designed
-    /// to be called from a dataflow ActionBlock, which will serialize calls, so we
-    /// needn't perform any locking or protection here.
-    /// </para>
     /// </remarks>
     /// <param name="update">The project update to integrate.</param>
     /// <returns>A task that completes when the update has been integrated.</returns>
-    internal async Task OnWorkspaceUpdateAsync(IProjectVersionedValue<WorkspaceUpdate> update)
+    internal Task OnWorkspaceUpdateAsync(IProjectVersionedValue<WorkspaceUpdate> update)
     {
-        Verify.NotDisposed(this);
+        // Prevent disposal during the update.
+        return ExecuteUnderLockAsync(async token =>
+        {
+            Verify.NotDisposed(this);
+
+            await InitializeAsync(_unloadCancellationToken);
 
-        await InitializeAsync(_unloadCancellationToken);
+            Assumes.True(_state is WorkspaceState.Uninitialized or WorkspaceState.Initialized);
 
-        Assumes.True(_state is WorkspaceState.Uninitialized or WorkspaceState.Initialized);
+            await _joinableTaskFactory.RunAsync(ApplyUpdateWithinLockAsync);
+        });
 
-        await _joinableTaskFactory.RunAsync(
-            async () =>
+        async Task ApplyUpdateWithinLockAsync()
+        {
+            // We will always receive an evaluation update before the first build update.
+
+            if (TryTransition(WorkspaceState.Uninitialized, WorkspaceState.Initialized))
             {
-                // Calls never overlap. No synchronisation is needed here.
-                // We can receive either evaluation OR build data first.
+                // Note that we create operation progress registrations using the first primary (active) configuration
+                // within the slice. Over time this may change, but we keep the same registration to the first seen.
 
-                if (TryTransition(WorkspaceState.Uninitialized, WorkspaceState.Initialized))
+                ConfiguredProject configuredProject = update.Value switch
                 {
-                    // Note that we create operation progress registrations using the first primary (active) configuration
-                    // within the slice. Over time this may change, but we keep the same registration to the first seen.
-
-                    ConfiguredProject configuredProject = update.Value switch
-                    {
-                        { EvaluationUpdate: EvaluationUpdate update } => update.ConfiguredProject,
-                        { BuildUpdate: BuildUpdate update } => update.ConfiguredProject,
-                        _ => throw Assumes.NotReachable()
-                    };
+                    { EvaluationUpdate: EvaluationUpdate update } => update.ConfiguredProject,
+                    { BuildUpdate: BuildUpdate update } => update.ConfiguredProject,
+                    _ => throw Assumes.NotReachable()
+                };
 
-                    _evaluationProgressRegistration = _dataProgressTrackerService.RegisterForIntelliSense(this, configuredProject, "LanguageServiceHost.Workspace.Evaluation");
-                    _buildProgressRegistration = _dataProgressTrackerService.RegisterForIntelliSense(this, configuredProject, "LanguageServiceHost.Workspace.ProjectBuild");
+                _evaluationProgressRegistration = _dataProgressTrackerService.RegisterForIntelliSense(this, configuredProject, "LanguageServiceHost.Workspace.Evaluation");
+                _buildProgressRegistration = _dataProgressTrackerService.RegisterForIntelliSense(this, configuredProject, "LanguageServiceHost.Workspace.ProjectBuild");
 
-                    _disposableBag.Add(_evaluationProgressRegistration);
-                    _disposableBag.Add(_buildProgressRegistration);
-                }
+                _disposableBag.Add(_evaluationProgressRegistration);
+                _disposableBag.Add(_buildProgressRegistration);
+            }
 
-                try
-                {
-                    await (update.Value switch
-                    {
-                        { EvaluationUpdate: not null } => OnEvaluationUpdateAsync(update.Derive(u => u.EvaluationUpdate!)),
-                        { BuildUpdate: not null } => OnBuildUpdateAsync(update.Derive(u => u.BuildUpdate!)),
-                        _ => throw Assumes.NotReachable()
-                    });
-                }
-                catch
+            try
+            {
+                await (update.Value switch
                 {
-                    // Tear down on any exception
-                    await DisposeAsync();
+                    { EvaluationUpdate: not null } => OnEvaluationUpdateAsync(update.Derive(u => u.EvaluationUpdate!)),
+                    { BuildUpdate: not null } => OnBuildUpdateAsync(update.Derive(u => u.BuildUpdate!)),
+                    _ => throw Assumes.NotReachable()
+                });
+            }
+            catch
+            {
+                // Tear down on any exception
+                await DisposeAsync();
 
-                    // Exceptions here are product errors, so let the exception escape in order
-                    // to produce an upstream NFE.
-                    throw;
-                }
-            });
+                // Exceptions here are product errors, so let the exception escape in order
+                // to produce an upstream NFE.
+                throw;
+            }
+        }
     }
 
     private async Task OnEvaluationUpdateAsync(IProjectVersionedValue<EvaluationUpdate> evaluationUpdate)

tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs

@@ -85,11 +85,20 @@ public async Task Dispose_TriggersObjectDisposedExceptionsOnPublicMembers()
 
         await Assert.ThrowsAsync<ObjectDisposedException>(() => workspace.WriteAsync(w => Task.CompletedTask, CancellationToken.None));
         await Assert.ThrowsAsync<ObjectDisposedException>(() => workspace.WriteAsync(w => TaskResult.EmptyString, CancellationToken.None));
-        await Assert.ThrowsAsync<ObjectDisposedException>(() => workspace.OnWorkspaceUpdateAsync(null!));
 
         Assert.Throws<ObjectDisposedException>(() => workspace.ChainDisposal(null!));
     }
 
+    [Fact]
+    public async Task Dispose_TriggersOperationCanceledExceptionOnWorkspaceUpdates()
+    {
+        var workspace = await CreateInstanceAsync();
+
+        await workspace.DisposeAsync();
+
+        await Assert.ThrowsAsync<OperationCanceledException>(() => workspace.OnWorkspaceUpdateAsync(null!));
+    }
+
     [Theory]
     [CombinatorialData]
     public async Task WriteAsync_ThrowsIfNullAction(bool isGeneric)