Skip to content

Commit 87d95e9

Browse files
Improve event cleanup, thread safety, and error handling
--- - 🧹 Implement IDisposable in DocumentEvents for proper unsubscription from RunningDocumentTable - 🧵 Use a single _syncLock in TableDataSource for thread-safe access to collections - 🛡️ Wrap InfoBar host addition in try-catch and clean up COM event subscription on failure - ✂️ Simplify SolutionEvents.OnBeforeCloseSolution by removing unnecessary UI thread check and solution retrieval - 🧩 Update usings to include required Visual Studio namespaces
1 parent eedc1a9 commit 87d95e9

4 files changed

Lines changed: 53 additions & 25 deletions

File tree

src/toolkit/Community.VisualStudio.Toolkit.Shared/Documents/DocumentEvents.cs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using Microsoft.VisualStudio;
33
using Microsoft.VisualStudio.Shell;
44
using Microsoft.VisualStudio.Shell.Interop;
@@ -18,14 +18,40 @@ public partial class Events
1818
/// <summary>
1919
/// Events related to the editor documents.
2020
/// </summary>
21-
public class DocumentEvents : IVsRunningDocTableEvents
21+
public class DocumentEvents : IVsRunningDocTableEvents, IDisposable
2222
{
2323
private readonly RunningDocumentTable _rdt;
24+
private readonly uint _adviseCookie;
25+
private bool _disposed;
2426

2527
internal DocumentEvents()
2628
{
2729
_rdt = new RunningDocumentTable();
28-
_rdt.Advise(this);
30+
_adviseCookie = _rdt.Advise(this);
31+
}
32+
33+
/// <summary>
34+
/// Disposes the document events and unsubscribes from the running document table.
35+
/// </summary>
36+
public void Dispose()
37+
{
38+
Dispose(true);
39+
GC.SuppressFinalize(this);
40+
}
41+
42+
/// <summary>
43+
/// Disposes the document events and unsubscribes from the running document table.
44+
/// </summary>
45+
protected virtual void Dispose(bool disposing)
46+
{
47+
if (!_disposed)
48+
{
49+
if (disposing)
50+
{
51+
_rdt.Unadvise(_adviseCookie);
52+
}
53+
_disposed = true;
54+
}
2955
}
3056

3157
/// <summary>

src/toolkit/Community.VisualStudio.Toolkit.Shared/ErrorList/TableDataSource.cs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Collections.Generic;
33
using System.Linq;
44
using Microsoft;
@@ -25,6 +25,11 @@ public TableDataSource(string name)
2525
Initialize();
2626
}
2727

28+
/// <summary>
29+
/// Single lock object for thread-safe access to _managers and _snapshots
30+
/// </summary>
31+
private readonly object _syncLock = new();
32+
2833
/// <summary>
2934
/// Data sink subscriptions
3035
/// </summary>
@@ -91,7 +96,7 @@ protected void Initialize()
9196
/// <param name="manager">Subscription to register</param>
9297
private void AddSinkManager(SinkManager manager)
9398
{
94-
lock (_managers)
99+
lock (_syncLock)
95100
{
96101
_managers.Add(manager);
97102
}
@@ -103,7 +108,7 @@ private void AddSinkManager(SinkManager manager)
103108
/// <param name="manager">Subscription to unregister</param>
104109
private void RemoveSinkManager(SinkManager manager)
105110
{
106-
lock (_managers)
111+
lock (_syncLock)
107112
{
108113
_managers.Remove(manager);
109114
}
@@ -114,7 +119,7 @@ private void RemoveSinkManager(SinkManager manager)
114119
/// </summary>
115120
private void UpdateAllSinks()
116121
{
117-
lock (_managers)
122+
lock (_syncLock)
118123
{
119124
foreach (SinkManager manager in _managers)
120125
{
@@ -137,7 +142,7 @@ public void AddErrors(IEnumerable<ErrorListItem> errors)
137142

138143
IEnumerable<ErrorListItem> cleanErrors = errors.Where(e => e != null && !string.IsNullOrEmpty(e.FileName));
139144

140-
lock (_snapshots)
145+
lock (_syncLock)
141146
{
142147
foreach (IGrouping<string?, ErrorListItem>? fileErrorMap in cleanErrors.GroupBy(e => e.FileName))
143148
{
@@ -164,14 +169,11 @@ public void AddErrors(IEnumerable<ErrorListItem> errors)
164169
/// </summary>
165170
public void CleanAllErrors()
166171
{
167-
lock (_snapshots)
172+
lock (_syncLock)
168173
{
169-
lock (_managers)
174+
foreach (SinkManager manager in _managers)
170175
{
171-
foreach (SinkManager manager in _managers)
172-
{
173-
manager.Clear();
174-
}
176+
manager.Clear();
175177
}
176178

177179
foreach (TableEntriesSnapshot snapshot in _snapshots.Values)

src/toolkit/Community.VisualStudio.Toolkit.Shared/Notifications/InfoBar.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Threading.Tasks;
33
using System.Windows.Controls;
44
using Microsoft.VisualStudio.Shell;
@@ -122,11 +122,18 @@ public async Task<bool> TryShowInfoBarUIAsync()
122122
_uiElement = infoBarUIFactory.CreateInfoBar(_model);
123123
_uiElement.Advise(this, out _listenerCookie);
124124

125-
if (_host != null)
125+
try
126126
{
127127
_host.AddInfoBar(_uiElement);
128128
IsVisible = true;
129129
}
130+
catch
131+
{
132+
// If adding to host fails, clean up the COM event subscription to prevent leak
133+
_uiElement.Unadvise(_listenerCookie);
134+
_uiElement = null;
135+
throw;
136+
}
130137

131138
return IsVisible;
132139
}

src/toolkit/Community.VisualStudio.Toolkit.Shared/Solution/SolutionEvents.cs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using Microsoft.VisualStudio;
33
using Microsoft.VisualStudio.Shell;
44
using Microsoft.VisualStudio.Shell.Interop;
@@ -167,15 +167,8 @@ int IVsSolutionEvents.OnQueryCloseSolution(object pUnkReserved, ref int pfCancel
167167

168168
int IVsSolutionEvents.OnBeforeCloseSolution(object pUnkReserved)
169169
{
170-
ThreadHelper.ThrowIfNotOnUIThread();
171-
172-
if (OnBeforeCloseSolution != null)
173-
{
174-
SolutionItem? solution = VS.Solutions.GetCurrentSolution();
175-
OnBeforeCloseSolution?.Invoke();
176-
}
170+
OnBeforeCloseSolution?.Invoke();
177171
return VSConstants.S_OK;
178-
179172
}
180173

181174
int IVsSolutionEvents.OnAfterCloseSolution(object pUnkReserved)

0 commit comments

Comments
 (0)