diff --git a/src/Compilers/Test/Core/FX/EventWaiter.cs b/src/Compilers/Test/Core/FX/EventWaiter.cs index 524d7178c2598..145d0d2b7db87 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,6 +51,25 @@ public EventHandler Wrap(EventHandler input) }; } + public Action Wrap(Action input) + { + return args => + { + try + { + input(args); + } + catch (Exception ex) + { + _capturedException = ex; + } + finally + { + _eventSignal.Set(); + } + }; + } + /// /// 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..6c3434d5b8171 100644 --- a/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs +++ b/src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs @@ -62,7 +62,7 @@ public async Task TestEmptySolutionUpdateDoesNotFireEvents() var solution = workspace.CurrentSolution; var workspaceChanged = false; - workspace.WorkspaceChanged += (s, 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); @@ -815,15 +815,15 @@ public async Task TestDocumentEvents() 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, + 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( - (sender, args) => Assert.True(args.Document.Id == document.Id, + 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.")); - workspace.DocumentOpened += documentOpenedEventHandler; - workspace.DocumentClosed += documentClosedEventHandler; + var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandler); + var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandler); workspace.OpenDocument(document.Id); workspace.CloseDocument(document.Id); @@ -833,15 +833,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 +852,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)); } @@ -881,15 +881,15 @@ public async Task TestSourceGeneratedDocumentEvents() // 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.")); + 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( - (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.")); + 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.")); - workspace.DocumentOpened += documentOpenedEventHandler; - workspace.DocumentClosed += documentClosedEventHandler; + var documentOpenedDisposer = workspace.RegisterDocumentOpenedHandler(documentOpenedEventHandler); + var documentClosedDisposer = workspace.RegisterDocumentClosedHandler(documentClosedEventHandler); workspace.OpenSourceGeneratedDocument(document.Id); var sourceGeneratedDocumentId = workspace.GetDocumentIdInCurrentContext(document.GetOpenTextContainer()); @@ -902,15 +902,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 +921,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 +944,16 @@ 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 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.")); - 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.")); + 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.")); - workspace.TextDocumentOpened += documentOpenedEventHandler; - workspace.TextDocumentClosed += documentClosedEventHandler; + var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandler); + var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandler); workspace.OpenAdditionalDocument(document.Id); workspace.CloseAdditionalDocument(document.Id); @@ -963,15 +963,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 +982,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 +1005,16 @@ 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, + 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.")); - var documentClosedEventHandler = closeWaiter.Wrap( - (sender, args) => Assert.True(args.Document.Id == document.Id, + 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.")); - workspace.TextDocumentOpened += documentOpenedEventHandler; - workspace.TextDocumentClosed += documentClosedEventHandler; + var textDocumentOpenedDisposer = workspace.RegisterTextDocumentOpenedHandler(textDocumentOpenedEventHandler); + var textDocumentClosedDisposer = workspace.RegisterTextDocumentClosedHandler(textDocumentClosedEventHandler); workspace.OpenAnalyzerConfigDocument(document.Id); workspace.CloseAnalyzerConfigDocument(document.Id); @@ -1024,15 +1024,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 +1043,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 +1420,11 @@ 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); - }; + }); 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..c3200dd73944b 100644 --- a/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Classification/Syntactic/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -62,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 (); - _workspace.DocumentOpened += DocumentOpened; - _workspace.DocumentClosed += DocumentClosed; + _documentOpenedHandlerDisposer = _workspace.RegisterDocumentOpenedHandler(DocumentOpened); + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(DocumentClosed); } 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 void DocumentClosed(DocumentEventArgs e) { if (e.Document.FilePath != null) { @@ -153,7 +155,7 @@ private void DocumentClosed(object? sender, DocumentEventArgs e) } } - private void DocumentOpened(object? sender, DocumentEventArgs e) + private void DocumentOpened(DocumentEventArgs e) => _ = TrackActiveSpansAsync(e.Document); private async Task TrackActiveSpansAsync(Document designTimeDocument) diff --git a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs index 762beb5668272..479211f9a3f08 100644 --- a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs +++ b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs @@ -43,7 +43,8 @@ internal sealed class SolutionChecksumUpdater private readonly AsyncBatchingWorkQueue _synchronizeActiveDocumentQueue; private readonly object _gate = new(); - private bool _isSynchronizeWorkspacePaused; + private readonly WorkspaceEventRegistration _workspaceChangedDisposer; + private readonly WorkspaceEventRegistration _workspaceChangedImmediateDisposer; private readonly CancellationToken _shutdownToken; @@ -52,6 +53,8 @@ internal sealed class SolutionChecksumUpdater private const string SynchronizeTextChangesStatusSucceededKeyName = nameof(SolutionChecksumUpdater) + "." + SynchronizeTextChangesStatusSucceededMetricName; private const string SynchronizeTextChangesStatusFailedKeyName = nameof(SolutionChecksumUpdater) + "." + SynchronizeTextChangesStatusFailedMetricName; + private bool _isSynchronizeWorkspacePaused; + public SolutionChecksumUpdater( Workspace workspace, IAsynchronousOperationListenerProvider listenerProvider, @@ -79,8 +82,8 @@ public SolutionChecksumUpdater( shutdownToken); // start listening workspace change event - _workspace.WorkspaceChanged += OnWorkspaceChanged; - _workspace.WorkspaceChangedImmediate += OnWorkspaceChangedImmediate; + _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(this.OnWorkspaceChanged); + _workspaceChangedImmediateDisposer = _workspace.RegisterWorkspaceChangedImmediateHandler(OnWorkspaceChangedImmediate); _documentTrackingService.ActiveDocumentChanged += OnActiveDocumentChanged; if (_globalOperationService != null) @@ -100,8 +103,8 @@ public void Shutdown() PauseSynchronizingPrimaryWorkspace(); _documentTrackingService.ActiveDocumentChanged -= OnActiveDocumentChanged; - _workspace.WorkspaceChanged -= OnWorkspaceChanged; - _workspace.WorkspaceChangedImmediate -= OnWorkspaceChangedImmediate; + _workspaceChangedDisposer.Dispose(); + _workspaceChangedImmediateDisposer.Dispose(); if (_globalOperationService != null) { @@ -136,7 +139,7 @@ private void ResumeSynchronizingPrimaryWorkspace() } } - private void OnWorkspaceChanged(object? sender, 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. @@ -147,7 +150,7 @@ private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) } } - 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 f027bad3102a9..ee02ce7ef6cf7 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -11,20 +11,21 @@ internal partial class TaggerEventSources { private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subjectBuffer) : AbstractWorkspaceTrackingTaggerEventSource(subjectBuffer) { + private WorkspaceEventRegistration? _documentActiveContextChangedDisposer; + + // Require main thread on the callback as RaiseChanged implementors may have main thread dependencies. protected override void ConnectToWorkspace(Workspace workspace) - => workspace.DocumentActiveContextChanged += OnDocumentActiveContextChanged; + => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.RequiresMainThreadOptions); protected override void DisconnectFromWorkspace(Workspace workspace) - => workspace.DocumentActiveContextChanged -= OnDocumentActiveContextChanged; + => _documentActiveContextChangedDisposer?.Dispose(); - private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContextChangedEventArgs e) + private void OnDocumentActiveContextChanged(DocumentActiveContextChangedEventArgs e) { var document = SubjectBuffer.AsTextContainer().GetOpenDocumentInCurrentContext(); if (document != null && document.Id == e.NewActiveContextDocumentId) - { this.RaiseChanged(); - } } } } diff --git a/src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService_IncrementalAnalyzer.cs b/src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService_IncrementalAnalyzer.cs index b9598aacd82f4..74a5ef511712f 100644 --- a/src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService_IncrementalAnalyzer.cs +++ b/src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService_IncrementalAnalyzer.cs @@ -14,11 +14,8 @@ private DiagnosticIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace worksp private DiagnosticIncrementalAnalyzer CreateIncrementalAnalyzerCallback(Workspace workspace) { // subscribe to active context changed event for new workspace - workspace.DocumentActiveContextChanged += OnDocumentActiveContextChanged; + _ = workspace.RegisterDocumentActiveContextChangedHandler(args => RequestDiagnosticRefresh()); return new DiagnosticIncrementalAnalyzer(this, AnalyzerInfoCache, this.GlobalOptions); } - - private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContextChangedEventArgs e) - => RequestDiagnosticRefresh(); } diff --git a/src/Features/Lsif/Generator/Program.cs b/src/Features/Lsif/Generator/Program.cs index 3284c2673c433..8d32b07de419a 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/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs b/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs index 9ed6a051b03d4..1b7a230ddb6b6 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; }, workspace)); + // 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; - } -} diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs index 01e7b4d0faf02..d608cda53e5a9 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractCreateServicesOnTextViewConnection.cs @@ -26,10 +26,11 @@ 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 readonly WorkspaceEventRegistration _workspaceDocumentOpenedDisposer; private bool _initialized = false; protected VisualStudioWorkspace Workspace { get; } @@ -56,9 +57,12 @@ public AbstractCreateServicesOnTextViewConnection( listenerProvider.GetListener(FeatureAttribute.CompletionSet), threadingContext.DisposalToken); - Workspace.DocumentOpened += QueueWorkOnDocumentOpened; + _workspaceDocumentOpenedDisposer = Workspace.RegisterDocumentOpenedHandler(QueueWorkOnDocumentOpened); } + public void Dispose() + => _workspaceDocumentOpenedDisposer.Dispose(); + void IWpfTextViewConnectionListener.SubjectBuffersConnected(IWpfTextView textView, ConnectionReason reason, Collection subjectBuffers) { if (!_initialized) @@ -96,7 +100,7 @@ private async ValueTask BatchProcessProjectsWithOpenedDocumentAsync(ImmutableSeg } } - private void QueueWorkOnDocumentOpened(object sender, DocumentEventArgs e) + private void QueueWorkOnDocumentOpened(DocumentEventArgs e) { if (e.Document.Project.Language == _languageName) _workQueue.AddWork(e.Document.Project.Id); diff --git a/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs b/src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs index 635b2e643bad0..7e82dc6a8a2a2 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 Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; @@ -172,7 +173,11 @@ 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)); + // 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); return; } diff --git a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs index d0a6733054fdf..229cee06bb58a 100644 --- a/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs +++ b/src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs @@ -62,10 +62,9 @@ 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)); await textViewWindowVerifier.TestServices.Editor.ShowLightBulbAsync(cancellationToken); @@ -155,12 +154,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 +188,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..00a894fa3f2b6 100644 --- a/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb +++ b/src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/WorkspaceChangeWatcher.vb @@ -4,26 +4,27 @@ Imports Microsoft.CodeAnalysis Imports Microsoft.CodeAnalysis.Shared.TestHooks -Imports Roslyn.Test.Utilities Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework Friend Class WorkspaceChangeWatcher Implements IDisposable - Private ReadOnly _environment As TestEnvironment Private ReadOnly _asynchronousOperationWaiter As IAsynchronousOperationWaiter + Private ReadOnly _workspaceChangedDisposer As WorkspaceEventRegistration Private _changeEvents As New List(Of WorkspaceChangeEventArgs) Public Sub New(environment As TestEnvironment) - _environment = environment - Dim listenerProvider = environment.ExportProvider.GetExportedValue(Of AsynchronousOperationListenerProvider)() _asynchronousOperationWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace) - AddHandler environment.Workspace.WorkspaceChanged, AddressOf OnWorkspaceChanged + _workspaceChangedDisposer = environment.Workspace.RegisterWorkspaceChangedHandler(AddressOf OnWorkspaceChanged) + End Sub + + Public Sub Dispose() Implements IDisposable.Dispose + _workspaceChangedDisposer.Dispose() End Sub - Private Sub OnWorkspaceChanged(sender As Object, e As WorkspaceChangeEventArgs) + Private Sub OnWorkspaceChanged(e As WorkspaceChangeEventArgs) _changeEvents.Add(e) End Sub @@ -35,9 +36,5 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr _changeEvents = New List(Of WorkspaceChangeEventArgs)() Return changeEvents End Function - - Public Sub Dispose() Implements IDisposable.Dispose - RemoveHandler _environment.Workspace.WorkspaceChanged, AddressOf OnWorkspaceChanged - 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..a8f3afa25c7c7 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/XamlProjectService.cs @@ -27,7 +27,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 +36,7 @@ internal sealed partial class XamlProjectService private readonly IThreadingContext _threadingContext; private readonly Dictionary _xamlProjects = []; private readonly ConcurrentDictionary _documentIds = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + private readonly WorkspaceEventRegistration _documentClosedHandlerDisposer; private RunningDocumentTable? _rdt; private IVsSolution? _vsSolution; @@ -58,9 +59,13 @@ public XamlProjectService( AnalyzerService = analyzerService; - _workspace.DocumentClosed += OnDocumentClosed; + // Require main thread on the callback as OnDocumentClosed is not thread safe. + _documentClosedHandlerDisposer = _workspace.RegisterDocumentClosedHandler(OnDocumentClosed, WorkspaceEventOptions.RequiresMainThreadOptions); } + public void Dispose() + => _documentClosedHandlerDisposer.Dispose(); + public static IXamlDocumentAnalyzerService? AnalyzerService { get; private set; } public DocumentId? TrackOpenDocument(string filePath) @@ -188,13 +193,11 @@ public XamlProjectService( return null; } - private void OnDocumentClosed(object sender, DocumentEventArgs e) + private void OnDocumentClosed(DocumentEventArgs e) { var filePath = e.Document.FilePath; if (filePath == null) - { return; - } if (_documentIds.TryGetValue(filePath, out var documentId)) { diff --git a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs index 04560fce22dba..85bf53f439eba 100644 --- a/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs +++ b/src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingWorkspaceExtensions.cs @@ -16,28 +16,19 @@ 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 WorkspaceEventRegistration _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(HandleEvent) + : workspace.RegisterTextDocumentClosedHandler(HandleEvent); + + void HandleEvent(TextDocumentEventArgs args) + => action(new UnitTestingTextDocumentEventArgsWrapper(args)); } 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 a89523e20d5a3..751c3e213bcdb 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 Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; @@ -39,7 +40,7 @@ public abstract partial class Workspace : IDisposable private readonly IAsynchronousOperationListener _asyncOperationListener; - private readonly AsyncBatchingWorkQueue _workQueue; + private readonly AsyncBatchingWorkQueue<(EventArgs, EventHandlerSet)> _eventHandlerWorkQueue; private readonly CancellationTokenSource _workQueueTokenSource = new(); private readonly ITaskSchedulerProvider _taskSchedulerProvider; @@ -80,9 +81,9 @@ protected Workspace(HostServices host, string? workspaceKind) var listenerProvider = Services.GetRequiredService(); _asyncOperationListener = listenerProvider.GetListener(); - _workQueue = new( + _eventHandlerWorkQueue = new( TimeSpan.Zero, - ProcessWorkQueueAsync, + ProcessEventHandlerWorkQueueAsync, _asyncOperationListener, _workQueueTokenSource.Token); @@ -592,8 +593,18 @@ 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(); + // 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); + } + + [SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "This is a Task wrapper, not an asynchronous method.")] + internal Task ScheduleTask(EventArgs args, EventHandlerSet handlerSet) + { + _eventHandlerWorkQueue.AddWork((args, handlerSet)); + return _eventHandlerWorkQueue.WaitUntilCurrentBatchCompletesAsync(); } /// @@ -603,8 +614,12 @@ protected internal Task ScheduleTask(Action action, string? taskName = "Workspac protected internal async Task ScheduleTask(Func func, string? taskName = "Workspace.Task") { T? result = default; - _workQueue.AddWork(() => result = func()); - await _workQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(false); + + // 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); return result!; } #pragma warning restore IDE0060 // Remove unused parameter @@ -716,25 +731,37 @@ protected virtual void Dispose(bool finalize) _workQueueTokenSource.Cancel(); } - private async ValueTask ProcessWorkQueueAsync(ImmutableSegmentedList list, CancellationToken cancellationToken) + private async ValueTask ProcessEventHandlerWorkQueueAsync(ImmutableSegmentedList<(EventArgs Args, EventHandlerSet HandlerSet)> list, CancellationToken cancellationToken) { - // Hop over to the right scheduler to execute all this work. - await Task.Factory.StartNew(() => + // 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); + + // 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))) { - foreach (var item in list) + 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); + } + + static void ProcessWorkQueueHelper( + ImmutableSegmentedList<(EventArgs Args, EventHandlerSet handlerSet)> list, + Func shouldRaise, + CancellationToken cancellationToken) + { + foreach (var (args, handlerSet) in list) { cancellationToken.ThrowIfCancellationRequested(); - try - { - item(); - } - catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e)) - { - // Ensure we continue onto further items, even if one particular item fails. - } + handlerSet.RaiseEvent(args, shouldRaise); } - }, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false); + } } #region Host API diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs new file mode 100644 index 0000000000000..25c312703ccbc --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventMap.cs @@ -0,0 +1,135 @@ +// 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.Diagnostics; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.ErrorReporting; +using Roslyn.Utilities; +using static Microsoft.CodeAnalysis.Workspace; + +namespace Microsoft.CodeAnalysis; + +internal sealed class WorkspaceEventMap +{ + private readonly object _guard = new(); + private readonly Dictionary _eventTypeToHandlerSet = []; + + public WorkspaceEventRegistration AddEventHandler(WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) + { + lock (_guard) + { + _eventTypeToHandlerSet[eventType] = GetEventHandlerSet_NoLock(eventType).AddHandler(handlerAndOptions); + } + + return new WorkspaceEventRegistration(this, eventType, handlerAndOptions); + } + + public void RemoveEventHandler(WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) + { + lock (_guard) + { + 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 != newHandlers); + + _eventTypeToHandlerSet[eventType] = newHandlers; + } + } + + public EventHandlerSet GetEventHandlerSet(WorkspaceEventType eventType) + { + lock (_guard) + { + return GetEventHandlerSet_NoLock(eventType); + } + } + + 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([]); + private readonly ImmutableArray _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))); + + public EventHandlerSet RemoveHandler(WorkspaceEventHandlerAndOptions handlerAndOptions) + { + 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; + + // 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. + registry.Unregister(); + + var newRegistries = _registries.Remove(registry); + + return newRegistries.IsEmpty ? Empty : new(newRegistries); + } + + public bool HasHandlers + => !_registries.IsEmpty; + + public bool HasMatchingOptions(Func isMatch) + => _registries.Any(static (r, hasOptions) => r.HasMatchingOptions(hasOptions), isMatch); + + public void RaiseEvent(TEventArgs arg, Func shouldRaiseEvent) + where TEventArgs : EventArgs + { + foreach (var registry in _registries) + { + try + { + registry.RaiseEvent(arg, shouldRaiseEvent); + } + catch (Exception e) when (FatalError.ReportAndCatch(e)) + { + // Ensure we continue onto further items, even if one particular item fails. + } + } + } + } + + public sealed class Registry(WorkspaceEventHandlerAndOptions handlerAndOptions) + { + private readonly WorkspaceEventHandlerAndOptions _handlerAndOptions = handlerAndOptions; + private bool _disableHandler = false; + + public void Unregister() + => _disableHandler = true; + + 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 (!_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 new file mode 100644 index 0000000000000..522e52fc13a47 --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/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 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 new file mode 100644 index 0000000000000..e54261036c404 --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceEventRegistration.cs @@ -0,0 +1,30 @@ +// 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; +using static Microsoft.CodeAnalysis.Workspace; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; + +namespace Microsoft.CodeAnalysis; + +/// +/// 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, WorkspaceEventType eventType, WorkspaceEventHandlerAndOptions handlerAndOptions) : IDisposable +{ + private readonly WorkspaceEventType _eventType = eventType; + private readonly WorkspaceEventHandlerAndOptions _handlerAndOptions = handlerAndOptions; + private WorkspaceEventMap? _eventMap = eventMap; + + public void Dispose() + { + // 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_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 38cba793833a0..9ac3bd1527c1c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs @@ -2,260 +2,162 @@ // 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; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; +using static Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; public abstract partial class Workspace { - private readonly EventMap _eventMap = new(); + private readonly WorkspaceEventMap _eventMap = new(); + + internal enum WorkspaceEventType + { + DocumentActiveContextChanged, + DocumentClosed, + DocumentOpened, + TextDocumentClosed, + TextDocumentOpened, + WorkspaceChange, + WorkspaceChangedImmediate, + WorkspaceFailed, + } - 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"; + private IWorkspaceEventListenerService? _workspaceEventListenerService; - 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(WorkspaceEventType.WorkspaceChange, handler, options); - remove - { - _eventMap.RemoveEventHandler(WorkspaceChangeEventName, value); - } - } + /// + /// 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); + + /// + /// 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(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(WorkspaceEventType.DocumentOpened, 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(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(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(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(WorkspaceEventType.DocumentActiveContextChanged, handler, options); + + private WorkspaceEventRegistration RegisterHandler(WorkspaceEventType eventType, Action handler, WorkspaceEventOptions? options = null) + where TEventArgs : EventArgs { - add - { - _eventMap.AddEventHandler(WorkspaceChangedImmediateEventName, value); - } + var handlerAndOptions = new WorkspaceEventHandlerAndOptions(args => handler((TEventArgs)args), options ?? WorkspaceEventOptions.DefaultOptions); - remove - { - _eventMap.RemoveEventHandler(WorkspaceChangedImmediateEventName, value); - } + return _eventMap.AddEventHandler(eventType, handlerAndOptions); } - protected Task RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind kind, Solution oldSolution, Solution newSolution, ProjectId projectId = null, DocumentId documentId = null) + #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); + WorkspaceChangeEventArgs? args = null; - if (ev.HasHandlers) + var immediateHandlerSet = GetEventHandlers(WorkspaceEventType.WorkspaceChangedImmediate); + 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); } - ev = GetEventHandlers(WorkspaceChangeEventName); - if (ev.HasHandlers) + var handlerSet = GetEventHandlers(WorkspaceEventType.WorkspaceChange); + if (handlerSet.HasHandlers) { args ??= new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId); - return this.ScheduleTask(() => - { - RaiseEventForHandlers(ev, sender: this, args, FunctionId.Workspace_Events); - }, WorkspaceChangeEventName); - } - else - { - return Task.CompletedTask; + return this.ScheduleTask(args, handlerSet); } - 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)); - } - } - } - - /// - /// 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(WorkspaceEventType.WorkspaceFailed); + 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); - } - } + => 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 ev = GetEventHandlers(eventName); - if (ev.HasHandlers && document != null) - { - return this.ScheduleTask(() => - { - ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); - }, eventName); - } - else - { - return Task.CompletedTask; - } - } - - /// - /// An event that is fired when a is closed in the editor. - /// - public event EventHandler DocumentClosed - { - add - { - _eventMap.AddEventHandler(DocumentClosedEventName, value); - } + var handlerSet = GetEventHandlers(eventType); + if (handlerSet.HasHandlers && document != null) + return this.ScheduleTask(args, handlerSet); - 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); - } - } + => RaiseTextDocumentOpenedOrClosedEventAsync(document, new DocumentEventArgs(document), WorkspaceEventType.DocumentClosed); 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); - } - } + => 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) @@ -263,30 +165,28 @@ protected Task RaiseDocumentActiveContextChangedEventAsync(Document document) protected Task RaiseDocumentActiveContextChangedEventAsync(SourceTextContainer sourceTextContainer, DocumentId oldActiveContextDocumentId, DocumentId newActiveContextDocumentId) { - var ev = GetEventHandlers(DocumentActiveContextChangedName); - if (ev.HasHandlers && sourceTextContainer != null && oldActiveContextDocumentId != null && newActiveContextDocumentId != null) + if (sourceTextContainer == null || oldActiveContextDocumentId == null || newActiveContextDocumentId == null) + return Task.CompletedTask; + + var handlerSet = GetEventHandlers(WorkspaceEventType.DocumentActiveContextChanged); + if (handlerSet.HasHandlers) { // Capture the current solution snapshot (inside the _serializationLock of OnDocumentContextUpdated) var currentSolution = this.CurrentSolution; + var args = new DocumentActiveContextChangedEventArgs(currentSolution, sourceTextContainer, oldActiveContextDocumentId, newActiveContextDocumentId); - return 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"); - } - else - { - return Task.CompletedTask; + return this.ScheduleTask(args, handlerSet); } + + return Task.CompletedTask; } - private EventMap.EventHandlerSet> GetEventHandlers(string eventName) where T : EventArgs + 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.GetEventHandlers>(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 new file mode 100644 index 0000000000000..58028bd0d7c9a --- /dev/null +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_EventsLegacy.cs @@ -0,0 +1,138 @@ +// 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.Diagnostics; + +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<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 => AddLegacyEventHandler(value, WorkspaceEventType.WorkspaceChange); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.WorkspaceChange); + } + + /// + /// 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 => AddLegacyEventHandler(value, WorkspaceEventType.WorkspaceFailed); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.WorkspaceFailed); + } + + /// + /// An event that is fired when a is opened in the editor. + /// + public event EventHandler DocumentOpened + { + add => AddLegacyEventHandler(value, WorkspaceEventType.DocumentOpened); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.DocumentOpened); + } + + /// + /// An event that is fired when any is opened in the editor. + /// + public event EventHandler TextDocumentOpened + { + add => AddLegacyEventHandler(value, WorkspaceEventType.TextDocumentOpened); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.TextDocumentOpened); + } + + /// + /// An event that is fired when a is closed in the editor. + /// + public event EventHandler DocumentClosed + { + add => AddLegacyEventHandler(value, WorkspaceEventType.DocumentClosed); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.DocumentClosed); + } + + /// + /// An event that is fired when any is closed in the editor. + /// + public event EventHandler TextDocumentClosed + { + add => AddLegacyEventHandler(value, WorkspaceEventType.TextDocumentClosed); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.TextDocumentClosed); + } + + /// + /// An event that is fired when the active context document associated with a buffer + /// changes. + /// + public event EventHandler DocumentActiveContextChanged + { + add => AddLegacyEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); + remove => RemoveLegacyEventHandler(value, WorkspaceEventType.DocumentActiveContextChanged); + } + + 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. + var key = (eventHandler, eventType); + + lock (_legacyWorkspaceEventsGate) + { + if (_disposableEventHandlers.TryGetValue(key, out var adviseCountAndDisposer)) + { + // If we already have a handler for this event type, update the map with the new advise count + _disposableEventHandlers[key] = (adviseCountAndDisposer.adviseCount + 1, adviseCountAndDisposer.disposer); + } + else + { + // 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 RemoveLegacyEventHandler(EventHandler eventHandler, WorkspaceEventType eventType) + where TEventArgs : EventArgs + { + IDisposable? disposer = null; + + 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. + if (adviseCountAndDisposer.adviseCount == 1) + { + disposer = adviseCountAndDisposer.disposer; + _disposableEventHandlers.Remove((eventHandler, eventType)); + } + else + { + _disposableEventHandlers[(eventHandler, eventType)] = (adviseCountAndDisposer.adviseCount - 1, adviseCountAndDisposer.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..ea11c37658788 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 Microsoft.CodeAnalysis.WorkspaceEventMap; namespace Microsoft.CodeAnalysis; @@ -42,7 +43,13 @@ protected void RegisterText(SourceTextContainer textContainer) var registration = GetWorkspaceRegistration(textContainer); registration.SetWorkspace(this); - this.ScheduleTask(registration.RaiseEvents, "Workspace.RegisterText"); + + // 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); } /// diff --git a/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs b/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs index 617c0cdcc393d..d6a71b46fb465 100644 --- a/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs +++ b/src/Workspaces/CoreTest/WorkspaceTests/AdhocWorkspaceTests.cs @@ -420,14 +420,14 @@ 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); } - }; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -453,14 +453,14 @@ 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); } - }; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -487,14 +487,14 @@ 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); } - }; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); @@ -518,14 +518,14 @@ 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); } - }; + }); Assert.True(ws.TryApplyChanges(changedDoc.Project.Solution)); diff --git a/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs b/src/Workspaces/CoreTestUtilities/WorkspaceExtensions.cs index da5f1a3cc3091..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.WorkspaceChanged += wew.Wrap((sender, args) => action(args)); + _ = workspace.RegisterWorkspaceChangedHandler(wew.Wrap(action)); return wew; } } 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) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index d6707a6081612..c05dfea8823bd 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -508,7 +508,6 @@ - diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs deleted file mode 100644 index c7e66cb79fe3e..0000000000000 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs +++ /dev/null @@ -1,176 +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.Linq; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.ErrorReporting; - -namespace Roslyn.Utilities; - -internal sealed class EventMap -{ - private readonly NonReentrantLock _guard = new(); - - private readonly Dictionary _eventNameToRegistries = []; - - public EventMap() - { - } - - public void AddEventHandler(string eventName, TEventHandler eventHandler) - where TEventHandler : class - { - using (_guard.DisposableWait()) - { - var registries = GetRegistries_NoLock(eventName); - var newRegistries = registries.Add(new Registry(eventHandler)); - SetRegistries_NoLock(eventName, newRegistries); - } - } - - public void RemoveEventHandler(string eventName, TEventHandler eventHandler) - where TEventHandler : class - { - 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); - } - } - } - - [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 - { - using (_guard.DisposableWait()) - { - return GetRegistries_NoLock(eventName); - } - } - - private ImmutableArray> GetRegistries_NoLock(string eventName) - where TEventHandler : class - { - _guard.AssertHasLock(); - if (_eventNameToRegistries.TryGetValue(eventName, out var registries)) - { - return (ImmutableArray>)registries; - } - - return []; - } - - private void SetRegistries_NoLock(string eventName, ImmutableArray> registries) - where TEventHandler : class - { - _guard.AssertHasLock(); - - _eventNameToRegistries[eventName] = registries; - } - - internal sealed class Registry(TEventHandler handler) : IEquatable?> - where TEventHandler : class - { - private TEventHandler? _handler = handler; - - public void Unregister() - => _handler = null; - - public void Invoke(Action invoker, TArg arg) - { - var handler = _handler; - if (handler != null) - { - invoker(handler, arg); - } - } - - public bool HasHandler(TEventHandler handler) - => handler.Equals(_handler); - - public bool Equals(Registry? other) - { - if (other == null) - { - return false; - } - - if (other._handler == null && _handler == null) - { - return true; - } - - if (other._handler == null || _handler == null) - { - return false; - } - - return other._handler.Equals(_handler); - } - - public override bool Equals(object? obj) - => Equals(obj as Registry); - - public override int GetHashCode() - => _handler == null ? 0 : _handler.GetHashCode(); - } - - internal struct EventHandlerSet - where TEventHandler : class - { - private readonly ImmutableArray> _registries; - - internal EventHandlerSet(ImmutableArray> registries) - => _registries = registries; - - public readonly bool HasHandlers - { - get { return _registries != null && _registries.Length > 0; } - } - - public readonly void RaiseEvent(Action invoker, TArg arg) - { - 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 - } - } - } - } - } -}