Skip to content

Commit 67cce50

Browse files
Remove most remaining uses of WorkspaceChanged off the UI thread (#78778)
Commit-at-a-time is recommended, since each use gets its own commit with further analysis there. The remaining uses of eventing that hit the UI thread (of any kind of workspace event) are: 1. Inline rename subscribes when there is an active rename to cancel on changes it doesn't know about. Since this is only active during the session, it doesn't seem worth it change it. 2. MiscellanousFilesWorkspace subscribes to workspace _registration_ changed, which is when a document is opened/closed, and that's not urgent to fix and restricted to just registration events. 3. XamlProjectService subscribes to document _closed_. This only happens if XAML is loaded in the first place, and then is only raised on the close path. I looked at making a small change to move it off but the code is generally quite scary and since it's document close only that's not chatty.
2 parents aafd6eb + c6c55d5 commit 67cce50

File tree

7 files changed

+14
-32
lines changed

7 files changed

+14
-32
lines changed

src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@ private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subject
1313
{
1414
private WorkspaceEventRegistration? _documentActiveContextChangedDisposer;
1515

16-
// Require main thread on the callback as RaiseChanged implementors may have main thread dependencies.
1716
protected override void ConnectToWorkspace(Workspace workspace)
18-
=> _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.RequiresMainThreadOptions);
17+
=> _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged);
1918

2019
protected override void DisconnectFromWorkspace(Workspace workspace)
2120
=> _documentActiveContextChangedDisposer?.Dispose();

src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ internal interface ITaggerEventSource
4141

4242
/// <summary>
4343
/// An event has happened on the thing the tagger is attached to. The tagger should
44-
/// recompute tags.
44+
/// recompute tags. May be raised on any thread.
4545
/// </summary>
4646
event EventHandler<TaggerEventArgs> Changed;
4747
}

src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,7 @@ public CodeAnalysisDiagnosticAnalyzerService(
4949
_workspace = workspace;
5050
_diagnosticAnalyzerService = _workspace.Services.GetRequiredService<IDiagnosticAnalyzerService>();
5151

52-
// Main thread as OnWorkspaceChanged's call to IDiagnosticAnalyzerService.RequestDiagnosticRefresh isn't clear on
53-
// threading requirements
54-
_ = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions);
52+
_ = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged);
5553
}
5654

5755
private void OnWorkspaceChanged(WorkspaceChangeEventArgs e)

src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ internal interface IDiagnosticAnalyzerService : IWorkspaceService
2222
/// <summary>
2323
/// Re-analyze all projects and documents. This will cause an LSP diagnostic refresh request to be sent.
2424
/// </summary>
25+
/// <remarks>
26+
/// This implementation must be safe to call on any thread.
27+
/// </remarks>
2528
void RequestDiagnosticRefresh();
2629

2730
/// <summary>

src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ public virtual void Register(Workspace? workspace)
3939
m["WorkspacePartialSemanticsEnabled"] = workspace.PartialSemanticsEnabled;
4040
}, workspace));
4141

42-
// Forward workspace change events for all registered LSP workspaces. Requires main thread as it
43-
// fires LspSolutionChanged which hasn't been guaranteed to be thread safe.
44-
var workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnLspWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions);
42+
var workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnLspWorkspaceChanged);
4543

4644
lock (_gate)
4745
{
@@ -94,9 +92,7 @@ public void Dispose()
9492
}
9593

9694
/// <summary>
97-
/// Indicates whether the LSP solution has changed in a non-tracked document context.
98-
///
99-
/// <b>IMPORTANT:</b> Implementations of this event handler should do as little synchronous work as possible since this will block.
95+
/// Indicates whether the LSP solution has changed in a non-tracked document context. May be raised on any thread.
10096
/// </summary>
10197
public EventHandler<WorkspaceChangeEventArgs>? LspSolutionChanged;
10298
}

src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ public StackTraceExplorerViewModel(IThreadingContext threadingContext, Workspace
5353
_threadingContext = threadingContext;
5454
_workspace = workspace;
5555

56-
// Main thread dependency as Workspace_WorkspaceChanged modifies an ObservableCollection
57-
_ = workspace.RegisterWorkspaceChangedHandler(Workspace_WorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions);
58-
5956
_classificationTypeMap = classificationTypeMap;
6057
_formatMap = formatMap;
6158

@@ -106,15 +103,6 @@ private void CallstackLines_CollectionChanged(object sender, System.Collections.
106103
NotifyPropertyChanged(nameof(IsInstructionTextVisible));
107104
}
108105

109-
private void Workspace_WorkspaceChanged(WorkspaceChangeEventArgs e)
110-
{
111-
if (e.Kind == WorkspaceChangeKind.SolutionChanged)
112-
{
113-
Selection = null;
114-
Frames.Clear();
115-
}
116-
}
117-
118106
private FrameViewModel GetViewModel(ParsedFrame frame)
119107
=> frame switch
120108
{

src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ internal sealed partial class CpsDiagnosticItemSource : BaseDiagnosticAndGenerat
1919
{
2020
private readonly IVsHierarchyItem _item;
2121
private readonly string _projectDirectoryPath;
22+
private readonly string? _analyzerFilePath;
2223

2324
private WorkspaceEventRegistration? _workspaceChangedDisposer;
2425

@@ -37,6 +38,8 @@ public CpsDiagnosticItemSource(
3738
_item = item;
3839
_projectDirectoryPath = Path.GetDirectoryName(projectPath);
3940

41+
_analyzerFilePath = CpsUtilities.ExtractAnalyzerFilePath(_projectDirectoryPath, _item.CanonicalName);
42+
4043
this.AnalyzerReference = TryGetAnalyzerReference(Workspace.CurrentSolution);
4144
if (this.AnalyzerReference == null)
4245
{
@@ -47,9 +50,7 @@ public CpsDiagnosticItemSource(
4750
// then connect to it.
4851
if (workspace.CurrentSolution.ContainsProject(projectId))
4952
{
50-
// Main thread dependency as OnWorkspaceChangedLookForAnalyzer accesses the IVsHierarchy
51-
// and fires the PropertyChanged event
52-
_workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChangedLookForAnalyzer, WorkspaceEventOptions.RequiresMainThreadOptions);
53+
_workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChangedLookForAnalyzer);
5354
item.PropertyChanged += IVsHierarchyItem_PropertyChanged;
5455

5556
// Now that we've subscribed, check once more in case we missed the event
@@ -118,14 +119,11 @@ private void OnWorkspaceChangedLookForAnalyzer(WorkspaceChangeEventArgs e)
118119
return null;
119120
}
120121

121-
var canonicalName = _item.CanonicalName;
122-
var analyzerFilePath = CpsUtilities.ExtractAnalyzerFilePath(_projectDirectoryPath, canonicalName);
123-
124-
if (string.IsNullOrEmpty(analyzerFilePath))
122+
if (string.IsNullOrEmpty(_analyzerFilePath))
125123
{
126124
return null;
127125
}
128126

129-
return project.AnalyzerReferences.FirstOrDefault(r => string.Equals(r.FullPath, analyzerFilePath, StringComparison.OrdinalIgnoreCase));
127+
return project.AnalyzerReferences.FirstOrDefault(r => string.Equals(r.FullPath, _analyzerFilePath, StringComparison.OrdinalIgnoreCase));
130128
}
131129
}

0 commit comments

Comments
 (0)