diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs index 10f68a3a7d4..9bc2b27d997 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs +++ b/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. /// - /// - /// 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. - /// /// /// The project update to integrate. /// A task that completes when the update has been integrated. - internal async Task OnWorkspaceUpdateAsync(IProjectVersionedValue update) + internal Task OnWorkspaceUpdateAsync(IProjectVersionedValue 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) diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs index 7fb1a421313..89f5f430646 100644 --- a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs @@ -85,11 +85,20 @@ public async Task Dispose_TriggersObjectDisposedExceptionsOnPublicMembers() await Assert.ThrowsAsync(() => workspace.WriteAsync(w => Task.CompletedTask, CancellationToken.None)); await Assert.ThrowsAsync(() => workspace.WriteAsync(w => TaskResult.EmptyString, CancellationToken.None)); - await Assert.ThrowsAsync(() => workspace.OnWorkspaceUpdateAsync(null!)); Assert.Throws(() => workspace.ChainDisposal(null!)); } + [Fact] + public async Task Dispose_TriggersOperationCanceledExceptionOnWorkspaceUpdates() + { + var workspace = await CreateInstanceAsync(); + + await workspace.DisposeAsync(); + + await Assert.ThrowsAsync(() => workspace.OnWorkspaceUpdateAsync(null!)); + } + [Theory] [CombinatorialData] public async Task WriteAsync_ThrowsIfNullAction(bool isGeneric)