Skip to content

Commit 6cc637e

Browse files
retailcoderretailcoder
authored
Merge pull request #2687 from comintern/next
Stop colossal memory leak of event delegates.
2 parents 75be978 + 01cdb4d commit 6cc637e

File tree

8 files changed

+147
-115
lines changed

8 files changed

+147
-115
lines changed

‎RetailCoder.VBE/Inspections/ObsoleteGlobalInspection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
2525
{
2626
var issues = from item in UserDeclarations
2727
where item.Accessibility == Accessibility.Global && item.Context != null
28-
select new ObsoleteGlobalInspectionResult(this, new QualifiedContext<ParserRuleContext>(item.QualifiedName.QualifiedModuleName, item.Context));
28+
select new ObsoleteGlobalInspectionResult(this, new QualifiedContext<ParserRuleContext>(item.QualifiedName.QualifiedModuleName, item.Context), item);
2929

3030
return issues;
3131
}

‎RetailCoder.VBE/Inspections/Results/ObsoleteGlobalInspectionResult.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@
55
using Rubberduck.Inspections.QuickFixes;
66
using Rubberduck.Inspections.Resources;
77
using Rubberduck.Parsing;
8+
using Rubberduck.Parsing.Symbols;
89

910
namespace Rubberduck.Inspections.Results
1011
{
1112
public class ObsoleteGlobalInspectionResult : InspectionResultBase
1213
{
1314
private IEnumerable<QuickFixBase> _quickFixes;
1415

15-
public ObsoleteGlobalInspectionResult(IInspection inspection, QualifiedContext<ParserRuleContext> context)
16-
: base(inspection, context.ModuleName, context.Context)
16+
public ObsoleteGlobalInspectionResult(IInspection inspection, QualifiedContext<ParserRuleContext> context, Declaration target)
17+
: base(inspection, context.ModuleName, context.Context, target)
1718
{ }
1819

1920
public override IEnumerable<QuickFixBase> QuickFixes

‎RetailCoder.VBE/UI/SourceControl/SourceControlViewViewModel.cs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Linq;
99
using System.Windows.Forms;
1010
using System.Windows.Media.Imaging;
11+
using Microsoft.Vbe.Interop;
1112
using NLog;
1213
using Rubberduck.Parsing.VBA;
1314
using Rubberduck.SettingsProvider;
@@ -16,6 +17,7 @@
1617
using Rubberduck.UI.Command.MenuItems;
1718
using Rubberduck.VBEditor.Events;
1819
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
20+
using Rubberduck.VBEditor.SafeComWrappers.VBA;
1921
using resx = Rubberduck.UI.SourceControl.SourceControl;
2022

2123
namespace Rubberduck.UI.SourceControl
@@ -80,7 +82,7 @@ public SourceControlViewViewModel(
8082

8183
_openCommandPromptCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), _ => OpenCommandPrompt());
8284

83-
_vbe.VBProjects.ProjectRemoved += ProjectRemoved;
85+
Rubberduck.VBEditor.SafeComWrappers.VBA.VBProjects.ProjectRemoved += ProjectRemoved;
8486
foreach (var project in _vbe.VBProjects.Where(proj => proj.VBComponents != null))
8587
{
8688
AddComponentEventHandlers(project);
@@ -108,16 +110,16 @@ public void SetTab(SourceControlTab tab)
108110

109111
private void AddComponentEventHandlers(IVBProject project)
110112
{
111-
project.VBComponents.ComponentAdded += ComponentAdded;
112-
project.VBComponents.ComponentRemoved += ComponentRemoved;
113-
project.VBComponents.ComponentRenamed += ComponentRenamed;
113+
VBEditor.SafeComWrappers.VBA.VBComponents.ComponentAdded += ComponentAdded;
114+
VBEditor.SafeComWrappers.VBA.VBComponents.ComponentRemoved += ComponentRemoved;
115+
VBEditor.SafeComWrappers.VBA.VBComponents.ComponentRenamed += ComponentRenamed;
114116
}
115117

116118
private void RemoveComponentEventHandlers(IVBProject project)
117119
{
118-
project.VBComponents.ComponentAdded -= ComponentAdded;
119-
project.VBComponents.ComponentRemoved -= ComponentRemoved;
120-
project.VBComponents.ComponentRenamed -= ComponentRenamed;
120+
VBEditor.SafeComWrappers.VBA.VBComponents.ComponentAdded -= ComponentAdded;
121+
VBEditor.SafeComWrappers.VBA.VBComponents.ComponentRemoved -= ComponentRemoved;
122+
VBEditor.SafeComWrappers.VBA.VBComponents.ComponentRenamed -= ComponentRenamed;
121123
}
122124

123125
private void ComponentAdded(object sender, ComponentEventArgs e)
@@ -1088,7 +1090,7 @@ public void Dispose()
10881090
_fileSystemWatcher.Dispose();
10891091
}
10901092

1091-
_vbe.VBProjects.ProjectRemoved -= ProjectRemoved;
1093+
VBEditor.SafeComWrappers.VBA.VBProjects.ProjectRemoved -= ProjectRemoved;
10921094
foreach (var project in _vbe.VBProjects.Where(proj => proj.VBComponents != null))
10931095
{
10941096
RemoveComponentEventHandlers(project);

‎Rubberduck.Parsing/VBA/RubberduckParserState.cs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using Rubberduck.VBEditor.Events;
1717
using Rubberduck.VBEditor.SafeComWrappers;
1818
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
19+
using Rubberduck.VBEditor.SafeComWrappers.VBA;
1920

2021
// ReSharper disable LoopCanBeConvertedToQuery
2122

@@ -84,9 +85,9 @@ public RubberduckParserState(IVBE vbe)
8485

8586
if (_vbe != null && _vbe.VBProjects != null)
8687
{
87-
_vbe.VBProjects.ProjectAdded += Sinks_ProjectAdded;
88-
_vbe.VBProjects.ProjectRemoved += Sinks_ProjectRemoved;
89-
_vbe.VBProjects.ProjectRenamed += Sinks_ProjectRenamed;
88+
VBProjects.ProjectAdded += Sinks_ProjectAdded;
89+
VBProjects.ProjectRemoved += Sinks_ProjectRemoved;
90+
VBProjects.ProjectRenamed += Sinks_ProjectRenamed;
9091
foreach (var project in _vbe.VBProjects.Where(proj => proj.VBComponents != null))
9192
{
9293
AddComponentEventHandlers(project);
@@ -100,16 +101,16 @@ public RubberduckParserState(IVBE vbe)
100101

101102
private void AddComponentEventHandlers(IVBProject project)
102103
{
103-
project.VBComponents.ComponentAdded += Sinks_ComponentAdded;
104-
project.VBComponents.ComponentRemoved += Sinks_ComponentRemoved;
105-
project.VBComponents.ComponentRenamed += Sinks_ComponentRenamed;
104+
VBComponents.ComponentAdded += Sinks_ComponentAdded;
105+
VBComponents.ComponentRemoved += Sinks_ComponentRemoved;
106+
VBComponents.ComponentRenamed += Sinks_ComponentRenamed;
106107
}
107108

108109
private void RemoveComponentEventHandlers(IVBProject project)
109110
{
110-
project.VBComponents.ComponentAdded -= Sinks_ComponentAdded;
111-
project.VBComponents.ComponentRemoved -= Sinks_ComponentRemoved;
112-
project.VBComponents.ComponentRenamed -= Sinks_ComponentRenamed;
111+
VBComponents.ComponentAdded -= Sinks_ComponentAdded;
112+
VBComponents.ComponentRemoved -= Sinks_ComponentRemoved;
113+
VBComponents.ComponentRenamed -= Sinks_ComponentRenamed;
113114
}
114115

115116
private void Sinks_ProjectAdded(object sender, ProjectEventArgs e)
@@ -1111,9 +1112,9 @@ public void Dispose()
11111112

11121113
if (_vbe != null && _vbe.VBProjects != null)
11131114
{
1114-
_vbe.VBProjects.ProjectAdded -= Sinks_ProjectAdded;
1115-
_vbe.VBProjects.ProjectRemoved -= Sinks_ProjectRemoved;
1116-
_vbe.VBProjects.ProjectRenamed -= Sinks_ProjectRenamed;
1115+
VBProjects.ProjectAdded -= Sinks_ProjectAdded;
1116+
VBProjects.ProjectRemoved -= Sinks_ProjectRemoved;
1117+
VBProjects.ProjectRenamed -= Sinks_ProjectRenamed;
11171118
foreach (var project in _vbe.VBProjects.Where(proj => proj.VBComponents != null))
11181119
{
11191120
RemoveComponentEventHandlers(project);

‎Rubberduck.VBEEditor/SafeComWrappers/Abstract/IVBComponents.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
66
{
77
public interface IVBComponents : ISafeComWrapper, IComCollection<IVBComponent>, IEquatable<IVBComponents>
88
{
9-
event EventHandler<ComponentEventArgs> ComponentAdded;
10-
event EventHandler<ComponentEventArgs> ComponentRemoved;
11-
event EventHandler<ComponentRenamedEventArgs> ComponentRenamed;
12-
event EventHandler<ComponentEventArgs> ComponentSelected;
13-
event EventHandler<ComponentEventArgs> ComponentActivated;
14-
event EventHandler<ComponentEventArgs> ComponentReloaded;
9+
//event EventHandler<ComponentEventArgs> ComponentAdded;
10+
//event EventHandler<ComponentEventArgs> ComponentRemoved;
11+
//event EventHandler<ComponentRenamedEventArgs> ComponentRenamed;
12+
//event EventHandler<ComponentEventArgs> ComponentSelected;
13+
//event EventHandler<ComponentEventArgs> ComponentActivated;
14+
//event EventHandler<ComponentEventArgs> ComponentReloaded;
1515

1616
new IVBComponent this[object index] { get; }
1717

‎Rubberduck.VBEEditor/SafeComWrappers/Abstract/IVBProjects.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
66
{
77
public interface IVBProjects : ISafeComWrapper, IComCollection<IVBProject>, IEquatable<IVBProjects>
88
{
9-
event EventHandler<ProjectEventArgs> ProjectActivated;
10-
event EventHandler<ProjectEventArgs> ProjectAdded;
11-
event EventHandler<ProjectEventArgs> ProjectRemoved;
12-
event EventHandler<ProjectRenamedEventArgs> ProjectRenamed;
9+
//event EventHandler<ProjectEventArgs> ProjectActivated;
10+
//event EventHandler<ProjectEventArgs> ProjectAdded;
11+
//event EventHandler<ProjectEventArgs> ProjectRemoved;
12+
//event EventHandler<ProjectRenamedEventArgs> ProjectRenamed;
1313

1414
IVBE VBE { get; }
1515
IVBE Parent { get; }

‎Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponents.cs

Lines changed: 60 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections;
3+
using System.Collections.Concurrent;
34
using System.Collections.Generic;
45
using System.IO;
56
using System.Linq;
@@ -14,6 +15,8 @@ namespace Rubberduck.VBEditor.SafeComWrappers.VBA
1415
public class VBComponents : SafeComWrapper<VB.VBComponents>, IVBComponents
1516
{
1617
private static readonly Guid VBComponentsEventsGuid = new Guid("0002E116-0000-0000-C000-000000000046");
18+
private static HashSet<VB.VBComponents> _handlers = new HashSet<VB.VBComponents>();
19+
private static object _lockObject = new object();
1720

1821
private enum ComponentEventDispId
1922
{
@@ -27,7 +30,7 @@ private enum ComponentEventDispId
2730

2831
public VBComponents(VB.VBComponents target) : base(target)
2932
{
30-
AttachEvents();
33+
AttachEvents(Target);
3134
}
3235

3336
public int Count
@@ -96,7 +99,7 @@ public override void Release(bool final = false)
9699
{
97100
if (!IsWrappingNullReference)
98101
{
99-
DetatchEvents();
102+
DetatchEvents(Target);
100103
for (var i = 1; i <= Count; i++)
101104
{
102105
this[i].Release();
@@ -192,99 +195,108 @@ public void RemoveSafely(IVBComponent component)
192195

193196
#region Events
194197

195-
private bool _eventsAttached;
196-
private void AttachEvents()
198+
private static void AttachEvents(VB.VBComponents components)
197199
{
198-
if (!_eventsAttached && !IsWrappingNullReference)
200+
lock (_lockObject)
199201
{
200-
_componentAdded = OnComponentAdded;
201-
_componentRemoved = OnComponentRemoved;
202-
_componentRenamed = OnComponentRenamed;
203-
_componentSelected = OnComponentSelected;
204-
_componentActivated = OnComponentActivated;
205-
_componentReloaded = OnComponentReloaded;
206-
ComEventsHelper.Combine(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemAdded, _componentAdded);
207-
ComEventsHelper.Combine(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemRemoved, _componentRemoved);
208-
ComEventsHelper.Combine(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemRenamed, _componentRenamed);
209-
ComEventsHelper.Combine(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemSelected, _componentSelected);
210-
ComEventsHelper.Combine(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemActivated, _componentActivated);
211-
ComEventsHelper.Combine(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemReloaded, _componentReloaded);
212-
_eventsAttached = true;
202+
if (!_handlers.Contains(components))
203+
{
204+
_componentAdded = OnComponentAdded;
205+
_componentRemoved = OnComponentRemoved;
206+
_componentRenamed = OnComponentRenamed;
207+
_componentSelected = OnComponentSelected;
208+
_componentActivated = OnComponentActivated;
209+
_componentReloaded = OnComponentReloaded;
210+
ComEventsHelper.Combine(components, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemAdded, _componentAdded);
211+
ComEventsHelper.Combine(components, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemRemoved, _componentRemoved);
212+
ComEventsHelper.Combine(components, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemRenamed, _componentRenamed);
213+
ComEventsHelper.Combine(components, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemSelected, _componentSelected);
214+
ComEventsHelper.Combine(components, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemActivated, _componentActivated);
215+
ComEventsHelper.Combine(components, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemReloaded, _componentReloaded);
216+
_handlers.Add(components);
217+
}
213218
}
214219
}
215220

216-
private void DetatchEvents()
221+
private static void DetatchEvents(VB.VBComponents components)
217222
{
218-
if (!_eventsAttached && !IsWrappingNullReference)
223+
lock (_lockObject)
219224
{
220-
ComEventsHelper.Remove(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemAdded, _componentAdded);
221-
ComEventsHelper.Remove(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemRemoved, _componentRemoved);
222-
ComEventsHelper.Remove(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemRenamed, _componentRenamed);
223-
ComEventsHelper.Remove(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemSelected, _componentSelected);
224-
ComEventsHelper.Remove(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemActivated, _componentActivated);
225-
ComEventsHelper.Remove(Target, VBComponentsEventsGuid, (int)ComponentEventDispId.ItemReloaded, _componentReloaded);
226-
_eventsAttached = false;
225+
if (_handlers.Contains(components))
226+
{
227+
ComEventsHelper.Remove(components, VBComponentsEventsGuid, (int) ComponentEventDispId.ItemAdded, _componentAdded);
228+
ComEventsHelper.Remove(components, VBComponentsEventsGuid, (int) ComponentEventDispId.ItemRemoved, _componentRemoved);
229+
ComEventsHelper.Remove(components, VBComponentsEventsGuid, (int) ComponentEventDispId.ItemRenamed, _componentRenamed);
230+
ComEventsHelper.Remove(components, VBComponentsEventsGuid, (int) ComponentEventDispId.ItemSelected, _componentSelected);
231+
ComEventsHelper.Remove(components, VBComponentsEventsGuid, (int) ComponentEventDispId.ItemActivated, _componentActivated);
232+
ComEventsHelper.Remove(components, VBComponentsEventsGuid, (int) ComponentEventDispId.ItemReloaded, _componentReloaded);
233+
_handlers.Remove(components);
234+
}
227235
}
228236
}
229237

230238
private delegate void ItemAddedDelegate(VB.VBComponent vbComponent);
231-
private ItemAddedDelegate _componentAdded;
232-
public event EventHandler<ComponentEventArgs> ComponentAdded;
233-
private void OnComponentAdded(VB.VBComponent vbComponent)
239+
private static ItemAddedDelegate _componentAdded;
240+
public static event EventHandler<ComponentEventArgs> ComponentAdded;
241+
private static void OnComponentAdded(VB.VBComponent vbComponent)
234242
{
235243
OnDispatch(ComponentAdded, vbComponent);
236244
}
237245

238246
private delegate void ItemRemovedDelegate(VB.VBComponent vbComponent);
239-
private ItemRemovedDelegate _componentRemoved;
240-
public event EventHandler<ComponentEventArgs> ComponentRemoved;
241-
private void OnComponentRemoved(VB.VBComponent vbComponent)
247+
private static ItemRemovedDelegate _componentRemoved;
248+
public static event EventHandler<ComponentEventArgs> ComponentRemoved;
249+
private static void OnComponentRemoved(VB.VBComponent vbComponent)
242250
{
243251
OnDispatch(ComponentRemoved, vbComponent);
244252
}
245253

246254
private delegate void ItemRenamedDelegate(VB.VBComponent vbComponent, string oldName);
247-
private ItemRenamedDelegate _componentRenamed;
248-
public event EventHandler<ComponentRenamedEventArgs> ComponentRenamed;
249-
private void OnComponentRenamed(VB.VBComponent vbComponent, string oldName)
255+
private static ItemRenamedDelegate _componentRenamed;
256+
public static event EventHandler<ComponentRenamedEventArgs> ComponentRenamed;
257+
private static void OnComponentRenamed(VB.VBComponent vbComponent, string oldName)
250258
{
251259
var handler = ComponentRenamed;
252260
if (handler != null)
253261
{
254-
handler.Invoke(this, new ComponentRenamedEventArgs(Parent.ProjectId, Parent, new VBComponent(vbComponent), oldName));
262+
var component = new VBComponent(vbComponent);
263+
var project = component.Collection.Parent;
264+
handler.Invoke(component, new ComponentRenamedEventArgs(project.ProjectId, project, new VBComponent(vbComponent), oldName));
255265
}
256266
}
257267

258268
private delegate void ItemSelectedDelegate(VB.VBComponent vbComponent);
259-
private ItemSelectedDelegate _componentSelected;
260-
public event EventHandler<ComponentEventArgs> ComponentSelected;
261-
private void OnComponentSelected(VB.VBComponent vbComponent)
269+
private static ItemSelectedDelegate _componentSelected;
270+
public static event EventHandler<ComponentEventArgs> ComponentSelected;
271+
private static void OnComponentSelected(VB.VBComponent vbComponent)
262272
{
263273
OnDispatch(ComponentSelected, vbComponent);
264274
}
265275

266276
private delegate void ItemActivatedDelegate(VB.VBComponent vbComponent);
267-
private ItemActivatedDelegate _componentActivated;
268-
public event EventHandler<ComponentEventArgs> ComponentActivated;
269-
private void OnComponentActivated(VB.VBComponent vbComponent)
277+
private static ItemActivatedDelegate _componentActivated;
278+
public static event EventHandler<ComponentEventArgs> ComponentActivated;
279+
private static void OnComponentActivated(VB.VBComponent vbComponent)
270280
{
271281
OnDispatch(ComponentActivated, vbComponent);
272282
}
273283

274284
private delegate void ItemReloadedDelegate(VB.VBComponent vbComponent);
275-
private ItemReloadedDelegate _componentReloaded;
276-
public event EventHandler<ComponentEventArgs> ComponentReloaded;
277-
private void OnComponentReloaded(VB.VBComponent vbComponent)
285+
private static ItemReloadedDelegate _componentReloaded;
286+
public static event EventHandler<ComponentEventArgs> ComponentReloaded;
287+
private static void OnComponentReloaded(VB.VBComponent vbComponent)
278288
{
279289
OnDispatch(ComponentReloaded, vbComponent);
280290
}
281291

282-
private void OnDispatch(EventHandler<ComponentEventArgs> dispatched, VB.VBComponent component)
292+
private static void OnDispatch(EventHandler<ComponentEventArgs> dispatched, VB.VBComponent vbComponent)
283293
{
284294
var handler = dispatched;
285295
if (handler != null)
286296
{
287-
handler.Invoke(this, new ComponentEventArgs(Parent.ProjectId, Parent, new VBComponent(component)));
297+
var component = new VBComponent(vbComponent);
298+
var project = component.Collection.Parent;
299+
handler.Invoke(component, new ComponentEventArgs(project.ProjectId, project, component));
288300
}
289301
}
290302

0 commit comments

Comments
 (0)