From c553551c708c04eb5d65e7612f23709f43e33247 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Sat, 12 Apr 2025 17:47:29 -0700 Subject: [PATCH 01/17] WIP: Start moving workspace events to fire on background threads This addresses a major portion of https://github.com/dotnet/roslyn/issues/78091 There are still some WorkspaceChanged events that haven't been switched over. This is big enough without doing all those and I'd like to get a ci run on this to see how broken tests are. Additionally, this didn't tackle adding the WorkspaceChangedEventOptions, that can be added later if we determine it would provide value. --- src/Compilers/Test/Core/FX/EventWaiter.cs | 21 +++ .../WorkspaceTests_EditorFeatures.cs | 163 +++++++++++------- ...lassificationTaggerProvider.TagComputer.cs | 31 ++-- .../ActiveStatementTrackingService.cs | 22 ++- .../Core/Remote/SolutionChecksumUpdater.cs | 9 +- ...DocumentActiveContextChangedEventSource.cs | 14 +- ...sticAnalyzerService_IncrementalAnalyzer.cs | 12 +- ...tractCreateServicesOnTextViewConnection.cs | 14 +- ...xtViewWindowVerifierInProcessExtensions.cs | 30 +--- .../Framework/WorkspaceChangeWatcher.vb | 12 +- .../Impl/Implementation/XamlProjectService.cs | 17 +- .../Api/UnitTestingWorkspaceExtensions.cs | 30 ++-- .../Core/Portable/Workspace/Workspace.cs | 31 ++++ .../Workspace/WorkspaceEventRegistration.cs | 15 ++ .../Portable/Workspace/Workspace_Events.cs | 74 ++++++-- .../Workspace/Workspace_EventsAsync.cs | 59 +++++++ .../WorkspaceTests/AdhocWorkspaceTests.cs | 21 ++- .../CoreTestUtilities/WorkspaceExtensions.cs | 5 +- .../Test/VisualStudioMSBuildWorkspaceTests.cs | 4 + .../Core/CompilerExtensions.projitems | 1 + .../Compiler/Core/Utilities/AsyncEventMap.cs | 73 ++++++++ 21 files changed, 496 insertions(+), 162 deletions(-) create mode 100644 src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs create mode 100644 src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs create mode 100644 src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs diff --git a/src/Compilers/Test/Core/FX/EventWaiter.cs b/src/Compilers/Test/Core/FX/EventWaiter.cs index 524d7178c2598..c753674c7db0e 100644 --- a/src/Compilers/Test/Core/FX/EventWaiter.cs +++ b/src/Compilers/Test/Core/FX/EventWaiter.cs @@ -55,6 +55,27 @@ public EventHandler Wrap(EventHandler input) }; } + public Func Wrap(Func input) + { + return (args) => + { + try + { + return input(args); + } + catch (Exception ex) + { + _capturedException = ex; + } + finally + { + _eventSignal.Set(); + } + + return Task.CompletedTask; + }; + } + /// /// Use this method to block the test until the operation enclosed in the Wrap method completes /// diff --git a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs index 0a778c32da08d..3409388915ffb 100644 --- a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs +++ b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs @@ -62,7 +62,11 @@ public async Task TestEmptySolutionUpdateDoesNotFireEvents() var solution = workspace.CurrentSolution; var workspaceChanged = false; - workspace.WorkspaceChanged += (s, e) => workspaceChanged = true; + using var _ = workspace.RegisterWorkspaceChangedHandler(e => + { + workspaceChanged = true; + return Task.CompletedTask; + }); // make an 'empty' update by claiming something changed, but its the same as before workspace.OnParseOptionsChanged(project.Id, project.ParseOptions); @@ -814,16 +818,26 @@ public async Task TestDocumentEvents() using var closeWaiter = new EventWaiter(); using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var documentOpenedEventHandler = openWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - "The document given to the 'DocumentOpened' event handler did not have the same id as the one created for the test.")); + var documentOpenedEventHandlerAsync = openWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + "The document given to the 'DocumentOpened' event handler did not have the same id as the one created for the test."); + + return Task.CompletedTask; + }); + + var documentClosedEventHandlerAsync = closeWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + "The document given to the 'DocumentClosed' event handler did not have the same id as the one created for the test."); - var documentClosedEventHandler = closeWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - "The document given to the 'DocumentClosed' event handler did not have the same id as the one created for the test.")); + return Task.CompletedTask; + }); - workspace.DocumentOpened += documentOpenedEventHandler; - workspace.DocumentClosed += documentClosedEventHandler; + var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandlerAsync); + var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandlerAsync); workspace.OpenDocument(document.Id); workspace.CloseDocument(document.Id); @@ -833,15 +847,15 @@ public async Task TestDocumentEvents() // Wait to receive signal that events have fired. Assert.True(openWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'DocumentOpened' was not fired within {0} minutes.", + string.Format("event for 'RegisterDocumentOpenedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); Assert.True(closeWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'DocumentClosed' was not fired within {0} minutes.", + string.Format("event for 'RegisterDocumentClosedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); - workspace.DocumentOpened -= documentOpenedEventHandler; - workspace.DocumentClosed -= documentClosedEventHandler; + documentOpenedDisposer.Dispose(); + documentClosedDisposer.Dispose(); workspace.OpenDocument(document.Id); workspace.CloseDocument(document.Id); @@ -852,11 +866,11 @@ public async Task TestDocumentEvents() // Verifying that an event has not been called is difficult to prove. // All events should have already been called so we wait 5 seconds and then assume the event handler was removed correctly. Assert.False(openWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'DocumentOpened' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterDocumentOpenedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); Assert.False(closeWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'DocumentClosed' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterDocumentClosedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); } @@ -880,16 +894,26 @@ public async Task TestSourceGeneratedDocumentEvents() using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var documentOpenedEventHandler = openWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - $"The source generated document given to the '{nameof(Workspace.DocumentOpened)}' event handler did not have the same id as the one created for the test.")); + var documentOpenedEventHandlerAsync = openWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + $"The source generated document given to the '{nameof(Workspace.RegisterDocumentOpenedHandler)}' did not have the same id as the one created for the test."); + + return Task.CompletedTask; + }); + + var documentClosedEventHandlerAsync = closeWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + $"The source generated document given to the '{nameof(Workspace.RegisterDocumentClosedHandler)}' did not have the same id as the one created for the test."); - var documentClosedEventHandler = closeWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - $"The source generated document given to the '{nameof(Workspace.DocumentClosed)}' event handler did not have the same id as the one created for the test.")); + return Task.CompletedTask; + }); - workspace.DocumentOpened += documentOpenedEventHandler; - workspace.DocumentClosed += documentClosedEventHandler; + var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandlerAsync); + var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandlerAsync); workspace.OpenSourceGeneratedDocument(document.Id); var sourceGeneratedDocumentId = workspace.GetDocumentIdInCurrentContext(document.GetOpenTextContainer()); @@ -902,15 +926,15 @@ public async Task TestSourceGeneratedDocumentEvents() // Wait to receive signal that events have fired. Assert.True(openWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'DocumentOpened' was not fired within {0} minutes.", + string.Format("event for 'RegisterDocumentOpenedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); Assert.True(closeWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'DocumentClosed' was not fired within {0} minutes.", + string.Format("event for 'RegisterDocumentClosedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); - workspace.DocumentOpened -= documentOpenedEventHandler; - workspace.DocumentClosed -= documentClosedEventHandler; + documentOpenedDisposer.Dispose(); + documentClosedDisposer.Dispose(); workspace.OpenSourceGeneratedDocument(document.Id); await workspace.CloseSourceGeneratedDocumentAsync(document.Id); @@ -921,11 +945,11 @@ public async Task TestSourceGeneratedDocumentEvents() // Verifying that an event has not been called is difficult to prove. // All events should have already been called so we wait 5 seconds and then assume the event handler was removed correctly. Assert.False(openWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'DocumentOpened' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterDocumentOpenedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); Assert.False(closeWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'DocumentClosed' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterDocumentClosedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); } @@ -944,16 +968,26 @@ public async Task TestAdditionalDocumentEvents() using var closeWaiter = new EventWaiter(); using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var documentOpenedEventHandler = openWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - "The document given to the 'AdditionalDocumentOpened' event handler did not have the same id as the one created for the test.")); + var textDocumentOpenedEventHandlerAsync = openWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + "The document given to the 'RegisterTextDocumentOpenedHandler' event handler did not have the same id as the one created for the test."); + + return Task.CompletedTask; + }); + + var textDocumentClosedEventHandlerAsync = closeWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + "The document given to the 'RegisterTextDocumentClosedHandler' event handler did not have the same id as the one created for the test."); - var documentClosedEventHandler = closeWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - "The document given to the 'AdditionalDocumentClosed' event handler did not have the same id as the one created for the test.")); + return Task.CompletedTask; + }); - workspace.TextDocumentOpened += documentOpenedEventHandler; - workspace.TextDocumentClosed += documentClosedEventHandler; + var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandlerAsync); + var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandlerAsync); workspace.OpenAdditionalDocument(document.Id); workspace.CloseAdditionalDocument(document.Id); @@ -963,15 +997,15 @@ public async Task TestAdditionalDocumentEvents() // Wait to receive signal that events have fired. Assert.True(openWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'AdditionalDocumentOpened' was not fired within {0} minutes.", + string.Format("event for 'RegisterTextDocumentOpenedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); Assert.True(closeWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'AdditionalDocumentClosed' was not fired within {0} minutes.", + string.Format("event for 'RegisterTextDocumentClosedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); - workspace.TextDocumentOpened -= documentOpenedEventHandler; - workspace.TextDocumentClosed -= documentClosedEventHandler; + textDocumentOpenedDisposer.Dispose(); + textDocumentClosedDisposer.Dispose(); workspace.OpenAdditionalDocument(document.Id); workspace.CloseAdditionalDocument(document.Id); @@ -982,11 +1016,11 @@ public async Task TestAdditionalDocumentEvents() // Verifying that an event has not been called is difficult to prove. // All events should have already been called so we wait 5 seconds and then assume the event handler was removed correctly. Assert.False(openWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'AdditionalDocumentOpened' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterTextDocumentOpenedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); Assert.False(closeWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'AdditionalDocumentClosed' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterTextDocumentClosedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); } @@ -1005,16 +1039,26 @@ public async Task TestAnalyzerConfigDocumentEvents() using var closeWaiter = new EventWaiter(); using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var documentOpenedEventHandler = openWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - "The document given to the 'AnalyzerConfigDocumentOpened' event handler did not have the same id as the one created for the test.")); + var textDocumentOpenedEventHandlerAsync = openWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + "The document given to the 'AnalyzerConfigDocumentOpened' event handler did not have the same id as the one created for the test."); + + return Task.CompletedTask; + }); + + var textDocumentClosedEventHandlerAsync = closeWaiter.Wrap( + args => + { + Assert.True(args.Document.Id == document.Id, + "The document given to the 'AnalyzerConfigDocumentClosed' event handler did not have the same id as the one created for the test."); - var documentClosedEventHandler = closeWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, - "The document given to the 'AnalyzerConfigDocumentClosed' event handler did not have the same id as the one created for the test.")); + return Task.CompletedTask; + }); - workspace.TextDocumentOpened += documentOpenedEventHandler; - workspace.TextDocumentClosed += documentClosedEventHandler; + var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandlerAsync); + var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandlerAsync); workspace.OpenAnalyzerConfigDocument(document.Id); workspace.CloseAnalyzerConfigDocument(document.Id); @@ -1024,15 +1068,15 @@ public async Task TestAnalyzerConfigDocumentEvents() // Wait to receive signal that events have fired. Assert.True(openWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'AnalyzerConfigDocumentOpened' was not fired within {0} minutes.", + string.Format("event for 'RegisterTextDocumentOpenedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); Assert.True(closeWaiter.WaitForEventToFire(longEventTimeout), - string.Format("event 'AnalyzerConfigDocumentClosed' was not fired within {0} minutes.", + string.Format("event for 'RegisterTextDocumentClosedHandler' was not fired within {0} minutes.", longEventTimeout.Minutes)); - workspace.TextDocumentOpened -= documentOpenedEventHandler; - workspace.TextDocumentClosed -= documentClosedEventHandler; + textDocumentOpenedDisposer.Dispose(); + textDocumentClosedDisposer.Dispose(); workspace.OpenAnalyzerConfigDocument(document.Id); workspace.CloseAnalyzerConfigDocument(document.Id); @@ -1043,11 +1087,11 @@ public async Task TestAnalyzerConfigDocumentEvents() // Verifying that an event has not been called is difficult to prove. // All events should have already been called so we wait 5 seconds and then assume the event handler was removed correctly. Assert.False(openWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'AnalyzerConfigDocumentOpened' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterTextDocumentOpenedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); Assert.False(closeWaiter.WaitForEventToFire(shortEventTimeout), - string.Format("event handler 'AnalyzerConfigDocumentClosed' was called within {0} seconds though it was removed from the list.", + string.Format("event for 'RegisterTextDocumentClosedHandler' was called within {0} seconds though it was removed from the list.", shortEventTimeout.Seconds)); } @@ -1420,11 +1464,12 @@ public async Task TestLinkedFilesStayInSync() using var workspace = EditorTestWorkspace.Create(input, composition: EditorTestCompositions.EditorFeatures, openDocuments: true); var eventArgs = new List(); - workspace.WorkspaceChanged += (s, e) => + using var _ = workspace.RegisterWorkspaceChangedHandler(e => { Assert.Equal(WorkspaceChangeKind.DocumentChanged, e.Kind); eventArgs.Add(e); - }; + return Task.CompletedTask; + }); var originalDocumentId = workspace.GetOpenDocumentIds().Single(id => !workspace.GetTestDocument(id).IsLinkFile); var linkedDocumentId = workspace.GetOpenDocumentIds().Single(id => workspace.GetTestDocument(id).IsLinkFile); diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 64e62295d6774..1758cc44e3464 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -46,6 +46,8 @@ private sealed record CachedServices( private readonly SyntacticClassificationTaggerProvider _taggerProvider; private readonly ITextBuffer2 _subjectBuffer; private readonly WorkspaceRegistration _workspaceRegistration; + private IDisposable? _workspaceChangedDisposer; + private IDisposable? _workspaceDocumentActiveContextChangedDisposer; private readonly CancellationTokenSource _disposalCancellationSource = new(); @@ -224,8 +226,8 @@ private void ConnectToWorkspace(Workspace workspace) _taggerProvider.ThreadingContext.ThrowIfNotOnUIThread(); _workspace = workspace; - _workspace.WorkspaceChanged += this.OnWorkspaceChanged; - _workspace.DocumentActiveContextChanged += this.OnDocumentActiveContextChanged; + _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChangedAsync); + _workspaceDocumentActiveContextChangedDisposer = _workspace.RegisterDocumentActiveContextChangedHandler(this.OnDocumentActiveContextChangedAsync); // Now that we've connected to the workspace, kick off work to reclassify this buffer. _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); @@ -243,8 +245,11 @@ public void DisconnectFromWorkspace() if (_workspace != null) { - _workspace.WorkspaceChanged -= this.OnWorkspaceChanged; - _workspace.DocumentActiveContextChanged -= this.OnDocumentActiveContextChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + + _workspaceDocumentActiveContextChangedDisposer?.Dispose(); + _workspaceDocumentActiveContextChangedDisposer = null; _workspace = null; @@ -264,33 +269,34 @@ private void OnSubjectBufferChanged(object? sender, TextContentChangedEventArgs _workQueue.AddWork(args.After); } - private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContextChangedEventArgs args) + private Task OnDocumentActiveContextChangedAsync(DocumentActiveContextChangedEventArgs args) { if (_workspace == null) - return; + return Task.CompletedTask; var documentId = args.NewActiveContextDocumentId; var bufferDocumentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); if (bufferDocumentId != documentId) - return; + return Task.CompletedTask; _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); + return Task.CompletedTask; } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs args) + private Task OnWorkspaceChangedAsync(WorkspaceChangeEventArgs args) { // We may be getting an event for a workspace we already disconnected from. If so, // ignore them. We won't be able to find the Document corresponding to our text buffer, // so we can't reasonably classify this anyways. if (args.NewSolution.Workspace != _workspace) - return; + return Task.CompletedTask; if (args.Kind != WorkspaceChangeKind.ProjectChanged) - return; + return Task.CompletedTask; var documentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); if (args.ProjectId != documentId?.ProjectId) - return; + return Task.CompletedTask; var oldProject = args.OldSolution.GetProject(args.ProjectId); var newProject = args.NewSolution.GetProject(args.ProjectId); @@ -298,9 +304,10 @@ private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs args) // In case of parse options change reclassify the doc as it may have affected things // like preprocessor directives. if (Equals(oldProject?.ParseOptions, newProject?.ParseOptions)) - return; + return Task.CompletedTask; _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); + return Task.CompletedTask; } #endregion diff --git a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs index 48c04aed76a34..1af1bf7c181a4 100644 --- a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs +++ b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs @@ -103,6 +103,8 @@ internal sealed class TrackingSession private readonly CancellationTokenSource _cancellationSource = new(); private readonly IActiveStatementSpanFactory _spanProvider; private readonly ICompileTimeSolutionProvider _compileTimeSolutionProvider; + private readonly IDisposable _documentOpenedHandlerDisposer; + private readonly IDisposable _documentClosedHandlerDisposer; #region lock(_trackingSpans) @@ -121,8 +123,8 @@ public TrackingSession(Workspace workspace, IActiveStatementSpanFactory spanProv _spanProvider = spanProvider; _compileTimeSolutionProvider = workspace.Services.GetRequiredService(); - _workspace.DocumentOpened += DocumentOpened; - _workspace.DocumentClosed += DocumentClosed; + _documentOpenedHandlerDisposer = _workspace.RegisterDocumentOpenedHandler(DocumentOpenedAsync); + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(DocumentClosedAsync); } internal Dictionary> Test_GetTrackingSpans() @@ -133,8 +135,8 @@ public void EndTracking() _cancellationSource.Cancel(); _cancellationSource.Dispose(); - _workspace.DocumentOpened -= DocumentOpened; - _workspace.DocumentClosed -= DocumentClosed; + _documentOpenedHandlerDisposer.Dispose(); + _documentClosedHandlerDisposer.Dispose(); lock (_trackingSpans) { @@ -142,7 +144,7 @@ public void EndTracking() } } - private void DocumentClosed(object? sender, DocumentEventArgs e) + private Task DocumentClosedAsync(DocumentEventArgs e) { if (e.Document.FilePath != null) { @@ -151,10 +153,16 @@ private void DocumentClosed(object? sender, DocumentEventArgs e) _trackingSpans.Remove(e.Document.FilePath); } } + + return Task.CompletedTask; } - private void DocumentOpened(object? sender, DocumentEventArgs e) - => _ = TrackActiveSpansAsync(e.Document); + private Task DocumentOpenedAsync(DocumentEventArgs e) + { + _ = TrackActiveSpansAsync(e.Document); + + return Task.CompletedTask; + } private async Task TrackActiveSpansAsync(Document designTimeDocument) { diff --git a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs index 70dabe4a7aa68..5c9aed82d0e7b 100644 --- a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs +++ b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs @@ -44,6 +44,7 @@ internal sealed class SolutionChecksumUpdater private readonly object _gate = new(); private bool _isSynchronizeWorkspacePaused; + private readonly IDisposable _workspaceChangedDisposer; private readonly CancellationToken _shutdownToken; @@ -79,7 +80,7 @@ public SolutionChecksumUpdater( shutdownToken); // start listening workspace change event - _workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChangedAsync); _workspace.WorkspaceChangedImmediate += OnWorkspaceChangedImmediate; _documentTrackingService.ActiveDocumentChanged += OnActiveDocumentChanged; @@ -100,7 +101,7 @@ public void Shutdown() PauseSynchronizingPrimaryWorkspace(); _documentTrackingService.ActiveDocumentChanged -= OnActiveDocumentChanged; - _workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer.Dispose(); _workspace.WorkspaceChangedImmediate -= OnWorkspaceChangedImmediate; if (_globalOperationService != null) @@ -136,7 +137,7 @@ private void ResumeSynchronizingPrimaryWorkspace() } } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private Task OnWorkspaceChangedAsync(WorkspaceChangeEventArgs e) { // Check if we're currently paused. If so ignore this notification. We don't want to any work in response // to whatever the workspace is doing. @@ -145,6 +146,8 @@ private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) if (!_isSynchronizeWorkspacePaused) _synchronizeWorkspaceQueue.AddWork(); } + + return Task.CompletedTask; } private void OnWorkspaceChangedImmediate(object? sender, WorkspaceChangeEventArgs e) diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs index f027bad3102a9..d47239c6c1fd8 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; @@ -11,13 +13,17 @@ internal partial class TaggerEventSources { private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subjectBuffer) : AbstractWorkspaceTrackingTaggerEventSource(subjectBuffer) { + private IDisposable? _documentActiveContextChangedDisposer; + protected override void ConnectToWorkspace(Workspace workspace) - => workspace.DocumentActiveContextChanged += OnDocumentActiveContextChanged; + { + _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChangedAsync); + } protected override void DisconnectFromWorkspace(Workspace workspace) - => workspace.DocumentActiveContextChanged -= OnDocumentActiveContextChanged; + => _documentActiveContextChangedDisposer?.Dispose(); - private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContextChangedEventArgs e) + private Task OnDocumentActiveContextChangedAsync(DocumentActiveContextChangedEventArgs e) { var document = SubjectBuffer.AsTextContainer().GetOpenDocumentInCurrentContext(); @@ -25,6 +31,8 @@ private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContex { this.RaiseChanged(); } + + return Task.CompletedTask; } } } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs index b9598aacd82f4..dd891f763286e 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Threading.Tasks; + namespace Microsoft.CodeAnalysis.Diagnostics; internal sealed partial class DiagnosticAnalyzerService @@ -14,11 +16,15 @@ private DiagnosticIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace worksp private DiagnosticIncrementalAnalyzer CreateIncrementalAnalyzerCallback(Workspace workspace) { // subscribe to active context changed event for new workspace - workspace.DocumentActiveContextChanged += OnDocumentActiveContextChanged; + _ = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChangedAsync); return new DiagnosticIncrementalAnalyzer(this, AnalyzerInfoCache, this.GlobalOptions); } - private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContextChangedEventArgs e) - => RequestDiagnosticRefresh(); + private Task OnDocumentActiveContextChangedAsync(DocumentActiveContextChangedEventArgs e) + { + RequestDiagnosticRefresh(); + + return Task.CompletedTask; + } } diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs index 01e7b4d0faf02..64c357f274eec 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs @@ -26,11 +26,12 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.LanguageService /// Creates services on the first connection of an applicable subject buffer to an IWpfTextView. /// This ensures the services are available by the time an open document or the interactive window needs them. /// -internal abstract class AbstractCreateServicesOnTextViewConnection : IWpfTextViewConnectionListener +internal abstract class AbstractCreateServicesOnTextViewConnection : IWpfTextViewConnectionListener, IDisposable { private readonly string _languageName; private readonly AsyncBatchingWorkQueue _workQueue; private bool _initialized = false; + private readonly IDisposable _workspaceDocumentOpenedDisposer; protected VisualStudioWorkspace Workspace { get; } protected IGlobalOptionService GlobalOptions { get; } @@ -56,7 +57,7 @@ public AbstractCreateServicesOnTextViewConnection( listenerProvider.GetListener(FeatureAttribute.CompletionSet), threadingContext.DisposalToken); - Workspace.DocumentOpened += QueueWorkOnDocumentOpened; + _workspaceDocumentOpenedDisposer = Workspace.RegisterDocumentOpenedHandler(QueueWorkOnDocumentOpenedAsync); } void IWpfTextViewConnectionListener.SubjectBuffersConnected(IWpfTextView textView, ConnectionReason reason, Collection subjectBuffers) @@ -96,10 +97,12 @@ private async ValueTask BatchProcessProjectsWithOpenedDocumentAsync(ImmutableSeg } } - private void QueueWorkOnDocumentOpened(object sender, DocumentEventArgs e) + private Task QueueWorkOnDocumentOpenedAsync(DocumentEventArgs e) { if (e.Document.Project.Language == _languageName) _workQueue.AddWork(e.Document.Project.Id); + + return Task.CompletedTask; } private void InitializePerVSSessionServices() @@ -112,4 +115,9 @@ private void InitializePerVSSessionServices() // https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1242321 languageServices.GetService()?.LoadImportedProviders(); } + + public void Dispose() + { + _workspaceDocumentOpenedDisposer?.Dispose(); + } } diff --git a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs index d0a6733054fdf..256e8122a0f50 100644 --- a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs +++ b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs @@ -62,10 +62,13 @@ public static async Task CodeActionAsync( CancellationToken cancellationToken = default) { var events = new List(); - void WorkspaceChangedHandler(object sender, WorkspaceChangeEventArgs e) => events.Add(e); var workspace = await textViewWindowVerifier.TestServices.Shell.GetComponentModelServiceAsync(cancellationToken); - using var workspaceEventRestorer = WithWorkspaceChangedHandler(workspace, WorkspaceChangedHandler); + using var _ = workspace.RegisterWorkspaceChangedHandler(e => + { + events.Add(e); + return Task.CompletedTask; + }); await textViewWindowVerifier.TestServices.Editor.ShowLightBulbAsync(cancellationToken); @@ -155,12 +158,6 @@ await textViewWindowVerifier.TestServices.Workspace.WaitForAllAsyncOperationsAsy Assert.NotEqual("text", tokenType); } - private static WorkspaceEventRestorer WithWorkspaceChangedHandler(Workspace workspace, EventHandler eventHandler) - { - workspace.WorkspaceChanged += eventHandler; - return new WorkspaceEventRestorer(workspace, eventHandler); - } - private static LoggerRestorer WithLogger(ILogger logger) { return new LoggerRestorer(Logger.SetLogger(logger)); @@ -195,23 +192,6 @@ public void LogBlockStart(FunctionId functionId, LogMessage logMessage, int uniq } } - private readonly struct WorkspaceEventRestorer : IDisposable - { - private readonly Workspace _workspace; - private readonly EventHandler _eventHandler; - - public WorkspaceEventRestorer(Workspace workspace, EventHandler eventHandler) - { - _workspace = workspace; - _eventHandler = eventHandler; - } - - public void Dispose() - { - _workspace.WorkspaceChanged -= _eventHandler; - } - } - private readonly struct LoggerRestorer : IDisposable { private readonly ILogger? _logger; diff --git a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb index a17f533e73553..2fd6e35ff465d 100644 --- a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb +++ b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb @@ -4,7 +4,6 @@ Imports Microsoft.CodeAnalysis Imports Microsoft.CodeAnalysis.Shared.TestHooks -Imports Roslyn.Test.Utilities Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework Friend Class WorkspaceChangeWatcher @@ -13,6 +12,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr Private ReadOnly _environment As TestEnvironment Private ReadOnly _asynchronousOperationWaiter As IAsynchronousOperationWaiter Private _changeEvents As New List(Of WorkspaceChangeEventArgs) + Private ReadOnly _workspaceChangedDisposer As IDisposable Public Sub New(environment As TestEnvironment) _environment = environment @@ -20,12 +20,14 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr Dim listenerProvider = environment.ExportProvider.GetExportedValue(Of AsynchronousOperationListenerProvider)() _asynchronousOperationWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace) - AddHandler environment.Workspace.WorkspaceChanged, AddressOf OnWorkspaceChanged + _workspaceChangedDisposer = environment.Workspace.RegisterWorkspaceChangedHandler(AddressOf OnWorkspaceChangedAsync) End Sub - Private Sub OnWorkspaceChanged(sender As Object, e As WorkspaceChangeEventArgs) + Private Function OnWorkspaceChangedAsync(e As WorkspaceChangeEventArgs) As Task _changeEvents.Add(e) - End Sub + + Return Task.CompletedTask + End Function Friend Async Function GetNewChangeEventsAsync() As Task(Of IEnumerable(Of WorkspaceChangeEventArgs)) Await _asynchronousOperationWaiter.ExpeditedWaitAsync() @@ -37,7 +39,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr End Function Public Sub Dispose() Implements IDisposable.Dispose - RemoveHandler _environment.Workspace.WorkspaceChanged, AddressOf OnWorkspaceChanged + _workspaceChangedDisposer.Dispose() End Sub End Class End Namespace diff --git a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs index 7ca45df9a32fc..a2051f0e354da 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -8,6 +8,7 @@ using System.ComponentModel.Composition; using System.Linq; using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Editor; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; @@ -27,7 +28,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Xaml; [Export] -internal sealed partial class XamlProjectService +internal sealed partial class XamlProjectService : IDisposable { private readonly IServiceProvider _serviceProvider; private readonly Workspace _workspace; @@ -36,6 +37,7 @@ internal sealed partial class XamlProjectService private readonly IThreadingContext _threadingContext; private readonly Dictionary _xamlProjects = []; private readonly ConcurrentDictionary _documentIds = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + private IDisposable _documentClosedHandlerDisposer; private RunningDocumentTable? _rdt; private IVsSolution? _vsSolution; @@ -58,7 +60,7 @@ public XamlProjectService( AnalyzerService = analyzerService; - _workspace.DocumentClosed += OnDocumentClosed; + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosedAsync); } public static IXamlDocumentAnalyzerService? AnalyzerService { get; private set; } @@ -188,12 +190,12 @@ public XamlProjectService( return null; } - private void OnDocumentClosed(object sender, DocumentEventArgs e) + private Task OnDocumentClosedAsync(DocumentEventArgs e) { var filePath = e.Document.FilePath; if (filePath == null) { - return; + return Task.CompletedTask; } if (_documentIds.TryGetValue(filePath, out var documentId)) @@ -207,6 +209,8 @@ private void OnDocumentClosed(object sender, DocumentEventArgs e) _documentIds.TryRemove(filePath, out _); } + + return Task.CompletedTask; } private void OnProjectClosing(IVsHierarchy hierarchy) @@ -271,4 +275,9 @@ private void OnDocumentMonikerChanged(uint docCookie, IVsHierarchy hierarchy, st return null; } + + public void Dispose() + { + _documentClosedHandlerDisposer.Dispose(); + } } diff --git a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs index 04560fce22dba..852fe2323f28e 100644 --- a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs +++ b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Threading.Tasks; namespace Microsoft.CodeAnalysis.ExternalAccess.UnitTesting.Api; @@ -16,28 +17,23 @@ public static IDisposable RegisterTextDocumentClosedEventHandler(this Workspace private sealed class EventHandlerWrapper : IDisposable { - private readonly Workspace _workspace; - private readonly EventHandler _handler; - private readonly bool _opened; + private readonly IDisposable _textDocumentOperationDisposer; internal EventHandlerWrapper(Workspace workspace, Action action, bool opened) { - _workspace = workspace; - _handler = (sender, args) => action(new UnitTestingTextDocumentEventArgsWrapper(args)); - _opened = opened; - - if (_opened) - _workspace.TextDocumentOpened += _handler; - else - _workspace.TextDocumentClosed += _handler; + _textDocumentOperationDisposer = opened + ? workspace.RegisterTextDocumentOpenedHandler(HandleAsync) + : workspace.RegisterTextDocumentClosedHandler(HandleAsync); + + Task HandleAsync(TextDocumentEventArgs args) + { + action(new UnitTestingTextDocumentEventArgsWrapper(args)); + + return Task.CompletedTask; + } } public void Dispose() - { - if (_opened) - _workspace.TextDocumentOpened -= _handler; - else - _workspace.TextDocumentClosed -= _handler; - } + => _textDocumentOperationDisposer.Dispose(); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index c80caf069c7e3..fd26353e6ef91 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -40,6 +40,7 @@ public abstract partial class Workspace : IDisposable private readonly IAsynchronousOperationListener _asyncOperationListener; private readonly AsyncBatchingWorkQueue _workQueue; + private readonly AsyncBatchingWorkQueue> _backgroundAsyncWorkQueue; private readonly CancellationTokenSource _workQueueTokenSource = new(); private readonly ITaskSchedulerProvider _taskSchedulerProvider; @@ -86,6 +87,12 @@ protected Workspace(HostServices host, string? workspaceKind) _asyncOperationListener, _workQueueTokenSource.Token); + _backgroundAsyncWorkQueue = new( + TimeSpan.Zero, + ProcessBackgroundWorkQueueAsync, + _asyncOperationListener, + _workQueueTokenSource.Token); + // initialize with empty solution _latestSolution = CreateSolution( SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create()), @@ -596,6 +603,13 @@ protected internal Task ScheduleTask(Action action, string? taskName = "Workspac return _workQueue.WaitUntilCurrentBatchCompletesAsync(); } + [SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "This is a Task wrapper, not an asynchronous method.")] + internal Task ScheduleBackgroundTask(Func action) + { + _backgroundAsyncWorkQueue.AddWork(action); + return _backgroundAsyncWorkQueue.WaitUntilCurrentBatchCompletesAsync(); + } + /// /// Execute a function as a background task, as part of a sequential queue of tasks. /// @@ -737,6 +751,23 @@ await Task.Factory.StartNew(() => }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); } + private static async ValueTask ProcessBackgroundWorkQueueAsync(ImmutableSegmentedList> list, CancellationToken cancellationToken) + { + foreach (var item in list) + { + cancellationToken.ThrowIfCancellationRequested(); + + try + { + await item().ConfigureAwait(false); + } + catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e)) + { + // Ensure we continue onto further items, even if one particular item fails. + } + } + } + #region Host API private static Solution CheckAndAddProjects(Solution solution, IReadOnlyList projects) diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs new file mode 100644 index 0000000000000..061a8e323721c --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +namespace Microsoft.CodeAnalysis; + +internal sealed class WorkspaceEventRegistration(Action unregisterAction) : IDisposable +{ + private readonly Action _unregisterAction = unregisterAction; + + public void Dispose() + => _unregisterAction(); +} diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index 38cba793833a0..e18c973e5c523 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -5,6 +5,7 @@ #nullable disable using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; @@ -89,20 +90,31 @@ protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solutio RaiseEventForHandlers(ev, sender: this, args, FunctionId.Workspace_EventsImmediate); } + var eventHandlerTasks = new List(); + ev = GetEventHandlers(WorkspaceChangeEventName); if (ev.HasHandlers) { args ??= new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); - return this.ScheduleTask(() => + var syncEventHandlersTask = this.ScheduleTask(() => { RaiseEventForHandlers(ev, sender: this, args, FunctionId.Workspace_Events); }, WorkspaceChangeEventName); + + eventHandlerTasks.Add(syncEventHandlersTask); } - else + + var asyncEv = _asyncEventMap.GetEventHandlerSet(WorkspaceChangeEventName); + if (asyncEv.HasHandlers) { - return Task.CompletedTask; + args ??= new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); + var asyncEventHandlersTask = this.ScheduleBackgroundTask(() => RaiseEventForAsyncHandlersAsync(asyncEv, args, FunctionId.Workspace_Events)); + + eventHandlerTasks.Add(asyncEventHandlersTask); } + return Task.WhenAll(eventHandlerTasks); + static void RaiseEventForHandlers( EventMap.EventHandlerSet> handlers, Workspace sender, @@ -114,6 +126,17 @@ static void RaiseEventForHandlers( handlers.RaiseEvent(static (handler, arg) => handler(arg.sender, arg.args), (sender, args)); } } + + static async Task RaiseEventForAsyncHandlersAsync( + AsyncEventMap.AsyncEventHandlerSet asyncHandlers, + WorkspaceChangeEventArgs args, + FunctionId functionId) + { + using (Logger.LogBlock(functionId, (s, p, d, k) => $"{s.Id} - {p} - {d} {args.Kind.ToString()}", args.NewSolution, args.ProjectId, args.DocumentId, args.Kind, CancellationToken.None)) + { + await asyncHandlers.RaiseEventAsync(args).ConfigureAwait(false); + } + } } /// @@ -146,6 +169,7 @@ protected internal virtual void OnWorkspaceFailed(WorkspaceDiagnostic diagnostic /// /// An event that is fired when a is opened in the editor. /// + [Obsolete("This member is obsolete. Use the RegisterDocumentOpenedHandler(Func) overload instead.", error: true)] public event EventHandler DocumentOpened { add @@ -165,6 +189,7 @@ protected Task RaiseDocumentOpenedEventAsync(Document document) /// /// An event that is fired when any is opened in the editor. /// + [Obsolete("This member is obsolete. Use the RegisterTextDocumentOpenedHandler(Func) overload instead.", error: true)] public event EventHandler TextDocumentOpened { add @@ -188,23 +213,34 @@ private Task RaiseTextDocumentOpenedOrClosedEventAsync(); var ev = GetEventHandlers(eventName); + if (ev.HasHandlers && document != null) { - return this.ScheduleTask(() => + var syncEventHandlerTask = this.ScheduleTask(() => { ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); }, eventName); + + eventHandlerTasks.Add(syncEventHandlerTask); } - else + + var asyncEv = _asyncEventMap.GetEventHandlerSet(eventName); + if (asyncEv.HasHandlers) { - return Task.CompletedTask; + var asyncEventHandlersTask = this.ScheduleBackgroundTask(() => asyncEv.RaiseEventAsync(args)); + + eventHandlerTasks.Add(asyncEventHandlersTask); } + + return Task.WhenAll(eventHandlerTasks); } /// /// An event that is fired when a is closed in the editor. /// + [Obsolete("This member is obsolete. Use the RegisterDocumentClosedHandler(Func) overload instead.", error: true)] public event EventHandler DocumentClosed { add @@ -224,6 +260,7 @@ protected Task RaiseDocumentClosedEventAsync(Document document) /// /// An event that is fired when any is closed in the editor. /// + [Obsolete("This member is obsolete. Use the RegisterTextDocumentClosedHandler(Func) overload instead.", error: true)] public event EventHandler TextDocumentClosed { add @@ -244,6 +281,7 @@ protected Task RaiseTextDocumentClosedEventAsync(TextDocument document) /// An event that is fired when the active context document associated with a buffer /// changes. /// + [Obsolete("This member is obsolete. Use the RegisterActiveContextChangedHandler(Func) overload instead.", error: true)] public event EventHandler DocumentActiveContextChanged { add @@ -263,22 +301,36 @@ protected Task RaiseDocumentActiveContextChangedEventAsync(Document document) protected Task RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer sourceTextContainer, DocumentId oldActiveContextDocumentId, DocumentId newActiveContextDocumentId) { + if (sourceTextContainer == null || oldActiveContextDocumentId == null || newActiveContextDocumentId == null) + return Task.CompletedTask; + + var eventHandlerTasks = new List(); var ev = GetEventHandlers(DocumentActiveContextChangedName); - if (ev.HasHandlers && sourceTextContainer != null && oldActiveContextDocumentId != null && newActiveContextDocumentId != null) + DocumentActiveContextChangedEventArgs? args = null; + + if (ev.HasHandlers) { // Capture the current solution snapshot (inside the _serializationLock of OnDocumentContextUpdated) var currentSolution = this.CurrentSolution; + args ??= new DocumentActiveContextChangedEventArgs(currentSolution, sourceTextContainer, oldActiveContextDocumentId, newActiveContextDocumentId); - return this.ScheduleTask(() => + var syncEventHandlerTask = this.ScheduleTask(() => { - var args = new DocumentActiveContextChangedEventArgs(currentSolution, sourceTextContainer, oldActiveContextDocumentId, newActiveContextDocumentId); ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); }, "Workspace.WorkspaceChanged"); + + eventHandlerTasks.Add(syncEventHandlerTask); } - else + + var asyncEv = _asyncEventMap.GetEventHandlerSet(DocumentActiveContextChangedName); + if (asyncEv.HasHandlers) { - return Task.CompletedTask; + var asyncEventHandlersTask = this.ScheduleBackgroundTask(() => asyncEv.RaiseEventAsync(args)); + + eventHandlerTasks.Add(asyncEventHandlersTask); } + + return Task.WhenAll(eventHandlerTasks); } private EventMap.EventHandlerSet> GetEventHandlers(string eventName) where T : EventArgs diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs new file mode 100644 index 0000000000000..a82f381aa6915 --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading.Tasks; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis; + +public abstract partial class Workspace +{ + private readonly AsyncEventMap _asyncEventMap = new(); + + /// + /// Registers a handler that is fired whenever the current solution is changed. + /// + internal WorkspaceEventRegistration RegisterWorkspaceChangedHandler(Func handler) + => RegisterAsyncHandler(WorkspaceChangeEventName, handler); + + /// + /// Registers a handler that is fired when a is opened in the editor. + /// + internal WorkspaceEventRegistration RegisterDocumentOpenedHandler(Func handler) + => RegisterAsyncHandler(DocumentOpenedEventName, handler); + + /// + /// Registers a handler that is fired when a is closed in the editor. + /// + internal WorkspaceEventRegistration RegisterDocumentClosedHandler(Func handler) + => RegisterAsyncHandler(DocumentClosedEventName, handler); + + /// + /// Registers a handler that is fired when any is opened in the editor. + /// + internal WorkspaceEventRegistration RegisterTextDocumentOpenedHandler(Func handler) + => RegisterAsyncHandler(TextDocumentOpenedEventName, handler); + + /// + /// Registers a handler that is fired when any is closed in the editor. + /// + internal WorkspaceEventRegistration RegisterTextDocumentClosedHandler(Func handler) + => RegisterAsyncHandler(TextDocumentClosedEventName, handler); + + /// + /// Registers a handler that is fired when the active context document associated with a buffer + /// changes. + /// + internal WorkspaceEventRegistration RegisterDocumentActiveContextChangedHandler(Func handler) + => RegisterAsyncHandler(DocumentActiveContextChangedName, handler); + + private WorkspaceEventRegistration RegisterAsyncHandler(string eventName, Func handler) + where TEventArgs : EventArgs + { + _asyncEventMap.AddAsyncEventHandler(eventName, handler); + + return new WorkspaceEventRegistration(() => _asyncEventMap.RemoveAsyncEventHandler(eventName, handler)); + } +} diff --git a/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs b/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs index 617c0cdcc393d..522c6870cae00 100644 --- a/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs +++ b/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs @@ -420,14 +420,15 @@ public async Task TestChangeDocumentName_TryApplyChanges() Assert.Equal(newName, changedDoc.Name); var tcs = new TaskCompletionSource(); - ws.WorkspaceChanged += (s, args) => + using var _ = ws.RegisterWorkspaceChangedHandler(args => { if (args.Kind == WorkspaceChangeKind.DocumentInfoChanged && args.DocumentId == originalDoc.Id) { tcs.SetResult(true); } - }; + return Task.CompletedTask; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -453,14 +454,16 @@ public async Task TestChangeDocumentFolders_TryApplyChanges() Assert.Equal("B", changedDoc.Folders[1]); var tcs = new TaskCompletionSource(); - ws.WorkspaceChanged += (s, args) => + using var _ = ws.RegisterWorkspaceChangedHandler(args => { if (args.Kind == WorkspaceChangeKind.DocumentInfoChanged && args.DocumentId == originalDoc.Id) { tcs.SetResult(true); } - }; + + return Task.CompletedTask; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -487,14 +490,15 @@ public async Task TestChangeDocumentFilePath_TryApplyChanges() Assert.Equal(newPath, changedDoc.FilePath); var tcs = new TaskCompletionSource(); - ws.WorkspaceChanged += (s, args) => + using var _ = ws.RegisterWorkspaceChangedHandler(args => { if (args.Kind == WorkspaceChangeKind.DocumentInfoChanged && args.DocumentId == originalDoc.Id) { tcs.SetResult(true); } - }; + return Task.CompletedTask; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -518,14 +522,15 @@ public async Task TestChangeDocumentSourceCodeKind_TryApplyChanges() Assert.Equal(SourceCodeKind.Script, changedDoc.SourceCodeKind); var tcs = new TaskCompletionSource(); - ws.WorkspaceChanged += (s, args) => + using var _ = ws.RegisterWorkspaceChangedHandler(args => { if (args.Kind == WorkspaceChangeKind.DocumentInfoChanged && args.DocumentId == originalDoc.Id) { tcs.SetResult(true); } - }; + return Task.CompletedTask; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); diff --git a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs index da5f1a3cc3091..94a2fdfa2d4da 100644 --- a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs +++ b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.Text; using Roslyn.Test.Utilities; @@ -54,10 +55,10 @@ public static DocumentId CreateDocumentId(this ProjectId projectId, string name, public static IEnumerable GetProjectsByName(this Solution solution, string name) => solution.Projects.Where(p => string.Compare(p.Name, name, StringComparison.OrdinalIgnoreCase) == 0); - internal static EventWaiter VerifyWorkspaceChangedEvent(this Workspace workspace, Action action) + internal static EventWaiter VerifyWorkspaceChangedEvent(this Workspace workspace, Func action) { var wew = new EventWaiter(); - workspace.WorkspaceChanged += wew.Wrap((sender, args) => action(args)); + workspace.RegisterWorkspaceChangedHandler(wew.Wrap(action)); return wew; } } diff --git a/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs b/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs index 9128c40c08781..b5195d60ba506 100644 --- a/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs +++ b/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs @@ -2218,6 +2218,8 @@ public async Task TestWorkspaceChangedEvent() Assert.Equal(expectedEventKind, args.Kind); Assert.NotNull(args.NewSolution); Assert.NotSame(originalSolution, args.NewSolution); + + return Task.CompletedTask; }); // change document text (should fire SolutionChanged event) var doc = workspace.CurrentSolution.Projects.First().Documents.First(); @@ -2248,6 +2250,8 @@ public async Task TestWorkspaceChangedWeakEvent() Assert.Equal(expectedEventKind, args.Kind); Assert.NotNull(args.NewSolution); Assert.NotSame(originalSolution, args.NewSolution); + + return Task.CompletedTask; }); // change document text (should fire SolutionChanged event) var doc = workspace.CurrentSolution.Projects.First().Documents.First(); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index d462c1f17317a..6b09bd899d037 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -485,6 +485,7 @@ + diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs new file mode 100644 index 0000000000000..dc883442f880b --- /dev/null +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs @@ -0,0 +1,73 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading.Tasks; + +namespace Roslyn.Utilities; + +internal sealed class AsyncEventMap +{ + private readonly NonReentrantLock _guard = new(); + private readonly Dictionary _eventNameToHandlerSet = []; + + public void AddAsyncEventHandler(string eventName, Func asyncHandler) + where TEventArgs : EventArgs + { + using (_guard.DisposableWait()) + { + _eventNameToHandlerSet[eventName] = _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) + ? ((AsyncEventHandlerSet)handlers).WithHandler(asyncHandler) + : new AsyncEventHandlerSet([asyncHandler]); + } + } + + public void RemoveAsyncEventHandler(string eventName, Func asyncHandler) + where TEventArgs : EventArgs + { + using (_guard.DisposableWait()) + { + if (_eventNameToHandlerSet.TryGetValue(eventName, out var handlers)) + _eventNameToHandlerSet[eventName] = ((AsyncEventHandlerSet)handlers).WithHandlerRemoved(asyncHandler); + } + } + + public AsyncEventHandlerSet GetEventHandlerSet(string eventName) + where TEventArgs : EventArgs + { + using (_guard.DisposableWait()) + { + return _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) + ? (AsyncEventHandlerSet)handlers + : AsyncEventHandlerSet.Empty; + } + } + + public sealed class AsyncEventHandlerSet(ImmutableArray> asyncHandlers) + where TEventArgs : EventArgs + { + private readonly ImmutableArray> _asyncHandlers = asyncHandlers; + public static readonly AsyncEventHandlerSet Empty = new([]); + + public AsyncEventHandlerSet WithHandler(Func asyncHandler) + => new(_asyncHandlers.Add(asyncHandler)); + + public AsyncEventHandlerSet WithHandlerRemoved(Func asyncHandler) + { + var newAsyncHandlers = _asyncHandlers.RemoveAll(r => r.Equals(asyncHandler)); + + return newAsyncHandlers.IsEmpty ? Empty : new(newAsyncHandlers); + } + + public bool HasHandlers => !_asyncHandlers.IsEmpty; + + public async Task RaiseEventAsync(TEventArgs arg) + { + foreach (var asyncHandler in _asyncHandlers) + await asyncHandler(arg).ConfigureAwait(false); + } + } +} From 71b1d43a596d05885555ff201165b9541c06cb0b Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Sun, 13 Apr 2025 06:54:55 -0700 Subject: [PATCH 02/17] 1) Move Dispose methods closer to construction 2) Remove Obsolete attribute as that will require making the new registration public 3) Remove delegate passed to WorkspaceRegistration construction. 4) Switch new code to use SemaphoreSlim instead of NonReentrantLock 5) Rename methods on AsyncEventHandlerSet --- ...tractCreateServicesOnTextViewConnection.cs | 8 +++--- .../Framework/WorkspaceChangeWatcher.vb | 8 +++--- .../Impl/Implementation/XamlProjectService.cs | 8 +++--- .../Workspace/WorkspaceEventRegistration.cs | 25 ++++++++++++++++--- .../Portable/Workspace/Workspace_Events.cs | 5 ---- .../Workspace/Workspace_EventsAsync.cs | 2 +- .../Compiler/Core/Utilities/AsyncEventMap.cs | 11 ++++---- 7 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs index 64c357f274eec..818580006b88f 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs @@ -60,6 +60,9 @@ public AbstractCreateServicesOnTextViewConnection( _workspaceDocumentOpenedDisposer = Workspace.RegisterDocumentOpenedHandler(QueueWorkOnDocumentOpenedAsync); } + public void Dispose() + => _workspaceDocumentOpenedDisposer.Dispose(); + void IWpfTextViewConnectionListener.SubjectBuffersConnected(IWpfTextView textView, ConnectionReason reason, Collection subjectBuffers) { if (!_initialized) @@ -115,9 +118,4 @@ private void InitializePerVSSessionServices() // https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1242321 languageServices.GetService()?.LoadImportedProviders(); } - - public void Dispose() - { - _workspaceDocumentOpenedDisposer?.Dispose(); - } } diff --git a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb index 2fd6e35ff465d..cd58ae1f02252 100644 --- a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb +++ b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb @@ -23,6 +23,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr _workspaceChangedDisposer = environment.Workspace.RegisterWorkspaceChangedHandler(AddressOf OnWorkspaceChangedAsync) End Sub + Public Sub Dispose() Implements IDisposable.Dispose + _workspaceChangedDisposer.Dispose() + End Sub + Private Function OnWorkspaceChangedAsync(e As WorkspaceChangeEventArgs) As Task _changeEvents.Add(e) @@ -37,9 +41,5 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr _changeEvents = New List(Of WorkspaceChangeEventArgs)() Return changeEvents End Function - - Public Sub Dispose() Implements IDisposable.Dispose - _workspaceChangedDisposer.Dispose() - End Sub End Class End Namespace diff --git a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs index a2051f0e354da..04c2192699045 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -63,6 +63,9 @@ public XamlProjectService( _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosedAsync); } + public void Dispose() + => _documentClosedHandlerDisposer.Dispose(); + public static IXamlDocumentAnalyzerService? AnalyzerService { get; private set; } public DocumentId? TrackOpenDocument(string filePath) @@ -275,9 +278,4 @@ private void OnDocumentMonikerChanged(uint docCookie, IVsHierarchy hierarchy, st return null; } - - public void Dispose() - { - _documentClosedHandlerDisposer.Dispose(); - } } diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs index 061a8e323721c..0add07cf38edf 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -3,13 +3,30 @@ // See the LICENSE file in the project root for more information. using System; +using System.Threading.Tasks; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis; -internal sealed class WorkspaceEventRegistration(Action unregisterAction) : IDisposable +internal abstract class WorkspaceEventRegistration : IDisposable { - private readonly Action _unregisterAction = unregisterAction; + public static WorkspaceEventRegistration Create(AsyncEventMap asyncEventMap, string eventName, Func handler) + where TEventArgs : EventArgs + { + return new WorkspaceEventRegistrationImpl(asyncEventMap, eventName, handler); + } - public void Dispose() - => _unregisterAction(); + public abstract void Dispose(); + + private sealed class WorkspaceEventRegistrationImpl(AsyncEventMap asyncEventMap, string eventName, Func handler) + : WorkspaceEventRegistration + where TEventArgs : EventArgs + { + private readonly AsyncEventMap _asyncEventMap = asyncEventMap; + private readonly string _eventName = eventName; + private readonly Func _handler = handler; + + public override void Dispose() + => _asyncEventMap.RemoveAsyncEventHandler(_eventName, _handler); + } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index e18c973e5c523..75427f078cc47 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -169,7 +169,6 @@ protected internal virtual void OnWorkspaceFailed(WorkspaceDiagnostic diagnostic /// /// An event that is fired when a is opened in the editor. /// - [Obsolete("This member is obsolete. Use the RegisterDocumentOpenedHandler(Func) overload instead.", error: true)] public event EventHandler DocumentOpened { add @@ -189,7 +188,6 @@ protected Task RaiseDocumentOpenedEventAsync(Document document) /// /// An event that is fired when any is opened in the editor. /// - [Obsolete("This member is obsolete. Use the RegisterTextDocumentOpenedHandler(Func) overload instead.", error: true)] public event EventHandler TextDocumentOpened { add @@ -240,7 +238,6 @@ private Task RaiseTextDocumentOpenedOrClosedEventAsync /// An event that is fired when a is closed in the editor. /// - [Obsolete("This member is obsolete. Use the RegisterDocumentClosedHandler(Func) overload instead.", error: true)] public event EventHandler DocumentClosed { add @@ -260,7 +257,6 @@ protected Task RaiseDocumentClosedEventAsync(Document document) /// /// An event that is fired when any is closed in the editor. /// - [Obsolete("This member is obsolete. Use the RegisterTextDocumentClosedHandler(Func) overload instead.", error: true)] public event EventHandler TextDocumentClosed { add @@ -281,7 +277,6 @@ protected Task RaiseTextDocumentClosedEventAsync(TextDocument document) /// An event that is fired when the active context document associated with a buffer /// changes. /// - [Obsolete("This member is obsolete. Use the RegisterActiveContextChangedHandler(Func) overload instead.", error: true)] public event EventHandler DocumentActiveContextChanged { add diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs index a82f381aa6915..d88c0a705efca 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs @@ -54,6 +54,6 @@ private WorkspaceEventRegistration RegisterAsyncHandler(string event { _asyncEventMap.AddAsyncEventHandler(eventName, handler); - return new WorkspaceEventRegistration(() => _asyncEventMap.RemoveAsyncEventHandler(eventName, handler)); + return WorkspaceEventRegistration.Create(_asyncEventMap, eventName, handler); } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs index dc883442f880b..19ad943944893 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs @@ -5,13 +5,14 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Threading; using System.Threading.Tasks; namespace Roslyn.Utilities; internal sealed class AsyncEventMap { - private readonly NonReentrantLock _guard = new(); + private readonly SemaphoreSlim _guard = new(initialCount: 1); private readonly Dictionary _eventNameToHandlerSet = []; public void AddAsyncEventHandler(string eventName, Func asyncHandler) @@ -20,7 +21,7 @@ public void AddAsyncEventHandler(string eventName, Func)handlers).WithHandler(asyncHandler) + ? ((AsyncEventHandlerSet)handlers).AddHandler(asyncHandler) : new AsyncEventHandlerSet([asyncHandler]); } } @@ -31,7 +32,7 @@ public void RemoveAsyncEventHandler(string eventName, Func)handlers).WithHandlerRemoved(asyncHandler); + _eventNameToHandlerSet[eventName] = ((AsyncEventHandlerSet)handlers).RemoveHandler(asyncHandler); } } @@ -52,10 +53,10 @@ public sealed class AsyncEventHandlerSet(ImmutableArray> _asyncHandlers = asyncHandlers; public static readonly AsyncEventHandlerSet Empty = new([]); - public AsyncEventHandlerSet WithHandler(Func asyncHandler) + public AsyncEventHandlerSet AddHandler(Func asyncHandler) => new(_asyncHandlers.Add(asyncHandler)); - public AsyncEventHandlerSet WithHandlerRemoved(Func asyncHandler) + public AsyncEventHandlerSet RemoveHandler(Func asyncHandler) { var newAsyncHandlers = _asyncHandlers.RemoveAll(r => r.Equals(asyncHandler)); From c2e874f89531356e58abf24748306711ca197f91 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 15:38:52 -0700 Subject: [PATCH 03/17] Remove the asynchronous eventing and switch back to a synchronous model that takes in an eventoptions that allows picking the callback thread --- src/Compilers/Test/Core/FX/EventWaiter.cs | 10 +- .../WorkspaceTests_EditorFeatures.cs | 50 +-- ...lassificationTaggerProvider.TagComputer.cs | 24 +- .../ActiveStatementTrackingService.cs | 14 +- .../Core/Remote/SolutionChecksumUpdater.cs | 13 +- ...DocumentActiveContextChangedEventSource.cs | 11 +- ...sticAnalyzerService_IncrementalAnalyzer.cs | 11 +- ...tractCreateServicesOnTextViewConnection.cs | 6 +- .../MiscellaneousFilesWorkspace.cs | 6 +- ...xtViewWindowVerifierInProcessExtensions.cs | 1 - .../Framework/WorkspaceChangeWatcher.vb | 8 +- .../Impl/Implementation/XamlProjectService.cs | 13 +- .../Api/UnitTestingWorkspaceExtensions.cs | 13 +- .../Core/Portable/Workspace/Workspace.cs | 68 ++-- .../Workspace/WorkspaceEventRegistration.cs | 27 +- .../Portable/Workspace/Workspace_Events.cs | 292 ++++-------------- .../Workspace/Workspace_EventsAsync.cs | 59 ---- .../Workspace/Workspace_EventsLegacy.cs | 100 ++++++ .../Workspace/Workspace_Registration.cs | 6 +- .../WorkspaceTests/AdhocWorkspaceTests.cs | 5 - .../CoreTestUtilities/WorkspaceExtensions.cs | 3 +- .../Test/VisualStudioMSBuildWorkspaceTests.cs | 4 - .../Core/CompilerExtensions.projitems | 2 +- .../Compiler/Core/Utilities/AsyncEventMap.cs | 74 ----- .../Compiler/Core/Utilities/EventMap.cs | 166 ++++------ .../Core/Utilities/WorkspaceEventOptions.cs | 11 + 26 files changed, 337 insertions(+), 660 deletions(-) delete mode 100644 src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs create mode 100644 src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs delete mode 100644 src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs create mode 100644 src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WorkspaceEventOptions.cs diff --git a/src/Compilers/Test/Core/FX/EventWaiter.cs b/src/Compilers/Test/Core/FX/EventWaiter.cs index c753674c7db0e..2035065e6e703 100644 --- a/src/Compilers/Test/Core/FX/EventWaiter.cs +++ b/src/Compilers/Test/Core/FX/EventWaiter.cs @@ -5,11 +5,7 @@ #nullable disable using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading; -using System.Threading.Tasks; namespace Roslyn.Test.Utilities { @@ -55,13 +51,13 @@ public EventHandler Wrap(EventHandler input) }; } - public Func Wrap(Func input) + public Action Wrap(Action input) { return (args) => { try { - return input(args); + input(args); } catch (Exception ex) { @@ -71,8 +67,6 @@ public Func Wrap(Func input) { _eventSignal.Set(); } - - return Task.CompletedTask; }; } diff --git a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs index 3409388915ffb..2ecfeca3594eb 100644 --- a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs +++ b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs @@ -65,7 +65,6 @@ public async Task TestEmptySolutionUpdateDoesNotFireEvents() using var _ = workspace.RegisterWorkspaceChangedHandler(e => { workspaceChanged = true; - return Task.CompletedTask; }); // make an 'empty' update by claiming something changed, but its the same as before @@ -818,26 +817,22 @@ public async Task TestDocumentEvents() using var closeWaiter = new EventWaiter(); using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var documentOpenedEventHandlerAsync = openWaiter.Wrap( + var documentOpenedEventHandler = openWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, "The document given to the 'DocumentOpened' event handler did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var documentClosedEventHandlerAsync = closeWaiter.Wrap( + var documentClosedEventHandler = closeWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, "The document given to the 'DocumentClosed' event handler did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandlerAsync); - var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandlerAsync); + var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandler); + var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandler); workspace.OpenDocument(document.Id); workspace.CloseDocument(document.Id); @@ -894,26 +889,22 @@ public async Task TestSourceGeneratedDocumentEvents() using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var documentOpenedEventHandlerAsync = openWaiter.Wrap( + var documentOpenedEventHandler = openWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, $"The source generated document given to the '{nameof(Workspace.RegisterDocumentOpenedHandler)}' did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var documentClosedEventHandlerAsync = closeWaiter.Wrap( + var documentClosedEventHandler = closeWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, $"The source generated document given to the '{nameof(Workspace.RegisterDocumentClosedHandler)}' did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandlerAsync); - var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandlerAsync); + var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandler); + var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandler); workspace.OpenSourceGeneratedDocument(document.Id); var sourceGeneratedDocumentId = workspace.GetDocumentIdInCurrentContext(document.GetOpenTextContainer()); @@ -968,26 +959,22 @@ public async Task TestAdditionalDocumentEvents() using var closeWaiter = new EventWaiter(); using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var textDocumentOpenedEventHandlerAsync = openWaiter.Wrap( + var textDocumentOpenedEventHandler = openWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, "The document given to the 'RegisterTextDocumentOpenedHandler' event handler did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var textDocumentClosedEventHandlerAsync = closeWaiter.Wrap( + var textDocumentClosedEventHandler = closeWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, "The document given to the 'RegisterTextDocumentClosedHandler' event handler did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandlerAsync); - var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandlerAsync); + var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandler); + var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandler); workspace.OpenAdditionalDocument(document.Id); workspace.CloseAdditionalDocument(document.Id); @@ -1039,26 +1026,22 @@ public async Task TestAnalyzerConfigDocumentEvents() using var closeWaiter = new EventWaiter(); using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. - var textDocumentOpenedEventHandlerAsync = openWaiter.Wrap( + var textDocumentOpenedEventHandler = openWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, "The document given to the 'AnalyzerConfigDocumentOpened' event handler did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var textDocumentClosedEventHandlerAsync = closeWaiter.Wrap( + var textDocumentClosedEventHandler = closeWaiter.Wrap( args => { Assert.True(args.Document.Id == document.Id, "The document given to the 'AnalyzerConfigDocumentClosed' event handler did not have the same id as the one created for the test."); - - return Task.CompletedTask; }); - var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandlerAsync); - var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandlerAsync); + var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandler); + var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandler); workspace.OpenAnalyzerConfigDocument(document.Id); workspace.CloseAnalyzerConfigDocument(document.Id); @@ -1468,7 +1451,6 @@ public async Task TestLinkedFilesStayInSync() { Assert.Equal(WorkspaceChangeKind.DocumentChanged, e.Kind); eventArgs.Add(e); - return Task.CompletedTask; }); var originalDocumentId = workspace.GetOpenDocumentIds().Single(id => !workspace.GetTestDocument(id).IsLinkFile); diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 1758cc44e3464..734e6a78b8e7f 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -226,8 +226,8 @@ private void ConnectToWorkspace(Workspace workspace) _taggerProvider.ThreadingContext.ThrowIfNotOnUIThread(); _workspace = workspace; - _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChangedAsync); - _workspaceDocumentActiveContextChangedDisposer = _workspace.RegisterDocumentActiveContextChangedHandler(this.OnDocumentActiveContextChangedAsync); + _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChanged); + _workspaceDocumentActiveContextChangedDisposer = _workspace.RegisterDocumentActiveContextChangedHandler(this.OnDocumentActiveContextChanged); // Now that we've connected to the workspace, kick off work to reclassify this buffer. _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); @@ -269,34 +269,34 @@ private void OnSubjectBufferChanged(object? sender, TextContentChangedEventArgs _workQueue.AddWork(args.After); } - private Task OnDocumentActiveContextChangedAsync(DocumentActiveContextChangedEventArgs args) + private void OnDocumentActiveContextChanged(DocumentActiveContextChangedEventArgs args) { if (_workspace == null) - return Task.CompletedTask; + return; var documentId = args.NewActiveContextDocumentId; var bufferDocumentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); if (bufferDocumentId != documentId) - return Task.CompletedTask; + return; _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); - return Task.CompletedTask; + return; } - private Task OnWorkspaceChangedAsync(WorkspaceChangeEventArgs args) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs args) { // We may be getting an event for a workspace we already disconnected from. If so, // ignore them. We won't be able to find the Document corresponding to our text buffer, // so we can't reasonably classify this anyways. if (args.NewSolution.Workspace != _workspace) - return Task.CompletedTask; + return; if (args.Kind != WorkspaceChangeKind.ProjectChanged) - return Task.CompletedTask; + return; var documentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); if (args.ProjectId != documentId?.ProjectId) - return Task.CompletedTask; + return; var oldProject = args.OldSolution.GetProject(args.ProjectId); var newProject = args.NewSolution.GetProject(args.ProjectId); @@ -304,10 +304,10 @@ private Task OnWorkspaceChangedAsync(WorkspaceChangeEventArgs args) // In case of parse options change reclassify the doc as it may have affected things // like preprocessor directives. if (Equals(oldProject?.ParseOptions, newProject?.ParseOptions)) - return Task.CompletedTask; + return; _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); - return Task.CompletedTask; + return; } #endregion diff --git a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs index 1af1bf7c181a4..dc8667661f612 100644 --- a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs +++ b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs @@ -18,8 +18,8 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; using Microsoft.VisualStudio.Text; @@ -123,8 +123,8 @@ public TrackingSession(Workspace workspace, IActiveStatementSpanFactory spanProv _spanProvider = spanProvider; _compileTimeSolutionProvider = workspace.Services.GetRequiredService(); - _documentOpenedHandlerDisposer = _workspace.RegisterDocumentOpenedHandler(DocumentOpenedAsync); - _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(DocumentClosedAsync); + _documentOpenedHandlerDisposer = _workspace.RegisterDocumentOpenedHandler(DocumentOpened); + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(DocumentClosed); } internal Dictionary> Test_GetTrackingSpans() @@ -144,7 +144,7 @@ public void EndTracking() } } - private Task DocumentClosedAsync(DocumentEventArgs e) + private void DocumentClosed(DocumentEventArgs e) { if (e.Document.FilePath != null) { @@ -153,15 +153,11 @@ private Task DocumentClosedAsync(DocumentEventArgs e) _trackingSpans.Remove(e.Document.FilePath); } } - - return Task.CompletedTask; } - private Task DocumentOpenedAsync(DocumentEventArgs e) + private void DocumentOpened(DocumentEventArgs e) { _ = TrackActiveSpansAsync(e.Document); - - return Task.CompletedTask; } private async Task TrackActiveSpansAsync(Document designTimeDocument) diff --git a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs index 5c9aed82d0e7b..f7e8b9e3e2269 100644 --- a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs +++ b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs @@ -45,6 +45,7 @@ internal sealed class SolutionChecksumUpdater private readonly object _gate = new(); private bool _isSynchronizeWorkspacePaused; private readonly IDisposable _workspaceChangedDisposer; + private readonly IDisposable _workspaceChangedImmediateDisposer; private readonly CancellationToken _shutdownToken; @@ -80,8 +81,8 @@ public SolutionChecksumUpdater( shutdownToken); // start listening workspace change event - _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChangedAsync); - _workspace.WorkspaceChangedImmediate += OnWorkspaceChangedImmediate; + _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChanged); + _workspaceChangedImmediateDisposer = _workspace.RegisterWorkspaceChangedImmediateHandler(OnWorkspaceChangedImmediate); _documentTrackingService.ActiveDocumentChanged += OnActiveDocumentChanged; if (_globalOperationService != null) @@ -102,7 +103,7 @@ public void Shutdown() _documentTrackingService.ActiveDocumentChanged -= OnActiveDocumentChanged; _workspaceChangedDisposer.Dispose(); - _workspace.WorkspaceChangedImmediate -= OnWorkspaceChangedImmediate; + _workspaceChangedImmediateDisposer.Dispose(); if (_globalOperationService != null) { @@ -137,7 +138,7 @@ private void ResumeSynchronizingPrimaryWorkspace() } } - private Task OnWorkspaceChangedAsync(WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs _) { // Check if we're currently paused. If so ignore this notification. We don't want to any work in response // to whatever the workspace is doing. @@ -146,11 +147,9 @@ private Task OnWorkspaceChangedAsync(WorkspaceChangeEventArgs e) if (!_isSynchronizeWorkspacePaused) _synchronizeWorkspaceQueue.AddWork(); } - - return Task.CompletedTask; } - private void OnWorkspaceChangedImmediate(object? sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChangedImmediate(WorkspaceChangeEventArgs e) { if (e.Kind == WorkspaceChangeKind.DocumentChanged) { diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs index d47239c6c1fd8..89d7511ff297d 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Threading.Tasks; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; @@ -16,23 +15,17 @@ private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subject private IDisposable? _documentActiveContextChangedDisposer; protected override void ConnectToWorkspace(Workspace workspace) - { - _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChangedAsync); - } + => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged); protected override void DisconnectFromWorkspace(Workspace workspace) => _documentActiveContextChangedDisposer?.Dispose(); - private Task OnDocumentActiveContextChangedAsync(DocumentActiveContextChangedEventArgs e) + private void OnDocumentActiveContextChanged(DocumentActiveContextChangedEventArgs e) { var document = SubjectBuffer.AsTextContainer().GetOpenDocumentInCurrentContext(); if (document != null && document.Id == e.NewActiveContextDocumentId) - { this.RaiseChanged(); - } - - return Task.CompletedTask; } } } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs index dd891f763286e..74a5ef511712f 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Threading.Tasks; - namespace Microsoft.CodeAnalysis.Diagnostics; internal sealed partial class DiagnosticAnalyzerService @@ -16,15 +14,8 @@ private DiagnosticIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace worksp private DiagnosticIncrementalAnalyzer CreateIncrementalAnalyzerCallback(Workspace workspace) { // subscribe to active context changed event for new workspace - _ = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChangedAsync); + _ = workspace.RegisterDocumentActiveContextChangedHandler(args => RequestDiagnosticRefresh()); return new DiagnosticIncrementalAnalyzer(this, AnalyzerInfoCache, this.GlobalOptions); } - - private Task OnDocumentActiveContextChangedAsync(DocumentActiveContextChangedEventArgs e) - { - RequestDiagnosticRefresh(); - - return Task.CompletedTask; - } } diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs index 818580006b88f..088390c32f684 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs @@ -57,7 +57,7 @@ public AbstractCreateServicesOnTextViewConnection( listenerProvider.GetListener(FeatureAttribute.CompletionSet), threadingContext.DisposalToken); - _workspaceDocumentOpenedDisposer = Workspace.RegisterDocumentOpenedHandler(QueueWorkOnDocumentOpenedAsync); + _workspaceDocumentOpenedDisposer = Workspace.RegisterDocumentOpenedHandler(QueueWorkOnDocumentOpened); } public void Dispose() @@ -100,12 +100,10 @@ private async ValueTask BatchProcessProjectsWithOpenedDocumentAsync(ImmutableSeg } } - private Task QueueWorkOnDocumentOpenedAsync(DocumentEventArgs e) + private void QueueWorkOnDocumentOpened(DocumentEventArgs e) { if (e.Document.Project.Language == _languageName) _workQueue.AddWork(e.Document.Project.Id); - - return Task.CompletedTask; } private void InitializePerVSSessionServices() diff --git a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs index 635b2e643bad0..30c4b6cdbadc1 100644 --- a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs +++ b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs @@ -22,6 +22,7 @@ using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.TextManager.Interop; using Roslyn.Utilities; +using static Roslyn.Utilities.EventMap; namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; @@ -172,7 +173,10 @@ private void Registration_WorkspaceChanged(object sender, EventArgs e) // to the RDT in the background thread. Since this is all asynchronous a bit more asynchrony is fine. if (!_threadingContext.JoinableTaskContext.IsOnMainThread) { - ScheduleTask(() => Registration_WorkspaceChanged(sender, e)); + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => Registration_WorkspaceChanged(sender, e), WorkspaceEventOptions.MainThreadDependent); + var handlerSet = new EventHandlerSet(handlerAndOptions); + + ScheduleTask(e, handlerSet); return; } diff --git a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs index 256e8122a0f50..fbaae7e23fd8a 100644 --- a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs +++ b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs @@ -67,7 +67,6 @@ public static async Task CodeActionAsync( using var _ = workspace.RegisterWorkspaceChangedHandler(e => { events.Add(e); - return Task.CompletedTask; }); await textViewWindowVerifier.TestServices.Editor.ShowLightBulbAsync(cancellationToken); diff --git a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb index cd58ae1f02252..3d6a2096f9c74 100644 --- a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb +++ b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb @@ -20,18 +20,16 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr Dim listenerProvider = environment.ExportProvider.GetExportedValue(Of AsynchronousOperationListenerProvider)() _asynchronousOperationWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace) - _workspaceChangedDisposer = environment.Workspace.RegisterWorkspaceChangedHandler(AddressOf OnWorkspaceChangedAsync) + _workspaceChangedDisposer = environment.Workspace.RegisterWorkspaceChangedHandler(AddressOf OnWorkspaceChanged) End Sub Public Sub Dispose() Implements IDisposable.Dispose _workspaceChangedDisposer.Dispose() End Sub - Private Function OnWorkspaceChangedAsync(e As WorkspaceChangeEventArgs) As Task + Private Sub OnWorkspaceChanged(e As WorkspaceChangeEventArgs) _changeEvents.Add(e) - - Return Task.CompletedTask - End Function + End Sub Friend Async Function GetNewChangeEventsAsync() As Task(Of IEnumerable(Of WorkspaceChangeEventArgs)) Await _asynchronousOperationWaiter.ExpeditedWaitAsync() diff --git a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs index 04c2192699045..15052eca30cba 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -8,7 +8,6 @@ using System.ComponentModel.Composition; using System.Linq; using System.Threading; -using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Editor; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; @@ -37,7 +36,7 @@ internal sealed partial class XamlProjectService : IDisposable private readonly IThreadingContext _threadingContext; private readonly Dictionary _xamlProjects = []; private readonly ConcurrentDictionary _documentIds = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - private IDisposable _documentClosedHandlerDisposer; + private readonly IDisposable _documentClosedHandlerDisposer; private RunningDocumentTable? _rdt; private IVsSolution? _vsSolution; @@ -60,7 +59,7 @@ public XamlProjectService( AnalyzerService = analyzerService; - _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosedAsync); + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosed); } public void Dispose() @@ -193,13 +192,11 @@ public void Dispose() return null; } - private Task OnDocumentClosedAsync(DocumentEventArgs e) + private void OnDocumentClosed(DocumentEventArgs e) { var filePath = e.Document.FilePath; if (filePath == null) - { - return Task.CompletedTask; - } + return; if (_documentIds.TryGetValue(filePath, out var documentId)) { @@ -212,8 +209,6 @@ private Task OnDocumentClosedAsync(DocumentEventArgs e) _documentIds.TryRemove(filePath, out _); } - - return Task.CompletedTask; } private void OnProjectClosing(IVsHierarchy hierarchy) diff --git a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs index 852fe2323f28e..c1d3396225720 100644 --- a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs +++ b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Threading.Tasks; namespace Microsoft.CodeAnalysis.ExternalAccess.UnitTesting.Api; @@ -22,15 +21,11 @@ private sealed class EventHandlerWrapper : IDisposable internal EventHandlerWrapper(Workspace workspace, Action action, bool opened) { _textDocumentOperationDisposer = opened - ? workspace.RegisterTextDocumentOpenedHandler(HandleAsync) - : workspace.RegisterTextDocumentClosedHandler(HandleAsync); + ? workspace.RegisterTextDocumentOpenedHandler(HandleEvent) + : workspace.RegisterTextDocumentClosedHandler(HandleEvent); - Task HandleAsync(TextDocumentEventArgs args) - { - action(new UnitTestingTextDocumentEventArgsWrapper(args)); - - return Task.CompletedTask; - } + void HandleEvent(TextDocumentEventArgs args) + => action(new UnitTestingTextDocumentEventArgsWrapper(args)); } public void Dispose() diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index fd26353e6ef91..d4a12a92b2b15 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -23,6 +23,7 @@ using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Threading; using Roslyn.Utilities; +using static Roslyn.Utilities.EventMap; namespace Microsoft.CodeAnalysis; @@ -39,8 +40,7 @@ public abstract partial class Workspace : IDisposable private readonly IAsynchronousOperationListener _asyncOperationListener; - private readonly AsyncBatchingWorkQueue _workQueue; - private readonly AsyncBatchingWorkQueue> _backgroundAsyncWorkQueue; + private readonly AsyncBatchingWorkQueue<(EventArgs, EventHandlerSet)> _eventHandlerWorkQueue; private readonly CancellationTokenSource _workQueueTokenSource = new(); private readonly ITaskSchedulerProvider _taskSchedulerProvider; @@ -81,15 +81,9 @@ protected Workspace(HostServices host, string? workspaceKind) var listenerProvider = Services.GetRequiredService(); _asyncOperationListener = listenerProvider.GetListener(); - _workQueue = new( + _eventHandlerWorkQueue = new( TimeSpan.Zero, - ProcessWorkQueueAsync, - _asyncOperationListener, - _workQueueTokenSource.Token); - - _backgroundAsyncWorkQueue = new( - TimeSpan.Zero, - ProcessBackgroundWorkQueueAsync, + ProcessWorkQueueNewAsync, _asyncOperationListener, _workQueueTokenSource.Token); @@ -599,15 +593,17 @@ internal void UpdateCurrentSolutionOnOptionsChanged() #pragma warning disable IDE0060 // Remove unused parameter protected internal Task ScheduleTask(Action action, string? taskName = "Workspace.Task") { - _workQueue.AddWork(action); - return _workQueue.WaitUntilCurrentBatchCompletesAsync(); + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => action(), WorkspaceEventOptions.MainThreadDependent); + var handlerSet = new EventHandlerSet(handlerAndOptions); + + return ScheduleTask(EventArgs.Empty, handlerSet); } [SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "This is a Task wrapper, not an asynchronous method.")] - internal Task ScheduleBackgroundTask(Func action) + internal Task ScheduleTask(EventArgs args, EventHandlerSet handlerSet) { - _backgroundAsyncWorkQueue.AddWork(action); - return _backgroundAsyncWorkQueue.WaitUntilCurrentBatchCompletesAsync(); + _eventHandlerWorkQueue.AddWork((args, handlerSet)); + return _eventHandlerWorkQueue.WaitUntilCurrentBatchCompletesAsync(); } /// @@ -617,8 +613,11 @@ internal Task ScheduleBackgroundTask(Func action) protected internal async Task ScheduleTask(Func func, string? taskName = "Workspace.Task") { T? result = default; - _workQueue.AddWork(() => result = func()); - await _workQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(false); + + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => result = func(), WorkspaceEventOptions.MainThreadDependent); + var handlerSet = new EventHandlerSet(handlerAndOptions); + + await ScheduleTask(EventArgs.Empty, handlerSet).ConfigureAwait(false); return result!; } #pragma warning restore IDE0060 // Remove unused parameter @@ -730,41 +729,34 @@ protected virtual void Dispose(bool finalize) _workQueueTokenSource.Cancel(); } - private async ValueTask ProcessWorkQueueAsync(ImmutableSegmentedList list, CancellationToken cancellationToken) + private async ValueTask ProcessWorkQueueNewAsync(ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, CancellationToken cancellationToken) { - // Hop over to the right scheduler to execute all this work. + ProcessWorkQueueHelper(list, shouldRaise: static options => !options.RequiresMainThread, cancellationToken); + await Task.Factory.StartNew(() => { - foreach (var item in list) + ProcessWorkQueueHelper(list, shouldRaise: static options => options.RequiresMainThread, cancellationToken); + }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); + + void ProcessWorkQueueHelper( + ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, + Func shouldRaise, + CancellationToken cancellationToken) + { + + foreach (var (args, handlerSet) in list) { cancellationToken.ThrowIfCancellationRequested(); try { - item(); + handlerSet.RaiseEvent(args, shouldRaise); } catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e)) { // Ensure we continue onto further items, even if one particular item fails. } } - }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); - } - - private static async ValueTask ProcessBackgroundWorkQueueAsync(ImmutableSegmentedList> list, CancellationToken cancellationToken) - { - foreach (var item in list) - { - cancellationToken.ThrowIfCancellationRequested(); - - try - { - await item().ConfigureAwait(false); - } - catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e)) - { - // Ensure we continue onto further items, even if one particular item fails. - } } } diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs index 0add07cf38edf..132e9e5f8243d 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -3,30 +3,17 @@ // See the LICENSE file in the project root for more information. using System; -using System.Threading.Tasks; using Roslyn.Utilities; +using static Roslyn.Utilities.EventMap; namespace Microsoft.CodeAnalysis; -internal abstract class WorkspaceEventRegistration : IDisposable +internal sealed class WorkspaceEventRegistration(EventMap eventMap, string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) : IDisposable { - public static WorkspaceEventRegistration Create(AsyncEventMap asyncEventMap, string eventName, Func handler) - where TEventArgs : EventArgs - { - return new WorkspaceEventRegistrationImpl(asyncEventMap, eventName, handler); - } + private readonly EventMap _eventMap = eventMap; + private readonly string _eventName = eventName; + private readonly WorkspaceEventHandlerAndOptions _handlerAndOptions = handlerAndOptions; - public abstract void Dispose(); - - private sealed class WorkspaceEventRegistrationImpl(AsyncEventMap asyncEventMap, string eventName, Func handler) - : WorkspaceEventRegistration - where TEventArgs : EventArgs - { - private readonly AsyncEventMap _asyncEventMap = asyncEventMap; - private readonly string _eventName = eventName; - private readonly Func _handler = handler; - - public override void Dispose() - => _asyncEventMap.RemoveAsyncEventHandler(_eventName, _handler); - } + public void Dispose() + => _eventMap.RemoveEventHandler(_eventName, _handlerAndOptions); } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index 75427f078cc47..ff26970cea3d5 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -5,13 +5,11 @@ #nullable disable using System; -using System.Collections.Generic; -using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; +using static Roslyn.Utilities.EventMap; namespace Microsoft.CodeAnalysis; @@ -30,177 +28,105 @@ public abstract partial class Workspace private IWorkspaceEventListenerService _workspaceEventListenerService; + #region Event Registration + /// - /// An event raised whenever the current solution is changed. + /// Registers a handler that is fired whenever the current solution is changed. /// - public event EventHandler WorkspaceChanged - { - add - { - _eventMap.AddEventHandler(WorkspaceChangeEventName, value); - } + internal WorkspaceEventRegistration RegisterWorkspaceChangedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(WorkspaceChangeEventName, handler, options); - remove - { - _eventMap.RemoveEventHandler(WorkspaceChangeEventName, value); - } - } + /// + /// Registers a handler that is fired whenever the current solution is changed. + /// + internal WorkspaceEventRegistration RegisterWorkspaceChangedImmediateHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(WorkspaceChangedImmediateEventName, handler, options); + + /// + /// Registers a handler that is fired when a is opened in the editor. + /// + internal WorkspaceEventRegistration RegisterDocumentOpenedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(DocumentOpenedEventName, handler, options); /// - /// An event raised *immediately* whenever the current solution is changed. Handlers - /// should be written to be very fast. Called on the same thread changing the workspace, - /// which may vary depending on the workspace. + /// Registers a handler that is fired when a is closed in the editor. /// - internal event EventHandler WorkspaceChangedImmediate + internal WorkspaceEventRegistration RegisterDocumentClosedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(DocumentClosedEventName, handler, options); + + /// + /// Registers a handler that is fired when any is opened in the editor. + /// + internal WorkspaceEventRegistration RegisterTextDocumentOpenedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(TextDocumentOpenedEventName, handler, options); + + /// + /// Registers a handler that is fired when any is closed in the editor. + /// + internal WorkspaceEventRegistration RegisterTextDocumentClosedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(TextDocumentClosedEventName, handler, options); + + /// + /// Registers a handler that is fired when the active context document associated with a buffer + /// changes. + /// + internal WorkspaceEventRegistration RegisterDocumentActiveContextChangedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(DocumentActiveContextChangedName, handler, options); + + private WorkspaceEventRegistration RegisterHandler(string eventName, Action handler, WorkspaceEventOptions? options = null) + where TEventArgs : EventArgs { - add - { - _eventMap.AddEventHandler(WorkspaceChangedImmediateEventName, value); - } + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => handler((TEventArgs)args), options ?? WorkspaceEventOptions.Default); + _eventMap.AddEventHandler(eventName, handlerAndOptions); - remove - { - _eventMap.RemoveEventHandler(WorkspaceChangedImmediateEventName, value); - } + return new WorkspaceEventRegistration(_eventMap, eventName, handlerAndOptions); } + #endregion + protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solution oldSolution, Solution newSolution, ProjectId projectId = null, DocumentId documentId = null) { if (newSolution == null) - { throw new ArgumentNullException(nameof(newSolution)); - } if (oldSolution == newSolution) - { return Task.CompletedTask; - } if (projectId == null && documentId != null) - { projectId = documentId.ProjectId; - } WorkspaceChangeEventArgs args = null; - var ev = GetEventHandlers(WorkspaceChangedImmediateEventName); - if (ev.HasHandlers) + var immediateHandlerSet = GetEventHandlers(WorkspaceChangedImmediateEventName); + if (immediateHandlerSet.HasHandlers) { args = new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); - RaiseEventForHandlers(ev, sender: this, args, FunctionId.Workspace_EventsImmediate); + immediateHandlerSet.RaiseEvent(args, shouldRaiseEvent: static option => true); } - var eventHandlerTasks = new List(); - - ev = GetEventHandlers(WorkspaceChangeEventName); - if (ev.HasHandlers) + var handlerSet = GetEventHandlers(WorkspaceChangeEventName); + if (handlerSet.HasHandlers) { args ??= new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); - var syncEventHandlersTask = this.ScheduleTask(() => - { - RaiseEventForHandlers(ev, sender: this, args, FunctionId.Workspace_Events); - }, WorkspaceChangeEventName); - - eventHandlerTasks.Add(syncEventHandlersTask); - } - - var asyncEv = _asyncEventMap.GetEventHandlerSet(WorkspaceChangeEventName); - if (asyncEv.HasHandlers) - { - args ??= new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); - var asyncEventHandlersTask = this.ScheduleBackgroundTask(() => RaiseEventForAsyncHandlersAsync(asyncEv, args, FunctionId.Workspace_Events)); - - eventHandlerTasks.Add(asyncEventHandlersTask); - } - - return Task.WhenAll(eventHandlerTasks); - - static void RaiseEventForHandlers( - EventMap.EventHandlerSet> handlers, - Workspace sender, - WorkspaceChangeEventArgs args, - FunctionId functionId) - { - using (Logger.LogBlock(functionId, (s, p, d, k) => $"{s.Id} - {p} - {d} {args.Kind.ToString()}", args.NewSolution, args.ProjectId, args.DocumentId, args.Kind, CancellationToken.None)) - { - handlers.RaiseEvent(static (handler, arg) => handler(arg.sender, arg.args), (sender, args)); - } - } - - static async Task RaiseEventForAsyncHandlersAsync( - AsyncEventMap.AsyncEventHandlerSet asyncHandlers, - WorkspaceChangeEventArgs args, - FunctionId functionId) - { - using (Logger.LogBlock(functionId, (s, p, d, k) => $"{s.Id} - {p} - {d} {args.Kind.ToString()}", args.NewSolution, args.ProjectId, args.DocumentId, args.Kind, CancellationToken.None)) - { - await asyncHandlers.RaiseEventAsync(args).ConfigureAwait(false); - } + return this.ScheduleTask(args, handlerSet); } - } - /// - /// An event raised whenever the workspace or part of its solution model - /// fails to access a file or other external resource. - /// - public event EventHandler WorkspaceFailed - { - add - { - _eventMap.AddEventHandler(WorkspaceFailedEventName, value); - } - - remove - { - _eventMap.RemoveEventHandler(WorkspaceFailedEventName, value); - } + return Task.CompletedTask; } protected internal virtual void OnWorkspaceFailed(WorkspaceDiagnostic diagnostic) { - var ev = GetEventHandlers(WorkspaceFailedEventName); - if (ev.HasHandlers) + var handlerSet = GetEventHandlers(WorkspaceFailedEventName); + if (handlerSet.HasHandlers) { var args = new WorkspaceDiagnosticEventArgs(diagnostic); - ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); - } - } - - /// - /// An event that is fired when a is opened in the editor. - /// - public event EventHandler DocumentOpened - { - add - { - _eventMap.AddEventHandler(DocumentOpenedEventName, value); - } - - remove - { - _eventMap.RemoveEventHandler(DocumentOpenedEventName, value); + handlerSet.RaiseEvent(args, shouldRaiseEvent: static option => true); } } protected Task RaiseDocumentOpenedEventAsync(Document document) => RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), DocumentOpenedEventName); - /// - /// An event that is fired when any is opened in the editor. - /// - public event EventHandler TextDocumentOpened - { - add - { - _eventMap.AddEventHandler(TextDocumentOpenedEventName, value); - } - - remove - { - _eventMap.RemoveEventHandler(TextDocumentOpenedEventName, value); - } - } - protected Task RaiseTextDocumentOpenedEventAsync(TextDocument document) => RaiseTextDocumentOpenedOrClosedEventAsync(document, new TextDocumentEventArgs(document), TextDocumentOpenedEventName); @@ -211,85 +137,19 @@ private Task RaiseTextDocumentOpenedOrClosedEventAsync(); - var ev = GetEventHandlers(eventName); + var handlerSet = GetEventHandlers(eventName); + if (handlerSet.HasHandlers && document != null) + return this.ScheduleTask(args, handlerSet); - if (ev.HasHandlers && document != null) - { - var syncEventHandlerTask = this.ScheduleTask(() => - { - ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); - }, eventName); - - eventHandlerTasks.Add(syncEventHandlerTask); - } - - var asyncEv = _asyncEventMap.GetEventHandlerSet(eventName); - if (asyncEv.HasHandlers) - { - var asyncEventHandlersTask = this.ScheduleBackgroundTask(() => asyncEv.RaiseEventAsync(args)); - - eventHandlerTasks.Add(asyncEventHandlersTask); - } - - return Task.WhenAll(eventHandlerTasks); - } - - /// - /// An event that is fired when a is closed in the editor. - /// - public event EventHandler DocumentClosed - { - add - { - _eventMap.AddEventHandler(DocumentClosedEventName, value); - } - - remove - { - _eventMap.RemoveEventHandler(DocumentClosedEventName, value); - } + return Task.CompletedTask; } protected Task RaiseDocumentClosedEventAsync(Document document) => RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), DocumentClosedEventName); - /// - /// An event that is fired when any is closed in the editor. - /// - public event EventHandler TextDocumentClosed - { - add - { - _eventMap.AddEventHandler(TextDocumentClosedEventName, value); - } - - remove - { - _eventMap.RemoveEventHandler(TextDocumentClosedEventName, value); - } - } - protected Task RaiseTextDocumentClosedEventAsync(TextDocument document) => RaiseTextDocumentOpenedOrClosedEventAsync(document, new TextDocumentEventArgs(document), TextDocumentClosedEventName); - /// - /// An event that is fired when the active context document associated with a buffer - /// changes. - /// - public event EventHandler DocumentActiveContextChanged - { - add - { - _eventMap.AddEventHandler(DocumentActiveContextChangedName, value); - } - - remove - { - _eventMap.RemoveEventHandler(DocumentActiveContextChangedName, value); - } - } - [Obsolete("This member is obsolete. Use the RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer, DocumentId, DocumentId) overload instead.", error: true)] protected Task RaiseDocumentActiveContextChangedEventAsync(Document document) => throw new NotImplementedException(); @@ -299,41 +159,25 @@ protected Task RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer s if (sourceTextContainer == null || oldActiveContextDocumentId == null || newActiveContextDocumentId == null) return Task.CompletedTask; - var eventHandlerTasks = new List(); - var ev = GetEventHandlers(DocumentActiveContextChangedName); - DocumentActiveContextChangedEventArgs? args = null; - - if (ev.HasHandlers) + var handlerSet = GetEventHandlers(DocumentActiveContextChangedName); + if (handlerSet.HasHandlers) { // Capture the current solution snapshot (inside the _serializationLock of OnDocumentContextUpdated) var currentSolution = this.CurrentSolution; - args ??= new DocumentActiveContextChangedEventArgs(currentSolution, sourceTextContainer, oldActiveContextDocumentId, newActiveContextDocumentId); - - var syncEventHandlerTask = this.ScheduleTask(() => - { - ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); - }, "Workspace.WorkspaceChanged"); - - eventHandlerTasks.Add(syncEventHandlerTask); - } - - var asyncEv = _asyncEventMap.GetEventHandlerSet(DocumentActiveContextChangedName); - if (asyncEv.HasHandlers) - { - var asyncEventHandlersTask = this.ScheduleBackgroundTask(() => asyncEv.RaiseEventAsync(args)); + var args = new DocumentActiveContextChangedEventArgs(currentSolution, sourceTextContainer, oldActiveContextDocumentId, newActiveContextDocumentId); - eventHandlerTasks.Add(asyncEventHandlersTask); + return this.ScheduleTask(args, handlerSet); } - return Task.WhenAll(eventHandlerTasks); + return Task.CompletedTask; } - private EventMap.EventHandlerSet> GetEventHandlers(string eventName) where T : EventArgs + private EventMap.EventHandlerSet GetEventHandlers(string eventName) { // this will register features that want to listen to workspace events // lazily first time workspace event is actually fired EnsureEventListeners(); - return _eventMap.GetEventHandlers>(eventName); + return _eventMap.GetEventHandlerSet(eventName); } private void EnsureEventListeners() diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs deleted file mode 100644 index d88c0a705efca..0000000000000 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsAsync.cs +++ /dev/null @@ -1,59 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Threading.Tasks; -using Roslyn.Utilities; - -namespace Microsoft.CodeAnalysis; - -public abstract partial class Workspace -{ - private readonly AsyncEventMap _asyncEventMap = new(); - - /// - /// Registers a handler that is fired whenever the current solution is changed. - /// - internal WorkspaceEventRegistration RegisterWorkspaceChangedHandler(Func handler) - => RegisterAsyncHandler(WorkspaceChangeEventName, handler); - - /// - /// Registers a handler that is fired when a is opened in the editor. - /// - internal WorkspaceEventRegistration RegisterDocumentOpenedHandler(Func handler) - => RegisterAsyncHandler(DocumentOpenedEventName, handler); - - /// - /// Registers a handler that is fired when a is closed in the editor. - /// - internal WorkspaceEventRegistration RegisterDocumentClosedHandler(Func handler) - => RegisterAsyncHandler(DocumentClosedEventName, handler); - - /// - /// Registers a handler that is fired when any is opened in the editor. - /// - internal WorkspaceEventRegistration RegisterTextDocumentOpenedHandler(Func handler) - => RegisterAsyncHandler(TextDocumentOpenedEventName, handler); - - /// - /// Registers a handler that is fired when any is closed in the editor. - /// - internal WorkspaceEventRegistration RegisterTextDocumentClosedHandler(Func handler) - => RegisterAsyncHandler(TextDocumentClosedEventName, handler); - - /// - /// Registers a handler that is fired when the active context document associated with a buffer - /// changes. - /// - internal WorkspaceEventRegistration RegisterDocumentActiveContextChangedHandler(Func handler) - => RegisterAsyncHandler(DocumentActiveContextChangedName, handler); - - private WorkspaceEventRegistration RegisterAsyncHandler(string eventName, Func handler) - where TEventArgs : EventArgs - { - _asyncEventMap.AddAsyncEventHandler(eventName, handler); - - return WorkspaceEventRegistration.Create(_asyncEventMap, eventName, handler); - } -} diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs new file mode 100644 index 0000000000000..45a11eb5835b7 --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -0,0 +1,100 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; +using Roslyn.Utilities; +using static Roslyn.Utilities.EventMap; + +namespace Microsoft.CodeAnalysis; + +public abstract partial class Workspace +{ + // Allows conversion of legacy event handlers to the new event system. + private readonly ConcurrentDictionary<(object, string), IDisposable> _disposableEventHandlers = new(); + + /// + /// An event raised whenever the current solution is changed. + /// + public event EventHandler WorkspaceChanged + { + add => AddEventHandler(value, WorkspaceChangeEventName); + remove => RemoveEventHandler(value, WorkspaceChangeEventName); + } + + /// + /// An event raised whenever the workspace or part of its solution model + /// fails to access a file or other external resource. + /// + public event EventHandler WorkspaceFailed + { + add => AddEventHandler(value, WorkspaceFailedEventName); + remove => RemoveEventHandler(value, WorkspaceFailedEventName); + } + + /// + /// An event that is fired when a is opened in the editor. + /// + public event EventHandler DocumentOpened + { + add => AddEventHandler(value, DocumentOpenedEventName); + remove => RemoveEventHandler(value, DocumentOpenedEventName); + } + + /// + /// An event that is fired when any is opened in the editor. + /// + public event EventHandler TextDocumentOpened + { + add => AddEventHandler(value, TextDocumentOpenedEventName); + remove => RemoveEventHandler(value, TextDocumentOpenedEventName); + } + + /// + /// An event that is fired when a is closed in the editor. + /// + public event EventHandler DocumentClosed + { + add => AddEventHandler(value, DocumentClosedEventName); + remove => RemoveEventHandler(value, DocumentClosedEventName); + } + + /// + /// An event that is fired when any is closed in the editor. + /// + public event EventHandler TextDocumentClosed + { + add => AddEventHandler(value, TextDocumentClosedEventName); + remove => RemoveEventHandler(value, TextDocumentClosedEventName); + } + + /// + /// An event that is fired when the active context document associated with a buffer + /// changes. + /// + public event EventHandler DocumentActiveContextChanged + { + add => AddEventHandler(value, DocumentActiveContextChangedName); + remove => RemoveEventHandler(value, DocumentActiveContextChangedName); + } + + private void AddEventHandler(EventHandler eventHandler, string eventName) + where TEventArgs : EventArgs + { + Action handler = arg => eventHandler(sender: this, (TEventArgs)arg); + var disposer = RegisterHandler(eventName, handler, WorkspaceEventOptions.MainThreadDependent); + + _disposableEventHandlers[(eventHandler, eventName)] = disposer; + } + + private void RemoveEventHandler(EventHandler eventHandler, string eventName) + where TEventArgs : EventArgs + { + _disposableEventHandlers.TryRemove((eventHandler, eventName), out var disposer); + + disposer?.Dispose(); + } +} diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs index fe7c7ca57253c..0420dc42418a3 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis.Text; +using static Roslyn.Utilities.EventMap; namespace Microsoft.CodeAnalysis; @@ -42,7 +43,10 @@ protected void RegisterText(SourceTextContainer textContainer) var registration = GetWorkspaceRegistration(textContainer); registration.SetWorkspace(this); - this.ScheduleTask(registration.RaiseEvents, "Workspace.RegisterText"); + + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => registration.RaiseEvents(), WorkspaceEventOptions.MainThreadDependent); + var handlerSet = new EventHandlerSet(handlerAndOptions); + this.ScheduleTask(EventArgs.Empty, handlerSet); } /// diff --git a/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs b/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs index 522c6870cae00..d6a71b46fb465 100644 --- a/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs +++ b/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs @@ -427,7 +427,6 @@ public async Task TestChangeDocumentName_TryApplyChanges() { tcs.SetResult(true); } - return Task.CompletedTask; }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -461,8 +460,6 @@ public async Task TestChangeDocumentFolders_TryApplyChanges() { tcs.SetResult(true); } - - return Task.CompletedTask; }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -497,7 +494,6 @@ public async Task TestChangeDocumentFilePath_TryApplyChanges() { tcs.SetResult(true); } - return Task.CompletedTask; }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -529,7 +525,6 @@ public async Task TestChangeDocumentSourceCodeKind_TryApplyChanges() { tcs.SetResult(true); } - return Task.CompletedTask; }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); diff --git a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs index 94a2fdfa2d4da..6c05e9f91908b 100644 --- a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs +++ b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; using Microsoft.CodeAnalysis.Text; using Roslyn.Test.Utilities; @@ -55,7 +54,7 @@ public static DocumentId CreateDocumentId(this ProjectId projectId, string name, public static IEnumerable GetProjectsByName(this Solution solution, string name) => solution.Projects.Where(p => string.Compare(p.Name, name, StringComparison.OrdinalIgnoreCase) == 0); - internal static EventWaiter VerifyWorkspaceChangedEvent(this Workspace workspace, Func action) + internal static EventWaiter VerifyWorkspaceChangedEvent(this Workspace workspace, Action action) { var wew = new EventWaiter(); workspace.RegisterWorkspaceChangedHandler(wew.Wrap(action)); diff --git a/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs b/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs index b5195d60ba506..9128c40c08781 100644 --- a/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs +++ b/src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs @@ -2218,8 +2218,6 @@ public async Task TestWorkspaceChangedEvent() Assert.Equal(expectedEventKind, args.Kind); Assert.NotNull(args.NewSolution); Assert.NotSame(originalSolution, args.NewSolution); - - return Task.CompletedTask; }); // change document text (should fire SolutionChanged event) var doc = workspace.CurrentSolution.Projects.First().Documents.First(); @@ -2250,8 +2248,6 @@ public async Task TestWorkspaceChangedWeakEvent() Assert.Equal(expectedEventKind, args.Kind); Assert.NotNull(args.NewSolution); Assert.NotSame(originalSolution, args.NewSolution); - - return Task.CompletedTask; }); // change document text (should fire SolutionChanged event) var doc = workspace.CurrentSolution.Projects.First().Documents.First(); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index 6b09bd899d037..63e8ed07e559d 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -485,7 +485,6 @@ - @@ -559,6 +558,7 @@ + diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs deleted file mode 100644 index 19ad943944893..0000000000000 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncEventMap.cs +++ /dev/null @@ -1,74 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Threading; -using System.Threading.Tasks; - -namespace Roslyn.Utilities; - -internal sealed class AsyncEventMap -{ - private readonly SemaphoreSlim _guard = new(initialCount: 1); - private readonly Dictionary _eventNameToHandlerSet = []; - - public void AddAsyncEventHandler(string eventName, Func asyncHandler) - where TEventArgs : EventArgs - { - using (_guard.DisposableWait()) - { - _eventNameToHandlerSet[eventName] = _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) - ? ((AsyncEventHandlerSet)handlers).AddHandler(asyncHandler) - : new AsyncEventHandlerSet([asyncHandler]); - } - } - - public void RemoveAsyncEventHandler(string eventName, Func asyncHandler) - where TEventArgs : EventArgs - { - using (_guard.DisposableWait()) - { - if (_eventNameToHandlerSet.TryGetValue(eventName, out var handlers)) - _eventNameToHandlerSet[eventName] = ((AsyncEventHandlerSet)handlers).RemoveHandler(asyncHandler); - } - } - - public AsyncEventHandlerSet GetEventHandlerSet(string eventName) - where TEventArgs : EventArgs - { - using (_guard.DisposableWait()) - { - return _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) - ? (AsyncEventHandlerSet)handlers - : AsyncEventHandlerSet.Empty; - } - } - - public sealed class AsyncEventHandlerSet(ImmutableArray> asyncHandlers) - where TEventArgs : EventArgs - { - private readonly ImmutableArray> _asyncHandlers = asyncHandlers; - public static readonly AsyncEventHandlerSet Empty = new([]); - - public AsyncEventHandlerSet AddHandler(Func asyncHandler) - => new(_asyncHandlers.Add(asyncHandler)); - - public AsyncEventHandlerSet RemoveHandler(Func asyncHandler) - { - var newAsyncHandlers = _asyncHandlers.RemoveAll(r => r.Equals(asyncHandler)); - - return newAsyncHandlers.IsEmpty ? Empty : new(newAsyncHandlers); - } - - public bool HasHandlers => !_asyncHandlers.IsEmpty; - - public async Task RaiseEventAsync(TEventArgs arg) - { - foreach (var asyncHandler in _asyncHandlers) - await asyncHandler(arg).ConfigureAwait(false); - } - } -} diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs index c7e66cb79fe3e..8047d74c1402d 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs @@ -6,171 +6,109 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.ErrorReporting; namespace Roslyn.Utilities; internal sealed class EventMap { - private readonly NonReentrantLock _guard = new(); + private readonly SemaphoreSlim _guard = new(initialCount: 1); + private readonly Dictionary _eventNameToHandlerSet = []; - private readonly Dictionary _eventNameToRegistries = []; - - public EventMap() - { - } - - public void AddEventHandler(string eventName, TEventHandler eventHandler) - where TEventHandler : class + public void AddEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) { using (_guard.DisposableWait()) { - var registries = GetRegistries_NoLock(eventName); - var newRegistries = registries.Add(new Registry(eventHandler)); - SetRegistries_NoLock(eventName, newRegistries); + _eventNameToHandlerSet[eventName] = _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) + ? handlers.AddHandler(handlerAndOptions) + : EventHandlerSet.Empty.AddHandler(handlerAndOptions); } } - public void RemoveEventHandler(string eventName, TEventHandler eventHandler) - where TEventHandler : class + public void RemoveEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) { using (_guard.DisposableWait()) { - var registries = GetRegistries_NoLock(eventName); - - // remove disabled registrations from list - var newRegistries = registries.RemoveAll(r => r.HasHandler(eventHandler)); - - if (newRegistries != registries) - { - // disable all registrations of this handler (so pending raise events can be squelched) - // This does not guarantee no race condition between Raise and Remove but greatly reduces it. - foreach (var registry in registries.Where(r => r.HasHandler(eventHandler))) - { - registry.Unregister(); - } - - SetRegistries_NoLock(eventName, newRegistries); - } + if (_eventNameToHandlerSet.TryGetValue(eventName, out var handlers)) + _eventNameToHandlerSet[eventName] = handlers.RemoveHandler(handlerAndOptions); } } - [PerformanceSensitive( - "https://developercommunity.visualstudio.com/content/problem/854696/changing-target-framework-takes-10-minutes-with-10.html", - AllowImplicitBoxing = false)] - public EventHandlerSet GetEventHandlers(string eventName) - where TEventHandler : class - { - return new EventHandlerSet(this.GetRegistries(eventName)); - } - - private ImmutableArray> GetRegistries(string eventName) - where TEventHandler : class + public EventHandlerSet GetEventHandlerSet(string eventName) { using (_guard.DisposableWait()) { - return GetRegistries_NoLock(eventName); + return _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) + ? handlers + : EventHandlerSet.Empty; } } - private ImmutableArray> GetRegistries_NoLock(string eventName) - where TEventHandler : class + public record struct WorkspaceEventHandlerAndOptions(Action Handler, WorkspaceEventOptions Options) { - _guard.AssertHasLock(); - if (_eventNameToRegistries.TryGetValue(eventName, out var registries)) - { - return (ImmutableArray>)registries; - } - - return []; } - private void SetRegistries_NoLock(string eventName, ImmutableArray> registries) - where TEventHandler : class + public sealed class EventHandlerSet { - _guard.AssertHasLock(); + public static readonly EventHandlerSet Empty = new([]); + private readonly ImmutableArray _registries; - _eventNameToRegistries[eventName] = registries; - } - - internal sealed class Registry(TEventHandler handler) : IEquatable?> - where TEventHandler : class - { - private TEventHandler? _handler = handler; - - public void Unregister() - => _handler = null; + public EventHandlerSet(WorkspaceEventHandlerAndOptions handlerAndOptions) + : this([new Registry(handlerAndOptions)]) + { + } - public void Invoke(Action invoker, TArg arg) + public EventHandlerSet(ImmutableArray registries) { - var handler = _handler; - if (handler != null) - { - invoker(handler, arg); - } + _registries = registries; } - public bool HasHandler(TEventHandler handler) - => handler.Equals(_handler); + public EventHandlerSet AddHandler(WorkspaceEventHandlerAndOptions handlerAndOptions) + => new EventHandlerSet(_registries.Add(new Registry(handlerAndOptions))); - public bool Equals(Registry? other) + public EventHandlerSet RemoveHandler(WorkspaceEventHandlerAndOptions handlerAndOptions) { - if (other == null) - { - return false; - } + var newRegistries = _registries.RemoveAll(r => r.HasHandlerAndOptions(handlerAndOptions)); - if (other._handler == null && _handler == null) + if (newRegistries != _registries) { - return true; - } + // disable all registrations of this handler (so pending raise events can be squelched) + // This does not guarantee no race condition between Raise and Remove but greatly reduces it. + foreach (var registry in _registries.Where(r => r.HasHandlerAndOptions(handlerAndOptions))) + registry.Unregister(); - if (other._handler == null || _handler == null) - { - return false; + return newRegistries.IsEmpty ? Empty : new(newRegistries); } - return other._handler.Equals(_handler); + return this; } - public override bool Equals(object? obj) - => Equals(obj as Registry); + public bool HasHandlers => _registries.Length > 0; - public override int GetHashCode() - => _handler == null ? 0 : _handler.GetHashCode(); + public void RaiseEvent(TEventArgs arg, Func shouldRaiseEvent) + where TEventArgs : EventArgs + { + foreach (var registry in _registries) + registry.RaiseEvent(arg, shouldRaiseEvent); + } } - internal struct EventHandlerSet - where TEventHandler : class + public sealed class Registry(WorkspaceEventHandlerAndOptions handlerAndOptions) { - private readonly ImmutableArray> _registries; + private WorkspaceEventHandlerAndOptions _handlerAndOptions = handlerAndOptions; + private bool _disableHandler = false; - internal EventHandlerSet(ImmutableArray> registries) - => _registries = registries; + public void Unregister() + => _disableHandler = true; - public readonly bool HasHandlers - { - get { return _registries != null && _registries.Length > 0; } - } + public bool HasHandlerAndOptions(WorkspaceEventHandlerAndOptions handlerAndOptions) + => _handlerAndOptions.Equals(handlerAndOptions); - public readonly void RaiseEvent(Action invoker, TArg arg) + public void RaiseEvent(EventArgs args, Func shouldRaiseEvent) { - if (this.HasHandlers) - { - foreach (var registry in _registries) - { - try - { - registry.Invoke(invoker, arg); - } - catch (Exception e) when (FatalError.ReportAndCatch(e)) - { - // Catch the exception and continue as this an event handler and propagating the exception would prevent - // other handlers from executing - } - } - } + if (shouldRaiseEvent(_handlerAndOptions.Options) && !_disableHandler) + _handlerAndOptions.Handler(args); } } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WorkspaceEventOptions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WorkspaceEventOptions.cs new file mode 100644 index 0000000000000..07b7950232f7f --- /dev/null +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WorkspaceEventOptions.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.CodeAnalysis; + +internal record struct WorkspaceEventOptions(bool RequiresMainThread) +{ + public static readonly WorkspaceEventOptions Default = new(RequiresMainThread: false); + public static readonly WorkspaceEventOptions MainThreadDependent = new(RequiresMainThread: true); +} From cdceae8221c81f0f04f73872517b36e32430543a Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 15:55:35 -0700 Subject: [PATCH 04/17] Implement missed workspace event (WorkspaceFailed) --- src/Features/Lsif/Generator/Program.cs | 2 +- src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs | 7 +++++++ .../Core/Portable/Workspace/Workspace_EventsLegacy.cs | 4 ---- src/Workspaces/MSBuild/Test/MSBuildWorkspaceTestBase.cs | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Features/Lsif/Generator/Program.cs b/src/Features/Lsif/Generator/Program.cs index 66bba2a3b918b..948bd61f80bb2 100644 --- a/src/Features/Lsif/Generator/Program.cs +++ b/src/Features/Lsif/Generator/Program.cs @@ -182,7 +182,7 @@ private static async Task GenerateWithMSBuildWorkspaceAsync( var solutionLoadStopwatch = Stopwatch.StartNew(); using var msbuildWorkspace = MSBuildWorkspace.Create(await Composition.CreateHostServicesAsync()); - msbuildWorkspace.WorkspaceFailed += (s, e) => logger.Log(e.Diagnostic.Kind == WorkspaceDiagnosticKind.Failure ? LogLevel.Error : LogLevel.Warning, "Problem while loading: " + e.Diagnostic.Message); + _ = msbuildWorkspace.RegisterWorkspaceFailedHandler((e) => logger.Log(e.Diagnostic.Kind == WorkspaceDiagnosticKind.Failure ? LogLevel.Error : LogLevel.Warning, "Problem while loading: " + e.Diagnostic.Message)); var solution = await openAsync(msbuildWorkspace, cancellationToken); diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index ff26970cea3d5..f27ae5800e9b3 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -42,6 +42,13 @@ internal WorkspaceEventRegistration RegisterWorkspaceChangedHandler(Action handler, WorkspaceEventOptions? options = null) => RegisterHandler(WorkspaceChangedImmediateEventName, handler, options); + /// + /// Registers a handler that is fired whenever the workspace or part of its solution model + /// fails to access a file or other external resource. + /// + internal WorkspaceEventRegistration RegisterWorkspaceFailedHandler(Action handler, WorkspaceEventOptions? options = null) + => RegisterHandler(WorkspaceFailedEventName, handler, options); + /// /// Registers a handler that is fired when a is opened in the editor. /// diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index 45a11eb5835b7..0d88a710390ff 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -4,10 +4,6 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Threading; -using Roslyn.Utilities; -using static Roslyn.Utilities.EventMap; namespace Microsoft.CodeAnalysis; diff --git a/src/Workspaces/MSBuild/Test/MSBuildWorkspaceTestBase.cs b/src/Workspaces/MSBuild/Test/MSBuildWorkspaceTestBase.cs index 97bb16c6fb3de..a94c9a22054d6 100644 --- a/src/Workspaces/MSBuild/Test/MSBuildWorkspaceTestBase.cs +++ b/src/Workspaces/MSBuild/Test/MSBuildWorkspaceTestBase.cs @@ -164,7 +164,7 @@ protected MSBuildWorkspace CreateMSBuildWorkspace( if (throwOnWorkspaceFailed) { - workspace.WorkspaceFailed += (s, e) => throw new Exception($"Workspace failure {e.Diagnostic.Kind}:{e.Diagnostic.Message}"); + _ = workspace.RegisterWorkspaceFailedHandler((e) => throw new Exception($"Workspace failure {e.Diagnostic.Kind}:{e.Diagnostic.Message}")); } if (skipUnrecognizedProjects) From b0a49f8ead8013c7994f965c357df714818c020e Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 16:16:23 -0700 Subject: [PATCH 05/17] mostly formatting cleanup --- .../WorkspaceTests_EditorFeatures.cs | 61 ++++++------------- ...lassificationTaggerProvider.TagComputer.cs | 13 ++-- .../ActiveStatementTrackingService.cs | 4 +- ...xtViewWindowVerifierInProcessExtensions.cs | 5 +- .../Core/Portable/Workspace/Workspace.cs | 4 +- .../CoreTestUtilities/WorkspaceExtensions.cs | 2 +- 6 files changed, 26 insertions(+), 63 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs index 2ecfeca3594eb..6c3434d5b8171 100644 --- a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs +++ b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs @@ -62,10 +62,7 @@ public async Task TestEmptySolutionUpdateDoesNotFireEvents() var solution = workspace.CurrentSolution; var workspaceChanged = false; - using var _ = workspace.RegisterWorkspaceChangedHandler(e => - { - workspaceChanged = true; - }); + using var _ = workspace.RegisterWorkspaceChangedHandler(e => workspaceChanged = true); // make an 'empty' update by claiming something changed, but its the same as before workspace.OnParseOptionsChanged(project.Id, project.ParseOptions); @@ -818,18 +815,12 @@ public async Task TestDocumentEvents() using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. var documentOpenedEventHandler = openWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - "The document given to the 'DocumentOpened' event handler did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + "The document given to the 'DocumentOpened' event handler did not have the same id as the one created for the test.")); var documentClosedEventHandler = closeWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - "The document given to the 'DocumentClosed' event handler did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + "The document given to the 'DocumentClosed' event handler did not have the same id as the one created for the test.")); var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandler); var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandler); @@ -890,18 +881,12 @@ public async Task TestSourceGeneratedDocumentEvents() // Wrapping event handlers so they can notify us on being called. var documentOpenedEventHandler = openWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - $"The source generated document given to the '{nameof(Workspace.RegisterDocumentOpenedHandler)}' did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + $"The source generated document given to the '{nameof(Workspace.RegisterDocumentOpenedHandler)}' did not have the same id as the one created for the test.")); var documentClosedEventHandler = closeWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - $"The source generated document given to the '{nameof(Workspace.RegisterDocumentClosedHandler)}' did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + $"The source generated document given to the '{nameof(Workspace.RegisterDocumentClosedHandler)}' did not have the same id as the one created for the test.")); var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandler); var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandler); @@ -960,18 +945,12 @@ public async Task TestAdditionalDocumentEvents() using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. var textDocumentOpenedEventHandler = openWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - "The document given to the 'RegisterTextDocumentOpenedHandler' event handler did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + "The document given to the 'RegisterTextDocumentOpenedHandler' event handler did not have the same id as the one created for the test.")); var textDocumentClosedEventHandler = closeWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - "The document given to the 'RegisterTextDocumentClosedHandler' event handler did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + "The document given to the 'RegisterTextDocumentClosedHandler' event handler did not have the same id as the one created for the test.")); var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandler); var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandler); @@ -1027,18 +1006,12 @@ public async Task TestAnalyzerConfigDocumentEvents() using var openWaiter = new EventWaiter(); // Wrapping event handlers so they can notify us on being called. var textDocumentOpenedEventHandler = openWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - "The document given to the 'AnalyzerConfigDocumentOpened' event handler did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + "The document given to the 'AnalyzerConfigDocumentOpened' event handler did not have the same id as the one created for the test.")); var textDocumentClosedEventHandler = closeWaiter.Wrap( - args => - { - Assert.True(args.Document.Id == document.Id, - "The document given to the 'AnalyzerConfigDocumentClosed' event handler did not have the same id as the one created for the test."); - }); + args => Assert.True(args.Document.Id == document.Id, + "The document given to the 'AnalyzerConfigDocumentClosed' event handler did not have the same id as the one created for the test.")); var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandler); var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandler); diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 734e6a78b8e7f..3fcf0606e48a7 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -46,8 +46,8 @@ private sealed record CachedServices( private readonly SyntacticClassificationTaggerProvider _taggerProvider; private readonly ITextBuffer2 _subjectBuffer; private readonly WorkspaceRegistration _workspaceRegistration; - private IDisposable? _workspaceChangedDisposer; - private IDisposable? _workspaceDocumentActiveContextChangedDisposer; + private IDisposable _workspaceChangedDisposer; + private IDisposable _workspaceDocumentActiveContextChangedDisposer; private readonly CancellationTokenSource _disposalCancellationSource = new(); @@ -245,11 +245,8 @@ public void DisconnectFromWorkspace() if (_workspace != null) { - _workspaceChangedDisposer?.Dispose(); - _workspaceChangedDisposer = null; - - _workspaceDocumentActiveContextChangedDisposer?.Dispose(); - _workspaceDocumentActiveContextChangedDisposer = null; + _workspaceChangedDisposer.Dispose(); + _workspaceDocumentActiveContextChangedDisposer.Dispose(); _workspace = null; @@ -280,7 +277,6 @@ private void OnDocumentActiveContextChanged(DocumentActiveContextChangedEventArg return; _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); - return; } private void OnWorkspaceChanged(WorkspaceChangeEventArgs args) @@ -307,7 +303,6 @@ private void OnWorkspaceChanged(WorkspaceChangeEventArgs args) return; _workQueue.AddWork(_subjectBuffer.CurrentSnapshot); - return; } #endregion diff --git a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs index dc8667661f612..9efaeb3d2b079 100644 --- a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs +++ b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs @@ -156,9 +156,7 @@ private void DocumentClosed(DocumentEventArgs e) } private void DocumentOpened(DocumentEventArgs e) - { - _ = TrackActiveSpansAsync(e.Document); - } + => _ = TrackActiveSpansAsync(e.Document); private async Task TrackActiveSpansAsync(Document designTimeDocument) { diff --git a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs index fbaae7e23fd8a..229cee06bb58a 100644 --- a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs +++ b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs @@ -64,10 +64,7 @@ public static async Task CodeActionAsync( var events = new List(); var workspace = await textViewWindowVerifier.TestServices.Shell.GetComponentModelServiceAsync(cancellationToken); - using var _ = workspace.RegisterWorkspaceChangedHandler(e => - { - events.Add(e); - }); + using var _ = workspace.RegisterWorkspaceChangedHandler(e => events.Add(e)); await textViewWindowVerifier.TestServices.Editor.ShowLightBulbAsync(cancellationToken); diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index d4a12a92b2b15..cd7298eb0e18f 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -83,7 +83,7 @@ protected Workspace(HostServices host, string? workspaceKind) _asyncOperationListener = listenerProvider.GetListener(); _eventHandlerWorkQueue = new( TimeSpan.Zero, - ProcessWorkQueueNewAsync, + ProcessEventHandlerWorkQueueAsync, _asyncOperationListener, _workQueueTokenSource.Token); @@ -729,7 +729,7 @@ protected virtual void Dispose(bool finalize) _workQueueTokenSource.Cancel(); } - private async ValueTask ProcessWorkQueueNewAsync(ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, CancellationToken cancellationToken) + private async ValueTask ProcessEventHandlerWorkQueueAsync(ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, CancellationToken cancellationToken) { ProcessWorkQueueHelper(list, shouldRaise: static options => !options.RequiresMainThread, cancellationToken); diff --git a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs index 6c05e9f91908b..4fb64efe2d2a6 100644 --- a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs +++ b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs @@ -57,7 +57,7 @@ public static IEnumerable GetProjectsByName(this Solution solution, str internal static EventWaiter VerifyWorkspaceChangedEvent(this Workspace workspace, Action action) { var wew = new EventWaiter(); - workspace.RegisterWorkspaceChangedHandler(wew.Wrap(action)); + _ = workspace.RegisterWorkspaceChangedHandler(wew.Wrap(action)); return wew; } } From b1f1b5206abf37317d220f2cf27704f9978524c9 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 16:32:05 -0700 Subject: [PATCH 06/17] Move a couple event handlers back to the main thread that appear to need it --- .../SyntacticClassificationTaggerProvider.TagComputer.cs | 5 +++-- ...erEventSources.DocumentActiveContextChangedEventSource.cs | 2 +- .../Xaml/Impl/Implementation/XamlProjectService.cs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 3fcf0606e48a7..730ef3ecaad36 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -284,13 +284,14 @@ private void OnWorkspaceChanged(WorkspaceChangeEventArgs args) // We may be getting an event for a workspace we already disconnected from. If so, // ignore them. We won't be able to find the Document corresponding to our text buffer, // so we can't reasonably classify this anyways. - if (args.NewSolution.Workspace != _workspace) + var workspace = _workspace; + if (args.NewSolution.Workspace != workspace) return; if (args.Kind != WorkspaceChangeKind.ProjectChanged) return; - var documentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); + var documentId = workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); if (args.ProjectId != documentId?.ProjectId) return; diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs index 89d7511ff297d..a33b1ef41ee0e 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -15,7 +15,7 @@ private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subject private IDisposable? _documentActiveContextChangedDisposer; protected override void ConnectToWorkspace(Workspace workspace) - => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged); + => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.MainThreadDependent); protected override void DisconnectFromWorkspace(Workspace workspace) => _documentActiveContextChangedDisposer?.Dispose(); diff --git a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs index 15052eca30cba..a097fa7e5cbfe 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -59,7 +59,7 @@ public XamlProjectService( AnalyzerService = analyzerService; - _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosed); + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosed, WorkspaceEventOptions.MainThreadDependent); } public void Dispose() From c48694a6dad7bfcb3523c7764001dcdc02624c67 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 16:54:07 -0700 Subject: [PATCH 07/17] A couple more small cleanups --- ...lassificationTaggerProvider.TagComputer.cs | 8 ++--- .../MiscellaneousFilesWorkspace.cs | 2 +- .../Core/Portable/Workspace/Workspace.cs | 4 +-- .../Workspace/Workspace_Registration.cs | 2 +- .../Compiler/Core/Utilities/EventMap.cs | 31 +++++++------------ 5 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 730ef3ecaad36..bae79c65dd9f5 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -46,8 +46,8 @@ private sealed record CachedServices( private readonly SyntacticClassificationTaggerProvider _taggerProvider; private readonly ITextBuffer2 _subjectBuffer; private readonly WorkspaceRegistration _workspaceRegistration; - private IDisposable _workspaceChangedDisposer; - private IDisposable _workspaceDocumentActiveContextChangedDisposer; + private IDisposable? _workspaceChangedDisposer; + private IDisposable? _workspaceDocumentActiveContextChangedDisposer; private readonly CancellationTokenSource _disposalCancellationSource = new(); @@ -245,8 +245,8 @@ public void DisconnectFromWorkspace() if (_workspace != null) { - _workspaceChangedDisposer.Dispose(); - _workspaceDocumentActiveContextChangedDisposer.Dispose(); + _workspaceChangedDisposer?.Dispose(); + _workspaceDocumentActiveContextChangedDisposer?.Dispose(); _workspace = null; diff --git a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs index 30c4b6cdbadc1..4e2c126abe16a 100644 --- a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs +++ b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs @@ -174,7 +174,7 @@ private void Registration_WorkspaceChanged(object sender, EventArgs e) if (!_threadingContext.JoinableTaskContext.IsOnMainThread) { var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => Registration_WorkspaceChanged(sender, e), WorkspaceEventOptions.MainThreadDependent); - var handlerSet = new EventHandlerSet(handlerAndOptions); + var handlerSet = EventHandlerSet.Create(handlerAndOptions); ScheduleTask(e, handlerSet); return; diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index cd7298eb0e18f..79eb357adc404 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -594,7 +594,7 @@ internal void UpdateCurrentSolutionOnOptionsChanged() protected internal Task ScheduleTask(Action action, string? taskName = "Workspace.Task") { var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => action(), WorkspaceEventOptions.MainThreadDependent); - var handlerSet = new EventHandlerSet(handlerAndOptions); + var handlerSet = EventHandlerSet.Create(handlerAndOptions); return ScheduleTask(EventArgs.Empty, handlerSet); } @@ -615,7 +615,7 @@ protected internal async Task ScheduleTask(Func func, string? taskName T? result = default; var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => result = func(), WorkspaceEventOptions.MainThreadDependent); - var handlerSet = new EventHandlerSet(handlerAndOptions); + var handlerSet = EventHandlerSet.Create(handlerAndOptions); await ScheduleTask(EventArgs.Empty, handlerSet).ConfigureAwait(false); return result!; diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs index 0420dc42418a3..af9fea9fd3f7b 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs @@ -45,7 +45,7 @@ protected void RegisterText(SourceTextContainer textContainer) registration.SetWorkspace(this); var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => registration.RaiseEvents(), WorkspaceEventOptions.MainThreadDependent); - var handlerSet = new EventHandlerSet(handlerAndOptions); + var handlerSet = EventHandlerSet.Create(handlerAndOptions); this.ScheduleTask(EventArgs.Empty, handlerSet); } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs index 8047d74c1402d..40c07cd753461 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs @@ -49,20 +49,13 @@ public record struct WorkspaceEventHandlerAndOptions(Action Handler, { } - public sealed class EventHandlerSet + public sealed class EventHandlerSet(ImmutableArray registries) { public static readonly EventHandlerSet Empty = new([]); - private readonly ImmutableArray _registries; + private readonly ImmutableArray _registries = registries; - public EventHandlerSet(WorkspaceEventHandlerAndOptions handlerAndOptions) - : this([new Registry(handlerAndOptions)]) - { - } - - public EventHandlerSet(ImmutableArray registries) - { - _registries = registries; - } + public static EventHandlerSet Create(WorkspaceEventHandlerAndOptions handlerAndOptions) + => new EventHandlerSet([new Registry(handlerAndOptions)]); public EventHandlerSet AddHandler(WorkspaceEventHandlerAndOptions handlerAndOptions) => new EventHandlerSet(_registries.Add(new Registry(handlerAndOptions))); @@ -71,17 +64,15 @@ public EventHandlerSet RemoveHandler(WorkspaceEventHandlerAndOptions handlerAndO { var newRegistries = _registries.RemoveAll(r => r.HasHandlerAndOptions(handlerAndOptions)); - if (newRegistries != _registries) - { - // disable all registrations of this handler (so pending raise events can be squelched) - // This does not guarantee no race condition between Raise and Remove but greatly reduces it. - foreach (var registry in _registries.Where(r => r.HasHandlerAndOptions(handlerAndOptions))) - registry.Unregister(); + if (newRegistries == _registries) + return this; - return newRegistries.IsEmpty ? Empty : new(newRegistries); - } + // disable all registrations of this handler (so pending raise events can be squelched) + // This does not guarantee no race condition between Raise and Remove but greatly reduces it. + foreach (var registry in _registries.Where(r => r.HasHandlerAndOptions(handlerAndOptions))) + registry.Unregister(); - return this; + return newRegistries.IsEmpty ? Empty : new(newRegistries); } public bool HasHandlers => _registries.Length > 0; From be3fd62b24f84fb8031d6f3b7e91fccd5e6b5030 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 17:14:34 -0700 Subject: [PATCH 08/17] Move EventMap/WorkspaceEventOptions out of SharedUtilitiesAndExtensions --- .../Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs | 2 +- .../Utilities => Core/Portable/Workspace}/EventMap.cs | 7 +++++-- src/Workspaces/Core/Portable/Workspace/Workspace.cs | 2 +- .../Portable/Workspace}/WorkspaceEventOptions.cs | 0 .../Core/Portable/Workspace/WorkspaceEventRegistration.cs | 3 +-- .../Core/Portable/Workspace/Workspace_Events.cs | 8 +++----- .../Core/Portable/Workspace/Workspace_Registration.cs | 2 +- .../Compiler/Core/CompilerExtensions.projitems | 2 -- 8 files changed, 12 insertions(+), 14 deletions(-) rename src/Workspaces/{SharedUtilitiesAndExtensions/Compiler/Core/Utilities => Core/Portable/Workspace}/EventMap.cs (93%) rename src/Workspaces/{SharedUtilitiesAndExtensions/Compiler/Core/Utilities => Core/Portable/Workspace}/WorkspaceEventOptions.cs (100%) diff --git a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs index 4e2c126abe16a..8a830196af7ec 100644 --- a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs +++ b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs @@ -22,7 +22,7 @@ using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.TextManager.Interop; using Roslyn.Utilities; -using static Roslyn.Utilities.EventMap; +using static Microsoft.CodeAnalysis.EventMap; namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs b/src/Workspaces/Core/Portable/Workspace/EventMap.cs similarity index 93% rename from src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs rename to src/Workspaces/Core/Portable/Workspace/EventMap.cs index 40c07cd753461..41d297d768b92 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs +++ b/src/Workspaces/Core/Portable/Workspace/EventMap.cs @@ -8,15 +8,16 @@ using System.Linq; using System.Threading; using Microsoft.CodeAnalysis; +using Roslyn.Utilities; -namespace Roslyn.Utilities; +namespace Microsoft.CodeAnalysis; internal sealed class EventMap { private readonly SemaphoreSlim _guard = new(initialCount: 1); private readonly Dictionary _eventNameToHandlerSet = []; - public void AddEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) + public WorkspaceEventRegistration AddEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) { using (_guard.DisposableWait()) { @@ -24,6 +25,8 @@ public void AddEventHandler(string eventName, WorkspaceEventHandlerAndOptions ha ? handlers.AddHandler(handlerAndOptions) : EventHandlerSet.Empty.AddHandler(handlerAndOptions); } + + return new WorkspaceEventRegistration(this, eventName, handlerAndOptions); } public void RemoveEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index 79eb357adc404..de2fc5185f735 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -23,7 +23,7 @@ using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Threading; using Roslyn.Utilities; -using static Roslyn.Utilities.EventMap; +using static Microsoft.CodeAnalysis.EventMap; namespace Microsoft.CodeAnalysis; diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WorkspaceEventOptions.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventOptions.cs similarity index 100% rename from src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WorkspaceEventOptions.cs rename to src/Workspaces/Core/Portable/Workspace/WorkspaceEventOptions.cs diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs index 132e9e5f8243d..8520cfd6e80ac 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -3,8 +3,7 @@ // See the LICENSE file in the project root for more information. using System; -using Roslyn.Utilities; -using static Roslyn.Utilities.EventMap; +using static Microsoft.CodeAnalysis.EventMap; namespace Microsoft.CodeAnalysis; diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index f27ae5800e9b3..69620f306b843 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -8,8 +8,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; -using static Roslyn.Utilities.EventMap; +using static Microsoft.CodeAnalysis.EventMap; namespace Microsoft.CodeAnalysis; @@ -84,9 +83,8 @@ private WorkspaceEventRegistration RegisterHandler(string eventName, where TEventArgs : EventArgs { var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => handler((TEventArgs)args), options ?? WorkspaceEventOptions.Default); - _eventMap.AddEventHandler(eventName, handlerAndOptions); - return new WorkspaceEventRegistration(_eventMap, eventName, handlerAndOptions); + return _eventMap.AddEventHandler(eventName, handlerAndOptions); } #endregion @@ -179,7 +177,7 @@ protected Task RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer s return Task.CompletedTask; } - private EventMap.EventHandlerSet GetEventHandlers(string eventName) + private EventHandlerSet GetEventHandlers(string eventName) { // this will register features that want to listen to workspace events // lazily first time workspace event is actually fired diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs index af9fea9fd3f7b..88b470f09fa3a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs @@ -6,7 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis.Text; -using static Roslyn.Utilities.EventMap; +using static Microsoft.CodeAnalysis.EventMap; namespace Microsoft.CodeAnalysis; diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index 63e8ed07e559d..02bc5cc1163f5 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -507,7 +507,6 @@ - @@ -558,7 +557,6 @@ - From 2254c0525034c984e402b1a11e41723e26e96883 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 14 Apr 2025 17:49:52 -0700 Subject: [PATCH 09/17] use WorkspaceEventRegistration type directly instead of IDisposable --- .../SyntacticClassificationTaggerProvider.TagComputer.cs | 4 ++-- .../Core/EditAndContinue/ActiveStatementTrackingService.cs | 4 ++-- src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs | 4 ++-- ...erEventSources.DocumentActiveContextChangedEventSource.cs | 3 +-- .../AbstractCreateServicesOnTextViewConnection.cs | 2 +- .../ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb | 5 +---- .../Xaml/Impl/Implementation/XamlProjectService.cs | 2 +- .../UnitTesting/Api/UnitTestingWorkspaceExtensions.cs | 2 +- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index bae79c65dd9f5..14d023eb67d75 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -46,8 +46,8 @@ private sealed record CachedServices( private readonly SyntacticClassificationTaggerProvider _taggerProvider; private readonly ITextBuffer2 _subjectBuffer; private readonly WorkspaceRegistration _workspaceRegistration; - private IDisposable? _workspaceChangedDisposer; - private IDisposable? _workspaceDocumentActiveContextChangedDisposer; + private WorkspaceEventRegistration? _workspaceChangedDisposer; + private WorkspaceEventRegistration? _workspaceDocumentActiveContextChangedDisposer; private readonly CancellationTokenSource _disposalCancellationSource = new(); diff --git a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs index 9efaeb3d2b079..b4c2cbb29f6af 100644 --- a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs +++ b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTrackingService.cs @@ -103,8 +103,8 @@ internal sealed class TrackingSession private readonly CancellationTokenSource _cancellationSource = new(); private readonly IActiveStatementSpanFactory _spanProvider; private readonly ICompileTimeSolutionProvider _compileTimeSolutionProvider; - private readonly IDisposable _documentOpenedHandlerDisposer; - private readonly IDisposable _documentClosedHandlerDisposer; + private readonly WorkspaceEventRegistration _documentOpenedHandlerDisposer; + private readonly WorkspaceEventRegistration _documentClosedHandlerDisposer; #region lock(_trackingSpans) diff --git a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs index f7e8b9e3e2269..72507158d155b 100644 --- a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs +++ b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs @@ -44,8 +44,8 @@ internal sealed class SolutionChecksumUpdater private readonly object _gate = new(); private bool _isSynchronizeWorkspacePaused; - private readonly IDisposable _workspaceChangedDisposer; - private readonly IDisposable _workspaceChangedImmediateDisposer; + private readonly WorkspaceEventRegistration _workspaceChangedDisposer; + private readonly WorkspaceEventRegistration _workspaceChangedImmediateDisposer; private readonly CancellationToken _shutdownToken; diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs index a33b1ef41ee0e..091ad5fec3c20 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; @@ -12,7 +11,7 @@ internal partial class TaggerEventSources { private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subjectBuffer) : AbstractWorkspaceTrackingTaggerEventSource(subjectBuffer) { - private IDisposable? _documentActiveContextChangedDisposer; + private WorkspaceEventRegistration? _documentActiveContextChangedDisposer; protected override void ConnectToWorkspace(Workspace workspace) => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.MainThreadDependent); diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs index 088390c32f684..e4c0c8f6a55f4 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs @@ -31,7 +31,7 @@ internal abstract class AbstractCreateServicesOnTextViewConnection : IWpfTextVie private readonly string _languageName; private readonly AsyncBatchingWorkQueue _workQueue; private bool _initialized = false; - private readonly IDisposable _workspaceDocumentOpenedDisposer; + private readonly WorkspaceEventRegistration _workspaceDocumentOpenedDisposer; protected VisualStudioWorkspace Workspace { get; } protected IGlobalOptionService GlobalOptions { get; } diff --git a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb index 3d6a2096f9c74..431cb97717986 100644 --- a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb +++ b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb @@ -9,14 +9,11 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr Friend Class WorkspaceChangeWatcher Implements IDisposable - Private ReadOnly _environment As TestEnvironment Private ReadOnly _asynchronousOperationWaiter As IAsynchronousOperationWaiter Private _changeEvents As New List(Of WorkspaceChangeEventArgs) - Private ReadOnly _workspaceChangedDisposer As IDisposable + Private ReadOnly _workspaceChangedDisposer As WorkspaceEventRegistration Public Sub New(environment As TestEnvironment) - _environment = environment - Dim listenerProvider = environment.ExportProvider.GetExportedValue(Of AsynchronousOperationListenerProvider)() _asynchronousOperationWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace) diff --git a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs index a097fa7e5cbfe..b5aa2c04a94ff 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -36,7 +36,7 @@ internal sealed partial class XamlProjectService : IDisposable private readonly IThreadingContext _threadingContext; private readonly Dictionary _xamlProjects = []; private readonly ConcurrentDictionary _documentIds = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - private readonly IDisposable _documentClosedHandlerDisposer; + private readonly WorkspaceEventRegistration _documentClosedHandlerDisposer; private RunningDocumentTable? _rdt; private IVsSolution? _vsSolution; diff --git a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs index c1d3396225720..85bf53f439eba 100644 --- a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs +++ b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs @@ -16,7 +16,7 @@ public static IDisposable RegisterTextDocumentClosedEventHandler(this Workspace private sealed class EventHandlerWrapper : IDisposable { - private readonly IDisposable _textDocumentOperationDisposer; + private readonly WorkspaceEventRegistration _textDocumentOperationDisposer; internal EventHandlerWrapper(Workspace workspace, Action action, bool opened) { From eaec791124c748c602afb82241cc8fbc5bfb8b9e Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 15 Apr 2025 08:54:29 -0700 Subject: [PATCH 10/17] PR feedback --- ...DocumentActiveContextChangedEventSource.cs | 3 +- ...tractCreateServicesOnTextViewConnection.cs | 2 +- .../MiscellaneousFilesWorkspace.cs | 5 ++- .../Impl/Implementation/XamlProjectService.cs | 3 +- .../Core/Portable/Workspace/Workspace.cs | 27 ++++++++---- .../{EventMap.cs => WorkspaceEventMap.cs} | 42 ++++++++++++++----- .../Workspace/WorkspaceEventOptions.cs | 4 +- .../Workspace/WorkspaceEventRegistration.cs | 16 +++++-- .../Portable/Workspace/Workspace_Events.cs | 6 +-- .../Workspace/Workspace_EventsLegacy.cs | 5 ++- .../Workspace/Workspace_Registration.cs | 6 ++- 11 files changed, 83 insertions(+), 36 deletions(-) rename src/Workspaces/Core/Portable/Workspace/{EventMap.cs => WorkspaceEventMap.cs} (67%) diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs index 091ad5fec3c20..ee02ce7ef6cf7 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -13,8 +13,9 @@ private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subject { private WorkspaceEventRegistration? _documentActiveContextChangedDisposer; + // Require main thread on the callback as RaiseChanged implementors may have main thread dependencies. protected override void ConnectToWorkspace(Workspace workspace) - => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.MainThreadDependent); + => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.RequiresMainThreadOptions); protected override void DisconnectFromWorkspace(Workspace workspace) => _documentActiveContextChangedDisposer?.Dispose(); diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs index e4c0c8f6a55f4..d608cda53e5a9 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs @@ -30,8 +30,8 @@ internal abstract class AbstractCreateServicesOnTextViewConnection : IWpfTextVie { private readonly string _languageName; private readonly AsyncBatchingWorkQueue _workQueue; - private bool _initialized = false; private readonly WorkspaceEventRegistration _workspaceDocumentOpenedDisposer; + private bool _initialized = false; protected VisualStudioWorkspace Workspace { get; } protected IGlobalOptionService GlobalOptions { get; } diff --git a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs index 8a830196af7ec..7e82dc6a8a2a2 100644 --- a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs +++ b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs @@ -22,7 +22,7 @@ using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.TextManager.Interop; using Roslyn.Utilities; -using static Microsoft.CodeAnalysis.EventMap; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; @@ -173,7 +173,8 @@ private void Registration_WorkspaceChanged(object sender, EventArgs e) // to the RDT in the background thread. Since this is all asynchronous a bit more asynchrony is fine. if (!_threadingContext.JoinableTaskContext.IsOnMainThread) { - var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => Registration_WorkspaceChanged(sender, e), WorkspaceEventOptions.MainThreadDependent); + // Require main thread on the callback as this method requires that per the comment above. + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => Registration_WorkspaceChanged(sender, e), WorkspaceEventOptions.RequiresMainThreadOptions); var handlerSet = EventHandlerSet.Create(handlerAndOptions); ScheduleTask(e, handlerSet); diff --git a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs index b5aa2c04a94ff..a8f3afa25c7c7 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -59,7 +59,8 @@ public XamlProjectService( AnalyzerService = analyzerService; - _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosed, WorkspaceEventOptions.MainThreadDependent); + // Require main thread on the callback as OnDocumentClosed is not thread safe. + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosed, WorkspaceEventOptions.RequiresMainThreadOptions); } public void Dispose() diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index de2fc5185f735..a4452439abdb7 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -23,7 +23,7 @@ using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Threading; using Roslyn.Utilities; -using static Microsoft.CodeAnalysis.EventMap; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; @@ -593,7 +593,8 @@ internal void UpdateCurrentSolutionOnOptionsChanged() #pragma warning disable IDE0060 // Remove unused parameter protected internal Task ScheduleTask(Action action, string? taskName = "Workspace.Task") { - var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => action(), WorkspaceEventOptions.MainThreadDependent); + // Require main thread on the callback as this is publicly exposed and the given actions may have main thread dependencies. + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => action(), WorkspaceEventOptions.RequiresMainThreadOptions); var handlerSet = EventHandlerSet.Create(handlerAndOptions); return ScheduleTask(EventArgs.Empty, handlerSet); @@ -614,7 +615,8 @@ protected internal async Task ScheduleTask(Func func, string? taskName { T? result = default; - var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => result = func(), WorkspaceEventOptions.MainThreadDependent); + // Require main thread on the callback as this is publicly exposed and the given actions may have main thread dependencies. + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => result = func(), WorkspaceEventOptions.RequiresMainThreadOptions); var handlerSet = EventHandlerSet.Create(handlerAndOptions); await ScheduleTask(EventArgs.Empty, handlerSet).ConfigureAwait(false); @@ -729,21 +731,30 @@ protected virtual void Dispose(bool finalize) _workQueueTokenSource.Cancel(); } - private async ValueTask ProcessEventHandlerWorkQueueAsync(ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, CancellationToken cancellationToken) + private async ValueTask ProcessEventHandlerWorkQueueAsync(ImmutableSegmentedList<(EventArgs Args, EventHandlerSet HandlerSet)> list, CancellationToken cancellationToken) { + // Currently, this method maintains ordering of events to their callbacks. If that were no longer necessary, + // further performance optimizations of this method could be considered, such as grouping together + // callbacks by eventargs/registries or parallelizing callbacks. + + // ABWQ callbacks occur on threadpool threads, so we can process the handlers immediately that don't require the main thread. ProcessWorkQueueHelper(list, shouldRaise: static options => !options.RequiresMainThread, cancellationToken); - await Task.Factory.StartNew(() => + // Verify there are handlers which require the main thread before performing the thread switch. + if (list.Any(static x => x.HandlerSet.HasMatchingOptions(isMatch: static options => options.RequiresMainThread))) { - ProcessWorkQueueHelper(list, shouldRaise: static options => options.RequiresMainThread, cancellationToken); - }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); + await Task.Factory.StartNew(() => + { + // As the scheduler has moved us to the main thread, we can now process the handlers that require the main thread. + ProcessWorkQueueHelper(list, shouldRaise: static options => options.RequiresMainThread, cancellationToken); + }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); + } void ProcessWorkQueueHelper( ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, Func shouldRaise, CancellationToken cancellationToken) { - foreach (var (args, handlerSet) in list) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/Workspaces/Core/Portable/Workspace/EventMap.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs similarity index 67% rename from src/Workspaces/Core/Portable/Workspace/EventMap.cs rename to src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs index 41d297d768b92..95cb159147e0f 100644 --- a/src/Workspaces/Core/Portable/Workspace/EventMap.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Linq; using System.Threading; using Microsoft.CodeAnalysis; @@ -12,7 +13,7 @@ namespace Microsoft.CodeAnalysis; -internal sealed class EventMap +internal sealed class WorkspaceEventMap { private readonly SemaphoreSlim _guard = new(initialCount: 1); private readonly Dictionary _eventNameToHandlerSet = []; @@ -33,8 +34,19 @@ public void RemoveEventHandler(string eventName, WorkspaceEventHandlerAndOptions { using (_guard.DisposableWait()) { - if (_eventNameToHandlerSet.TryGetValue(eventName, out var handlers)) - _eventNameToHandlerSet[eventName] = handlers.RemoveHandler(handlerAndOptions); + _eventNameToHandlerSet.TryGetValue(eventName, out var originalHandlers); + + // An earlier AddEventHandler call would have created the WorkspaceEventRegistration whose + // disposal would have called this method. + Debug.Assert(originalHandlers != null); + + if (originalHandlers != null) + { + var newHandlers = originalHandlers.RemoveHandler(handlerAndOptions); + Debug.Assert(originalHandlers != newHandlers); + + _eventNameToHandlerSet[eventName] = newHandlers; + } } } @@ -48,7 +60,7 @@ public EventHandlerSet GetEventHandlerSet(string eventName) } } - public record struct WorkspaceEventHandlerAndOptions(Action Handler, WorkspaceEventOptions Options) + public readonly record struct WorkspaceEventHandlerAndOptions(Action Handler, WorkspaceEventOptions Options) { } @@ -65,20 +77,25 @@ public EventHandlerSet AddHandler(WorkspaceEventHandlerAndOptions handlerAndOpti public EventHandlerSet RemoveHandler(WorkspaceEventHandlerAndOptions handlerAndOptions) { - var newRegistries = _registries.RemoveAll(r => r.HasHandlerAndOptions(handlerAndOptions)); + var registry = _registries.FirstOrDefault(static (r, handlerAndOptions) => r.HasHandlerAndOptions(handlerAndOptions), handlerAndOptions); - if (newRegistries == _registries) + if (registry == null) return this; - // disable all registrations of this handler (so pending raise events can be squelched) + // disable this handler (so pending raise events can be squelched) // This does not guarantee no race condition between Raise and Remove but greatly reduces it. - foreach (var registry in _registries.Where(r => r.HasHandlerAndOptions(handlerAndOptions))) - registry.Unregister(); + registry.Unregister(); + + var newRegistries = _registries.Remove(registry); return newRegistries.IsEmpty ? Empty : new(newRegistries); } - public bool HasHandlers => _registries.Length > 0; + public bool HasHandlers + => _registries.Any(static r => true); + + public bool HasMatchingOptions(Func isMatch) + => _registries.Any(static (r, hasOptions) => r.HasMatchingOptions(hasOptions), isMatch); public void RaiseEvent(TEventArgs arg, Func shouldRaiseEvent) where TEventArgs : EventArgs @@ -99,9 +116,12 @@ public void Unregister() public bool HasHandlerAndOptions(WorkspaceEventHandlerAndOptions handlerAndOptions) => _handlerAndOptions.Equals(handlerAndOptions); + public bool HasMatchingOptions(Func isMatch) + => !_disableHandler && isMatch(_handlerAndOptions.Options); + public void RaiseEvent(EventArgs args, Func shouldRaiseEvent) { - if (shouldRaiseEvent(_handlerAndOptions.Options) && !_disableHandler) + if (!_disableHandler && shouldRaiseEvent(_handlerAndOptions.Options)) _handlerAndOptions.Handler(args); } } diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventOptions.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventOptions.cs index 07b7950232f7f..522e52fc13a47 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventOptions.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventOptions.cs @@ -6,6 +6,6 @@ namespace Microsoft.CodeAnalysis; internal record struct WorkspaceEventOptions(bool RequiresMainThread) { - public static readonly WorkspaceEventOptions Default = new(RequiresMainThread: false); - public static readonly WorkspaceEventOptions MainThreadDependent = new(RequiresMainThread: true); + public static readonly WorkspaceEventOptions DefaultOptions = new(RequiresMainThread: false); + public static readonly WorkspaceEventOptions RequiresMainThreadOptions = new(RequiresMainThread: true); } diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs index 8520cfd6e80ac..e45ea409d3718 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -3,16 +3,24 @@ // See the LICENSE file in the project root for more information. using System; -using static Microsoft.CodeAnalysis.EventMap; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; -internal sealed class WorkspaceEventRegistration(EventMap eventMap, string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) : IDisposable +/// +/// Represents a registration for a workspace event, ensuring the event handler is properly unregistered when disposed. +/// +/// This class is used to manage the lifecycle of an event handler associated with a workspace event. +/// When the instance is disposed, the event handler is automatically unregistered from the event map. +internal sealed class WorkspaceEventRegistration(WorkspaceEventMap eventMap, string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) : IDisposable { - private readonly EventMap _eventMap = eventMap; private readonly string _eventName = eventName; private readonly WorkspaceEventHandlerAndOptions _handlerAndOptions = handlerAndOptions; + private WorkspaceEventMap? _eventMap = eventMap; public void Dispose() - => _eventMap.RemoveEventHandler(_eventName, _handlerAndOptions); + { + _eventMap?.RemoveEventHandler(_eventName, _handlerAndOptions); + _eventMap = null; + } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index 69620f306b843..1a9924f30f84a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -8,13 +8,13 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Text; -using static Microsoft.CodeAnalysis.EventMap; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; public abstract partial class Workspace { - private readonly EventMap _eventMap = new(); + private readonly WorkspaceEventMap _eventMap = new(); private const string WorkspaceChangeEventName = "WorkspaceChanged"; private const string WorkspaceChangedImmediateEventName = "WorkspaceChangedImmediate"; @@ -82,7 +82,7 @@ internal WorkspaceEventRegistration RegisterDocumentActiveContextChangedHandler( private WorkspaceEventRegistration RegisterHandler(string eventName, Action handler, WorkspaceEventOptions? options = null) where TEventArgs : EventArgs { - var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => handler((TEventArgs)args), options ?? WorkspaceEventOptions.Default); + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => handler((TEventArgs)args), options ?? WorkspaceEventOptions.DefaultOptions); return _eventMap.AddEventHandler(eventName, handlerAndOptions); } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index 0d88a710390ff..d7cde62ae62ae 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -81,7 +81,10 @@ private void AddEventHandler(EventHandler eventHandler, where TEventArgs : EventArgs { Action handler = arg => eventHandler(sender: this, (TEventArgs)arg); - var disposer = RegisterHandler(eventName, handler, WorkspaceEventOptions.MainThreadDependent); + + // Require main thread on the callback as this is used from publicly exposed eventss + // and those callbacks may have main thread dependencies. + var disposer = RegisterHandler(eventName, handler, WorkspaceEventOptions.RequiresMainThreadOptions); _disposableEventHandlers[(eventHandler, eventName)] = disposer; } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs index 88b470f09fa3a..e78e2db1d3bf7 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs @@ -6,7 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis.Text; -using static Microsoft.CodeAnalysis.EventMap; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; @@ -44,7 +44,9 @@ protected void RegisterText(SourceTextContainer textContainer) var registration = GetWorkspaceRegistration(textContainer); registration.SetWorkspace(this); - var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => registration.RaiseEvents(), WorkspaceEventOptions.MainThreadDependent); + // Require main thread on the callback as WorkspaceRegistration.RaiseEvents invokes Workspace.WorkspaecChanges which is publicly exposed + // and the event handlers may have main thread dependencies. + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => registration.RaiseEvents(), WorkspaceEventOptions.RequiresMainThreadOptions); var handlerSet = EventHandlerSet.Create(handlerAndOptions); this.ScheduleTask(EventArgs.Empty, handlerSet); } From 7ad41dfcab8d9c47f3eb02081f0f8ae867ce3d7c Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 15 Apr 2025 09:28:54 -0700 Subject: [PATCH 11/17] Create enum WorkspaceEventType for strings representing workspace event types --- .../Core/Portable/Workspace/Workspace.cs | 2 +- .../Portable/Workspace/WorkspaceEventMap.cs | 21 +++--- .../Workspace/WorkspaceEventRegistration.cs | 7 +- .../Portable/Workspace/Workspace_Editor.cs | 4 +- .../Portable/Workspace/Workspace_Events.cs | 71 ++++++++++--------- .../Workspace/Workspace_EventsLegacy.cs | 45 ++++++------ 6 files changed, 77 insertions(+), 73 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index a4452439abdb7..253a9c2d23e5b 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -750,7 +750,7 @@ await Task.Factory.StartNew(() => }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); } - void ProcessWorkQueueHelper( + static void ProcessWorkQueueHelper( ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, Func shouldRaise, CancellationToken cancellationToken) diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs index 95cb159147e0f..37417ddfc6474 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs @@ -10,31 +10,32 @@ using System.Threading; using Microsoft.CodeAnalysis; using Roslyn.Utilities; +using static Microsoft.CodeAnalysis.Workspace; namespace Microsoft.CodeAnalysis; internal sealed class WorkspaceEventMap { private readonly SemaphoreSlim _guard = new(initialCount: 1); - private readonly Dictionary _eventNameToHandlerSet = []; + private readonly Dictionary _eventTypeToHandlerSet = []; - public WorkspaceEventRegistration AddEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) + public WorkspaceEventRegistration AddEventHandler(WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) { using (_guard.DisposableWait()) { - _eventNameToHandlerSet[eventName] = _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) + _eventTypeToHandlerSet[eventType] = _eventTypeToHandlerSet.TryGetValue(eventType, out var handlers) ? handlers.AddHandler(handlerAndOptions) : EventHandlerSet.Empty.AddHandler(handlerAndOptions); } - return new WorkspaceEventRegistration(this, eventName, handlerAndOptions); + return new WorkspaceEventRegistration(this, eventType, handlerAndOptions); } - public void RemoveEventHandler(string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) + public void RemoveEventHandler(WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) { using (_guard.DisposableWait()) { - _eventNameToHandlerSet.TryGetValue(eventName, out var originalHandlers); + _eventTypeToHandlerSet.TryGetValue(eventType, out var originalHandlers); // An earlier AddEventHandler call would have created the WorkspaceEventRegistration whose // disposal would have called this method. @@ -45,16 +46,16 @@ public void RemoveEventHandler(string eventName, WorkspaceEventHandlerAndOptions var newHandlers = originalHandlers.RemoveHandler(handlerAndOptions); Debug.Assert(originalHandlers != newHandlers); - _eventNameToHandlerSet[eventName] = newHandlers; + _eventTypeToHandlerSet[eventType] = newHandlers; } } } - public EventHandlerSet GetEventHandlerSet(string eventName) + public EventHandlerSet GetEventHandlerSet(WorkspaceEventType eventType) { using (_guard.DisposableWait()) { - return _eventNameToHandlerSet.TryGetValue(eventName, out var handlers) + return _eventTypeToHandlerSet.TryGetValue(eventType, out var handlers) ? handlers : EventHandlerSet.Empty; } @@ -107,7 +108,7 @@ public void RaiseEvent(TEventArgs arg, Func /// This class is used to manage the lifecycle of an event handler associated with a workspace event. /// When the instance is disposed, the event handler is automatically unregistered from the event map. -internal sealed class WorkspaceEventRegistration(WorkspaceEventMap eventMap, string eventName, WorkspaceEventHandlerAndOptions handlerAndOptions) : IDisposable +internal sealed class WorkspaceEventRegistration(WorkspaceEventMap eventMap, WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) : IDisposable { - private readonly string _eventName = eventName; + private readonly WorkspaceEventType _eventType = eventType; private readonly WorkspaceEventHandlerAndOptions _handlerAndOptions = handlerAndOptions; private WorkspaceEventMap? _eventMap = eventMap; public void Dispose() { - _eventMap?.RemoveEventHandler(_eventName, _handlerAndOptions); + _eventMap?.RemoveEventHandler(_eventType, _handlerAndOptions); _eventMap = null; } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs index 24bf354de7229..471b32c7bbfa9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs @@ -457,7 +457,7 @@ internal void OnSourceGeneratedDocumentOpened( // We raise 2 events for source document opened. var token = _asyncOperationListener.BeginAsyncOperation(nameof(OnSourceGeneratedDocumentOpened)); _ = RaiseDocumentOpenedEventAsync(document).CompletesAsyncOperation(token); - token = _asyncOperationListener.BeginAsyncOperation(TextDocumentOpenedEventName); + token = _asyncOperationListener.BeginAsyncOperation(nameof(WorkspaceEventType.TextDocumentOpened)); _ = RaiseTextDocumentOpenedEventAsync(document).CompletesAsyncOperation(token); } @@ -477,7 +477,7 @@ internal void OnSourceGeneratedDocumentClosed(SourceGeneratedDocument document) // We raise 2 events for source document closed. var token = _asyncOperationListener.BeginAsyncOperation(nameof(OnSourceGeneratedDocumentClosed)); _ = RaiseDocumentClosedEventAsync(document).CompletesAsyncOperation(token); - token = _asyncOperationListener.BeginAsyncOperation(TextDocumentClosedEventName); + token = _asyncOperationListener.BeginAsyncOperation(nameof(WorkspaceEventType.TextDocumentClosed)); _ = RaiseTextDocumentClosedEventAsync(document).CompletesAsyncOperation(token); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index 1a9924f30f84a..8adfc2542f811 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; @@ -16,16 +14,19 @@ public abstract partial class Workspace { private readonly WorkspaceEventMap _eventMap = new(); - private const string WorkspaceChangeEventName = "WorkspaceChanged"; - private const string WorkspaceChangedImmediateEventName = "WorkspaceChangedImmediate"; - private const string WorkspaceFailedEventName = "WorkspaceFailed"; - private const string DocumentOpenedEventName = "DocumentOpened"; - private const string DocumentClosedEventName = "DocumentClosed"; - private const string DocumentActiveContextChangedName = "DocumentActiveContextChanged"; - private const string TextDocumentOpenedEventName = "TextDocumentOpened"; - private const string TextDocumentClosedEventName = "TextDocumentClosed"; + internal enum WorkspaceEventType + { + DocumentActiveContextChanged, + DocumentClosed, + DocumentOpened, + TextDocumentClosed, + TextDocumentOpened, + WorkspaceChange, + WorkspaceChangedImmediate, + WorkspaceFailed, + } - private IWorkspaceEventListenerService _workspaceEventListenerService; + private IWorkspaceEventListenerService? _workspaceEventListenerService; #region Event Registration @@ -33,63 +34,63 @@ public abstract partial class Workspace /// Registers a handler that is fired whenever the current solution is changed. /// internal WorkspaceEventRegistration RegisterWorkspaceChangedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(WorkspaceChangeEventName, handler, options); + => RegisterHandler(WorkspaceEventType.WorkspaceChange, handler, options); /// /// Registers a handler that is fired whenever the current solution is changed. /// internal WorkspaceEventRegistration RegisterWorkspaceChangedImmediateHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(WorkspaceChangedImmediateEventName, handler, options); + => RegisterHandler(WorkspaceEventType.WorkspaceChangedImmediate, handler, options); /// /// Registers a handler that is fired whenever the workspace or part of its solution model /// fails to access a file or other external resource. /// internal WorkspaceEventRegistration RegisterWorkspaceFailedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(WorkspaceFailedEventName, handler, options); + => RegisterHandler(WorkspaceEventType.WorkspaceFailed, handler, options); /// /// Registers a handler that is fired when a is opened in the editor. /// internal WorkspaceEventRegistration RegisterDocumentOpenedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(DocumentOpenedEventName, handler, options); + => RegisterHandler(WorkspaceEventType.DocumentOpened, handler, options); /// /// Registers a handler that is fired when a is closed in the editor. /// internal WorkspaceEventRegistration RegisterDocumentClosedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(DocumentClosedEventName, handler, options); + => RegisterHandler(WorkspaceEventType.DocumentClosed, handler, options); /// /// Registers a handler that is fired when any is opened in the editor. /// internal WorkspaceEventRegistration RegisterTextDocumentOpenedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(TextDocumentOpenedEventName, handler, options); + => RegisterHandler(WorkspaceEventType.TextDocumentOpened, handler, options); /// /// Registers a handler that is fired when any is closed in the editor. /// internal WorkspaceEventRegistration RegisterTextDocumentClosedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(TextDocumentClosedEventName, handler, options); + => RegisterHandler(WorkspaceEventType.TextDocumentClosed, handler, options); /// /// Registers a handler that is fired when the active context document associated with a buffer /// changes. /// internal WorkspaceEventRegistration RegisterDocumentActiveContextChangedHandler(Action handler, WorkspaceEventOptions? options = null) - => RegisterHandler(DocumentActiveContextChangedName, handler, options); + => RegisterHandler(WorkspaceEventType.DocumentActiveContextChanged, handler, options); - private WorkspaceEventRegistration RegisterHandler(string eventName, Action handler, WorkspaceEventOptions? options = null) + private WorkspaceEventRegistration RegisterHandler(WorkspaceEventType eventType, Action handler, WorkspaceEventOptions? options = null) where TEventArgs : EventArgs { var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => handler((TEventArgs)args), options ?? WorkspaceEventOptions.DefaultOptions); - return _eventMap.AddEventHandler(eventName, handlerAndOptions); + return _eventMap.AddEventHandler(eventType, handlerAndOptions); } #endregion - protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solution oldSolution, Solution newSolution, ProjectId projectId = null, DocumentId documentId = null) + protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solution oldSolution, Solution newSolution, ProjectId? projectId = null, DocumentId? documentId = null) { if (newSolution == null) throw new ArgumentNullException(nameof(newSolution)); @@ -100,16 +101,16 @@ protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solutio if (projectId == null && documentId != null) projectId = documentId.ProjectId; - WorkspaceChangeEventArgs args = null; + WorkspaceChangeEventArgs? args = null; - var immediateHandlerSet = GetEventHandlers(WorkspaceChangedImmediateEventName); + var immediateHandlerSet = GetEventHandlers(WorkspaceEventType.WorkspaceChangedImmediate); if (immediateHandlerSet.HasHandlers) { args = new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); immediateHandlerSet.RaiseEvent(args, shouldRaiseEvent: static option => true); } - var handlerSet = GetEventHandlers(WorkspaceChangeEventName); + var handlerSet = GetEventHandlers(WorkspaceEventType.WorkspaceChange); if (handlerSet.HasHandlers) { args ??= new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); @@ -121,7 +122,7 @@ protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solutio protected internal virtual void OnWorkspaceFailed(WorkspaceDiagnostic diagnostic) { - var handlerSet = GetEventHandlers(WorkspaceFailedEventName); + var handlerSet = GetEventHandlers(WorkspaceEventType.WorkspaceFailed); if (handlerSet.HasHandlers) { var args = new WorkspaceDiagnosticEventArgs(diagnostic); @@ -130,19 +131,19 @@ protected internal virtual void OnWorkspaceFailed(WorkspaceDiagnostic diagnostic } protected Task RaiseDocumentOpenedEventAsync(Document document) - => RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), DocumentOpenedEventName); + => RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), WorkspaceEventType.DocumentOpened); protected Task RaiseTextDocumentOpenedEventAsync(TextDocument document) - => RaiseTextDocumentOpenedOrClosedEventAsync(document, new TextDocumentEventArgs(document), TextDocumentOpenedEventName); + => RaiseTextDocumentOpenedOrClosedEventAsync(document, new TextDocumentEventArgs(document), WorkspaceEventType.TextDocumentOpened); private Task RaiseTextDocumentOpenedOrClosedEventAsync( TDocument document, TDocumentEventArgs args, - string eventName) + WorkspaceEventType eventType) where TDocument : TextDocument where TDocumentEventArgs : EventArgs { - var handlerSet = GetEventHandlers(eventName); + var handlerSet = GetEventHandlers(eventType); if (handlerSet.HasHandlers && document != null) return this.ScheduleTask(args, handlerSet); @@ -150,10 +151,10 @@ private Task RaiseTextDocumentOpenedOrClosedEventAsync RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), DocumentClosedEventName); + => RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), WorkspaceEventType.DocumentClosed); protected Task RaiseTextDocumentClosedEventAsync(TextDocument document) - => RaiseTextDocumentOpenedOrClosedEventAsync(document, new TextDocumentEventArgs(document), TextDocumentClosedEventName); + => RaiseTextDocumentOpenedOrClosedEventAsync(document, new TextDocumentEventArgs(document), WorkspaceEventType.TextDocumentClosed); [Obsolete("This member is obsolete. Use the RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer, DocumentId, DocumentId) overload instead.", error: true)] protected Task RaiseDocumentActiveContextChangedEventAsync(Document document) @@ -164,7 +165,7 @@ protected Task RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer s if (sourceTextContainer == null || oldActiveContextDocumentId == null || newActiveContextDocumentId == null) return Task.CompletedTask; - var handlerSet = GetEventHandlers(DocumentActiveContextChangedName); + var handlerSet = GetEventHandlers(WorkspaceEventType.DocumentActiveContextChanged); if (handlerSet.HasHandlers) { // Capture the current solution snapshot (inside the _serializationLock of OnDocumentContextUpdated) @@ -177,12 +178,12 @@ protected Task RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer s return Task.CompletedTask; } - private EventHandlerSet GetEventHandlers(string eventName) + private EventHandlerSet GetEventHandlers(WorkspaceEventType eventType) { // this will register features that want to listen to workspace events // lazily first time workspace event is actually fired EnsureEventListeners(); - return _eventMap.GetEventHandlerSet(eventName); + return _eventMap.GetEventHandlerSet(eventType); } private void EnsureEventListeners() diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index d7cde62ae62ae..983ae3b86fd2a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -10,15 +10,15 @@ namespace Microsoft.CodeAnalysis; public abstract partial class Workspace { // Allows conversion of legacy event handlers to the new event system. - private readonly ConcurrentDictionary<(object, string), IDisposable> _disposableEventHandlers = new(); + private readonly ConcurrentDictionary<(object, WorkspaceEventType), IDisposable> _disposableEventHandlers = new(); /// /// An event raised whenever the current solution is changed. /// public event EventHandler WorkspaceChanged { - add => AddEventHandler(value, WorkspaceChangeEventName); - remove => RemoveEventHandler(value, WorkspaceChangeEventName); + add => AddEventHandler(value, WorkspaceEventType.WorkspaceChange); + remove => RemoveEventHandler(value, WorkspaceEventType.WorkspaceChange); } /// @@ -27,8 +27,8 @@ public event EventHandler WorkspaceChanged /// public event EventHandler WorkspaceFailed { - add => AddEventHandler(value, WorkspaceFailedEventName); - remove => RemoveEventHandler(value, WorkspaceFailedEventName); + add => AddEventHandler(value, WorkspaceEventType.WorkspaceFailed); + remove => RemoveEventHandler(value, WorkspaceEventType.WorkspaceFailed); } /// @@ -36,8 +36,8 @@ public event EventHandler WorkspaceFailed /// public event EventHandler DocumentOpened { - add => AddEventHandler(value, DocumentOpenedEventName); - remove => RemoveEventHandler(value, DocumentOpenedEventName); + add => AddEventHandler(value, WorkspaceEventType.DocumentOpened); + remove => RemoveEventHandler(value, WorkspaceEventType.DocumentOpened); } /// @@ -45,8 +45,8 @@ public event EventHandler DocumentOpened /// public event EventHandler TextDocumentOpened { - add => AddEventHandler(value, TextDocumentOpenedEventName); - remove => RemoveEventHandler(value, TextDocumentOpenedEventName); + add => AddEventHandler(value, WorkspaceEventType.TextDocumentOpened); + remove => RemoveEventHandler(value, WorkspaceEventType.TextDocumentOpened); } /// @@ -54,8 +54,8 @@ public event EventHandler TextDocumentOpened /// public event EventHandler DocumentClosed { - add => AddEventHandler(value, DocumentClosedEventName); - remove => RemoveEventHandler(value, DocumentClosedEventName); + add => AddEventHandler(value, WorkspaceEventType.DocumentClosed); + remove => RemoveEventHandler(value, WorkspaceEventType.DocumentClosed); } /// @@ -63,8 +63,8 @@ public event EventHandler DocumentClosed /// public event EventHandler TextDocumentClosed { - add => AddEventHandler(value, TextDocumentClosedEventName); - remove => RemoveEventHandler(value, TextDocumentClosedEventName); + add => AddEventHandler(value, WorkspaceEventType.TextDocumentClosed); + remove => RemoveEventHandler(value, WorkspaceEventType.TextDocumentClosed); } /// @@ -73,26 +73,27 @@ public event EventHandler TextDocumentClosed /// public event EventHandler DocumentActiveContextChanged { - add => AddEventHandler(value, DocumentActiveContextChangedName); - remove => RemoveEventHandler(value, DocumentActiveContextChangedName); + add => AddEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); + remove => RemoveEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); } - private void AddEventHandler(EventHandler eventHandler, string eventName) + private void AddEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) where TEventArgs : EventArgs { - Action handler = arg => eventHandler(sender: this, (TEventArgs)arg); - // Require main thread on the callback as this is used from publicly exposed eventss // and those callbacks may have main thread dependencies. - var disposer = RegisterHandler(eventName, handler, WorkspaceEventOptions.RequiresMainThreadOptions); + var disposer = RegisterHandler(eventType, (Action)handler, WorkspaceEventOptions.RequiresMainThreadOptions); + + _disposableEventHandlers[(eventHandler, eventType)] = disposer; - _disposableEventHandlers[(eventHandler, eventName)] = disposer; + void handler(EventArgs arg) + => eventHandler(sender: this, (TEventArgs)arg); } - private void RemoveEventHandler(EventHandler eventHandler, string eventName) + private void RemoveEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) where TEventArgs : EventArgs { - _disposableEventHandlers.TryRemove((eventHandler, eventName), out var disposer); + _disposableEventHandlers.TryRemove((eventHandler, eventType), out var disposer); disposer?.Dispose(); } From 7c21263eb703c5951ec78e1cf624912101472dcd Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 15 Apr 2025 14:00:56 -0700 Subject: [PATCH 12/17] 1) move a couple declarations around 2) Move try/catch into RaiseEvent implementation 3) Switch from SemaphoreSlim -> lock 4) Create GetEventHandlerSet_NoLock --- src/Compilers/Test/Core/FX/EventWaiter.cs | 2 +- ...lassificationTaggerProvider.TagComputer.cs | 4 +- .../Core/Remote/SolutionChecksumUpdater.cs | 3 +- .../Framework/WorkspaceChangeWatcher.vb | 2 +- .../Core/Portable/Workspace/Workspace.cs | 9 +--- .../Portable/Workspace/WorkspaceEventMap.cs | 52 +++++++++++-------- 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/Compilers/Test/Core/FX/EventWaiter.cs b/src/Compilers/Test/Core/FX/EventWaiter.cs index 2035065e6e703..145d0d2b7db87 100644 --- a/src/Compilers/Test/Core/FX/EventWaiter.cs +++ b/src/Compilers/Test/Core/FX/EventWaiter.cs @@ -53,7 +53,7 @@ public EventHandler Wrap(EventHandler input) public Action Wrap(Action input) { - return (args) => + return args => { try { diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 14d023eb67d75..42d41b0ceb400 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -46,8 +46,6 @@ private sealed record CachedServices( private readonly SyntacticClassificationTaggerProvider _taggerProvider; private readonly ITextBuffer2 _subjectBuffer; private readonly WorkspaceRegistration _workspaceRegistration; - private WorkspaceEventRegistration? _workspaceChangedDisposer; - private WorkspaceEventRegistration? _workspaceDocumentActiveContextChangedDisposer; private readonly CancellationTokenSource _disposalCancellationSource = new(); @@ -64,6 +62,8 @@ private sealed record CachedServices( private readonly TimeSpan _diffTimeout; private Workspace? _workspace; + private WorkspaceEventRegistration? _workspaceChangedDisposer; + private WorkspaceEventRegistration? _workspaceDocumentActiveContextChangedDisposer; /// /// Cached values for the last services we computed for a particular and _eventTypeToHandlerSet = []; public WorkspaceEventRegistration AddEventHandler(WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) { - using (_guard.DisposableWait()) + lock (_guard) { - _eventTypeToHandlerSet[eventType] = _eventTypeToHandlerSet.TryGetValue(eventType, out var handlers) - ? handlers.AddHandler(handlerAndOptions) - : EventHandlerSet.Empty.AddHandler(handlerAndOptions); + _eventTypeToHandlerSet[eventType] = GetEventHandlerSet_NoLock(eventType).AddHandler(handlerAndOptions); } return new WorkspaceEventRegistration(this, eventType, handlerAndOptions); @@ -33,38 +31,36 @@ public WorkspaceEventRegistration AddEventHandler(WorkspaceEventType eventType, public void RemoveEventHandler(WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) { - using (_guard.DisposableWait()) + lock (_guard) { - _eventTypeToHandlerSet.TryGetValue(eventType, out var originalHandlers); + var originalHandlers = GetEventHandlerSet_NoLock(eventType); + var newHandlers = originalHandlers.RemoveHandler(handlerAndOptions); // An earlier AddEventHandler call would have created the WorkspaceEventRegistration whose // disposal would have called this method. - Debug.Assert(originalHandlers != null); + Debug.Assert(originalHandlers != newHandlers); - if (originalHandlers != null) - { - var newHandlers = originalHandlers.RemoveHandler(handlerAndOptions); - Debug.Assert(originalHandlers != newHandlers); - - _eventTypeToHandlerSet[eventType] = newHandlers; - } + _eventTypeToHandlerSet[eventType] = newHandlers; } } public EventHandlerSet GetEventHandlerSet(WorkspaceEventType eventType) { - using (_guard.DisposableWait()) + lock (_guard) { - return _eventTypeToHandlerSet.TryGetValue(eventType, out var handlers) - ? handlers - : EventHandlerSet.Empty; + return GetEventHandlerSet_NoLock(eventType); } } - public readonly record struct WorkspaceEventHandlerAndOptions(Action Handler, WorkspaceEventOptions Options) + private EventHandlerSet GetEventHandlerSet_NoLock(WorkspaceEventType eventType) { + return _eventTypeToHandlerSet.TryGetValue(eventType, out var handlers) + ? handlers + : EventHandlerSet.Empty; } + public readonly record struct WorkspaceEventHandlerAndOptions(Action Handler, WorkspaceEventOptions Options); + public sealed class EventHandlerSet(ImmutableArray registries) { public static readonly EventHandlerSet Empty = new([]); @@ -80,6 +76,7 @@ public EventHandlerSet RemoveHandler(WorkspaceEventHandlerAndOptions handlerAndO { var registry = _registries.FirstOrDefault(static (r, handlerAndOptions) => r.HasHandlerAndOptions(handlerAndOptions), handlerAndOptions); + Debug.Assert(registry != null, "Expected to find a registry for the handler and options."); if (registry == null) return this; @@ -93,7 +90,7 @@ public EventHandlerSet RemoveHandler(WorkspaceEventHandlerAndOptions handlerAndO } public bool HasHandlers - => _registries.Any(static r => true); + => !_registries.IsEmpty; public bool HasMatchingOptions(Func isMatch) => _registries.Any(static (r, hasOptions) => r.HasMatchingOptions(hasOptions), isMatch); @@ -102,7 +99,16 @@ public void RaiseEvent(TEventArgs arg, Func Date: Tue, 15 Apr 2025 14:24:39 -0700 Subject: [PATCH 13/17] 1) Comment and add tuple name to _disposableEventHandlers 2) Add assert 3) Formatting changes --- .../Portable/Workspace/Workspace_EventsLegacy.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index 983ae3b86fd2a..f38032b8fbedf 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -4,13 +4,15 @@ using System; using System.Collections.Concurrent; +using System.Diagnostics; namespace Microsoft.CodeAnalysis; public abstract partial class Workspace { - // Allows conversion of legacy event handlers to the new event system. - private readonly ConcurrentDictionary<(object, WorkspaceEventType), IDisposable> _disposableEventHandlers = new(); + // Allows conversion of legacy event handlers to the new event system. The first item in + // the key's tuple is an EventHandler and thus stored as an object. + private readonly ConcurrentDictionary<(object EventHandler, WorkspaceEventType EventType), IDisposable> _disposableEventHandlers = new(); /// /// An event raised whenever the current solution is changed. @@ -80,13 +82,13 @@ public event EventHandler DocumentActiveC private void AddEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) where TEventArgs : EventArgs { - // Require main thread on the callback as this is used from publicly exposed eventss + // Require main thread on the callback as this is used from publicly exposed events // and those callbacks may have main thread dependencies. - var disposer = RegisterHandler(eventType, (Action)handler, WorkspaceEventOptions.RequiresMainThreadOptions); + var disposer = RegisterHandler(eventType, (Action)Handler, WorkspaceEventOptions.RequiresMainThreadOptions); _disposableEventHandlers[(eventHandler, eventType)] = disposer; - void handler(EventArgs arg) + void Handler(EventArgs arg) => eventHandler(sender: this, (TEventArgs)arg); } @@ -95,6 +97,8 @@ private void RemoveEventHandler(EventHandler eventHandle { _disposableEventHandlers.TryRemove((eventHandler, eventType), out var disposer); + Debug.Assert(disposer != null, $"Event handler for event type {eventType} was not registered."); + disposer?.Dispose(); } } From 76229c7be467f12321b7abbda1612ed584dd1529 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 15 Apr 2025 17:05:16 -0700 Subject: [PATCH 14/17] Better handle getting advised by the same event handler multiple times --- .../Workspace/Workspace_EventsLegacy.cs | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index f38032b8fbedf..d67909d840f3a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -3,7 +3,7 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Concurrent; +using System.Collections.Generic; using System.Diagnostics; namespace Microsoft.CodeAnalysis; @@ -12,7 +12,8 @@ public abstract partial class Workspace { // Allows conversion of legacy event handlers to the new event system. The first item in // the key's tuple is an EventHandler and thus stored as an object. - private readonly ConcurrentDictionary<(object EventHandler, WorkspaceEventType EventType), IDisposable> _disposableEventHandlers = new(); + private readonly Dictionary<(object EventHandler, WorkspaceEventType EventType), (int AdviseCount, IDisposable Disposer)> _disposableEventHandlers = new(); + private readonly object _gate = new(); /// /// An event raised whenever the current solution is changed. @@ -84,9 +85,28 @@ private void AddEventHandler(EventHandler eventHandler, { // Require main thread on the callback as this is used from publicly exposed events // and those callbacks may have main thread dependencies. - var disposer = RegisterHandler(eventType, (Action)Handler, WorkspaceEventOptions.RequiresMainThreadOptions); + IDisposable? disposer = null; - _disposableEventHandlers[(eventHandler, eventType)] = disposer; + lock (_gate) + { + if (_disposableEventHandlers.TryGetValue((eventHandler, eventType), out var adviseCountAndDisposer)) + { + (var adviseCount, disposer) = adviseCountAndDisposer; + + // If we already have a handler for this event type, update the map with the new advise count + _disposableEventHandlers[(eventHandler, eventType)] = (adviseCount + 1, disposer); + } + } + + if (disposer == null) + { + disposer = RegisterHandler(eventType, (Action)Handler, WorkspaceEventOptions.RequiresMainThreadOptions); + + lock (_gate) + { + _disposableEventHandlers[(eventHandler, eventType)] = (AdviseCount: 1, disposer); + } + } void Handler(EventArgs arg) => eventHandler(sender: this, (TEventArgs)arg); @@ -95,10 +115,26 @@ void Handler(EventArgs arg) private void RemoveEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) where TEventArgs : EventArgs { - _disposableEventHandlers.TryRemove((eventHandler, eventType), out var disposer); - - Debug.Assert(disposer != null, $"Event handler for event type {eventType} was not registered."); - - disposer?.Dispose(); + var existingAdviseCount = 0; + IDisposable? disposer = null; + + lock (_gate) + { + if (_disposableEventHandlers.TryGetValue((eventHandler, eventType), out var adviseCountAndDisposer)) + { + // If we already have a handler for this event type, just increment the advise count + // and return. + (existingAdviseCount, disposer) = adviseCountAndDisposer; + if (existingAdviseCount == 1) + _disposableEventHandlers.Remove((eventHandler, eventType)); + else + _disposableEventHandlers[(eventHandler, eventType)] = (existingAdviseCount - 1, disposer); + } + } + + Debug.Assert(existingAdviseCount > 0, $"Event handler for event type {eventType} was not registered."); + + if (existingAdviseCount == 1) + disposer?.Dispose(); } } From 92876f79a03804525f866143605dbf7358ef07db Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 16 Apr 2025 07:01:06 -0700 Subject: [PATCH 15/17] Fix an issue found by lsp unit tests where the Register/Deregister calls in LspWorkspaceRegistrationServices weren't doing matched calls to workspace changed registration --- .../LspWorkspaceRegistrationService.cs | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs b/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs index c20e1b912c8ec..b95d54399884d 100644 --- a/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs +++ b/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs @@ -12,7 +12,11 @@ namespace Microsoft.CodeAnalysis.LanguageServer; internal abstract class LspWorkspaceRegistrationService : IDisposable { private readonly object _gate = new(); + + // These arrays are kept in sync, with _workspaceChangedDisposers[i] representing + // a disposer for a WorkspaceChanged event on the workspace at _registrations[i] private ImmutableArray _registrations = []; + private ImmutableArray _workspaceChangedDisposers = []; public ImmutableArray GetAllRegistrations() { @@ -35,13 +39,15 @@ public virtual void Register(Workspace? workspace) m["WorkspacePartialSemanticsEnabled"] = workspace.PartialSemanticsEnabled; })); + // Forward workspace change events for all registered LSP workspaces. Requires main thread as it + // fires LspSolutionChanged which hasn't been guaranteed to be thread safe. + var workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnLspWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); + lock (_gate) { _registrations = _registrations.Add(workspace); + _workspaceChangedDisposers = _workspaceChangedDisposers.Add(workspaceChangedDisposer); } - - // Forward workspace change events for all registered LSP workspaces. - workspace.WorkspaceChanged += OnLspWorkspaceChanged; } public void Deregister(Workspace? workspace) @@ -49,14 +55,26 @@ public void Deregister(Workspace? workspace) if (workspace is null) return; - workspace.WorkspaceChanged -= OnLspWorkspaceChanged; + WorkspaceEventRegistration? disposer = null; lock (_gate) { - _registrations = _registrations.Remove(workspace); + var index = _registrations.IndexOf(workspace); + + // Handle the case where we were registered with a null workspace, but deregistered + // with a non-null workspace + if (index >= 0) + { + _registrations = _registrations.RemoveAt(index); + + disposer = _workspaceChangedDisposers[index]; + _workspaceChangedDisposers = _workspaceChangedDisposers.RemoveAt(index); + } } + + disposer?.Dispose(); } - private void OnLspWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private void OnLspWorkspaceChanged(WorkspaceChangeEventArgs e) { LspSolutionChanged?.Invoke(this, e); } @@ -65,12 +83,13 @@ public void Dispose() { lock (_gate) { - foreach (var workspace in _registrations) + foreach (var disposer in _workspaceChangedDisposers) { - workspace.WorkspaceChanged -= OnLspWorkspaceChanged; + disposer.Dispose(); } - _registrations = _registrations.Clear(); + _registrations = []; + _workspaceChangedDisposers = []; } } @@ -81,13 +100,3 @@ public void Dispose() /// public EventHandler? LspSolutionChanged; } - -internal sealed class LspWorkspaceRegisteredEventArgs : EventArgs -{ - public Workspace Workspace { get; } - - public LspWorkspaceRegisteredEventArgs(Workspace workspace) - { - Workspace = workspace; - } -} From 34dd6eda636c2c17bab91d98bab74e69686b6d87 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 23 Apr 2025 19:34:37 -0700 Subject: [PATCH 16/17] 1) Null out a couple members in SyntacticClassificationTaggerProvider 2) Rename some methods/members in Workspace_EventsLegacy.cs file 3) Fix potential multi-threaded issue in AddLegacyEventHandler by only taking the lock once 4) Be a bit more paranoid in WorkspaceEventRegistration.Dispose --- ...lassificationTaggerProvider.TagComputer.cs | 3 + .../Workspace/WorkspaceEventRegistration.cs | 7 +- .../Workspace/Workspace_EventsLegacy.cs | 84 +++++++++---------- 3 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs index 42d41b0ceb400..c3200dd73944b 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -246,7 +246,10 @@ public void DisconnectFromWorkspace() if (_workspace != null) { _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + _workspaceDocumentActiveContextChangedDisposer?.Dispose(); + _workspaceDocumentActiveContextChangedDisposer = null; _workspace = null; diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs index 4f805a9c7783b..e54261036c404 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Threading; using static Microsoft.CodeAnalysis.Workspace; using static Microsoft.CodeAnalysis.WorkspaceEventMap; @@ -21,7 +22,9 @@ internal sealed class WorkspaceEventRegistration(WorkspaceEventMap eventMap, Wor public void Dispose() { - _eventMap?.RemoveEventHandler(_eventType, _handlerAndOptions); - _eventMap = null; + // Protect against simultaneous disposal from multiple threads + var eventMap = Interlocked.Exchange(ref _eventMap, null); + + eventMap?.RemoveEventHandler(_eventType, _handlerAndOptions); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index d67909d840f3a..5e1719d8789be 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -10,18 +10,20 @@ namespace Microsoft.CodeAnalysis; public abstract partial class Workspace { - // Allows conversion of legacy event handlers to the new event system. The first item in - // the key's tuple is an EventHandler and thus stored as an object. - private readonly Dictionary<(object EventHandler, WorkspaceEventType EventType), (int AdviseCount, IDisposable Disposer)> _disposableEventHandlers = new(); - private readonly object _gate = new(); + /// + /// Allows conversion of legacy event handlers to the new event system. The first item in + /// the key's tuple is an EventHandler<TEventArgs> and thus stored as an object. + /// + private readonly Dictionary<(object eventHandler, WorkspaceEventType eventType), (int adviseCount, IDisposable disposer)> _disposableEventHandlers = new(); + private readonly object _legacyWorkspaceEventsGate = new(); /// /// An event raised whenever the current solution is changed. /// public event EventHandler WorkspaceChanged { - add => AddEventHandler(value, WorkspaceEventType.WorkspaceChange); - remove => RemoveEventHandler(value, WorkspaceEventType.WorkspaceChange); + add => AddLegacyEventHandler(value, WorkspaceEventType.WorkspaceChange); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.WorkspaceChange); } /// @@ -30,8 +32,8 @@ public event EventHandler WorkspaceChanged /// public event EventHandler WorkspaceFailed { - add => AddEventHandler(value, WorkspaceEventType.WorkspaceFailed); - remove => RemoveEventHandler(value, WorkspaceEventType.WorkspaceFailed); + add => AddLegacyEventHandler(value, WorkspaceEventType.WorkspaceFailed); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.WorkspaceFailed); } /// @@ -39,8 +41,8 @@ public event EventHandler WorkspaceFailed /// public event EventHandler DocumentOpened { - add => AddEventHandler(value, WorkspaceEventType.DocumentOpened); - remove => RemoveEventHandler(value, WorkspaceEventType.DocumentOpened); + add => AddLegacyEventHandler(value, WorkspaceEventType.DocumentOpened); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.DocumentOpened); } /// @@ -48,8 +50,8 @@ public event EventHandler DocumentOpened /// public event EventHandler TextDocumentOpened { - add => AddEventHandler(value, WorkspaceEventType.TextDocumentOpened); - remove => RemoveEventHandler(value, WorkspaceEventType.TextDocumentOpened); + add => AddLegacyEventHandler(value, WorkspaceEventType.TextDocumentOpened); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.TextDocumentOpened); } /// @@ -57,8 +59,8 @@ public event EventHandler TextDocumentOpened /// public event EventHandler DocumentClosed { - add => AddEventHandler(value, WorkspaceEventType.DocumentClosed); - remove => RemoveEventHandler(value, WorkspaceEventType.DocumentClosed); + add => AddLegacyEventHandler(value, WorkspaceEventType.DocumentClosed); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.DocumentClosed); } /// @@ -66,8 +68,8 @@ public event EventHandler DocumentClosed /// public event EventHandler TextDocumentClosed { - add => AddEventHandler(value, WorkspaceEventType.TextDocumentClosed); - remove => RemoveEventHandler(value, WorkspaceEventType.TextDocumentClosed); + add => AddLegacyEventHandler(value, WorkspaceEventType.TextDocumentClosed); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.TextDocumentClosed); } /// @@ -76,65 +78,63 @@ public event EventHandler TextDocumentClosed /// public event EventHandler DocumentActiveContextChanged { - add => AddEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); - remove => RemoveEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); + add => AddLegacyEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); } - private void AddEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) + private void AddLegacyEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) where TEventArgs : EventArgs { // Require main thread on the callback as this is used from publicly exposed events // and those callbacks may have main thread dependencies. - IDisposable? disposer = null; + var key = (eventHandler, eventType); - lock (_gate) + lock (_legacyWorkspaceEventsGate) { - if (_disposableEventHandlers.TryGetValue((eventHandler, eventType), out var adviseCountAndDisposer)) + if (_disposableEventHandlers.TryGetValue(key, out var adviseCountAndDisposer)) { - (var adviseCount, disposer) = adviseCountAndDisposer; - // If we already have a handler for this event type, update the map with the new advise count - _disposableEventHandlers[(eventHandler, eventType)] = (adviseCount + 1, disposer); + _disposableEventHandlers[key] = (adviseCountAndDisposer.adviseCount + 1, adviseCountAndDisposer.disposer); } - } - - if (disposer == null) - { - disposer = RegisterHandler(eventType, (Action)Handler, WorkspaceEventOptions.RequiresMainThreadOptions); - - lock (_gate) + else { - _disposableEventHandlers[(eventHandler, eventType)] = (AdviseCount: 1, disposer); + // Safe to call RegisterHandler inside the lock as it doesn't invoke code outside the workspace event map code. + var disposer = RegisterHandler(eventType, (Action)Handler, WorkspaceEventOptions.RequiresMainThreadOptions); + _disposableEventHandlers[key] = (adviseCount: 1, disposer); } } + return; + void Handler(EventArgs arg) => eventHandler(sender: this, (TEventArgs)arg); } - private void RemoveEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) + private void RemoveLegacyEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) where TEventArgs : EventArgs { - var existingAdviseCount = 0; IDisposable? disposer = null; - lock (_gate) + lock (_legacyWorkspaceEventsGate) { if (_disposableEventHandlers.TryGetValue((eventHandler, eventType), out var adviseCountAndDisposer)) { // If we already have a handler for this event type, just increment the advise count // and return. - (existingAdviseCount, disposer) = adviseCountAndDisposer; - if (existingAdviseCount == 1) + if (adviseCountAndDisposer.adviseCount == 1) + { + disposer = adviseCountAndDisposer.disposer; _disposableEventHandlers.Remove((eventHandler, eventType)); + } else - _disposableEventHandlers[(eventHandler, eventType)] = (existingAdviseCount - 1, disposer); + { + _disposableEventHandlers[(eventHandler, eventType)] = (adviseCountAndDisposer.adviseCount - 1, adviseCountAndDisposer.disposer); + } } } - Debug.Assert(existingAdviseCount > 0, $"Event handler for event type {eventType} was not registered."); + Debug.Assert(disposer != null, $"Event handler for event type {eventType} was not registered."); - if (existingAdviseCount == 1) - disposer?.Dispose(); + disposer?.Dispose(); } } From 5c66609b17da9c505f3bea4029f2662e3e3d23ac Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Sat, 26 Apr 2025 05:45:21 -0700 Subject: [PATCH 17/17] Comment cleanup, remove improper assert --- src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs | 5 ++++- .../Core/Portable/Workspace/Workspace_EventsLegacy.cs | 2 -- .../Core/Portable/Workspace/Workspace_Registration.cs | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs index 8adfc2542f811..9ac3bd1527c1c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -37,7 +37,10 @@ internal WorkspaceEventRegistration RegisterWorkspaceChangedHandler(Action RegisterHandler(WorkspaceEventType.WorkspaceChange, handler, options); /// - /// Registers a handler that is fired whenever the current solution is changed. + /// Registers a handler that is fired *immediately* whenever the current solution is changed. + /// Handlers should be written to be very fast. Always called from the thread changing the workspace, + /// regardless of the preferences indicated by the passed in options. This thread my vary depending + /// on the workspace. /// internal WorkspaceEventRegistration RegisterWorkspaceChangedImmediateHandler(Action handler, WorkspaceEventOptions? options = null) => RegisterHandler(WorkspaceEventType.WorkspaceChangedImmediate, handler, options); diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs index 5e1719d8789be..58028bd0d7c9a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -133,8 +133,6 @@ private void RemoveLegacyEventHandler(EventHandler event } } - Debug.Assert(disposer != null, $"Event handler for event type {eventType} was not registered."); - disposer?.Dispose(); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs index e78e2db1d3bf7..ea11c37658788 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Registration.cs @@ -44,8 +44,9 @@ protected void RegisterText(SourceTextContainer textContainer) var registration = GetWorkspaceRegistration(textContainer); registration.SetWorkspace(this); - // Require main thread on the callback as WorkspaceRegistration.RaiseEvents invokes Workspace.WorkspaecChanges which is publicly exposed - // and the event handlers may have main thread dependencies. + // Require main thread on the callback as WorkspaceRegistration.RaiseEvents invokes Workspace.WorkspaceChanges which is publicly exposed + // and the event handlers may have main thread dependencies. Potential cleanup here with relation to + // https://github.com/dotnet/roslyn/issues/32551 var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => registration.RaiseEvents(), WorkspaceEventOptions.RequiresMainThreadOptions); var handlerSet = EventHandlerSet.Create(handlerAndOptions); this.ScheduleTask(EventArgs.Empty, handlerSet);