Skip to content

Commit 25797e9

Browse files
retailcoderretailcoder
authored
Merge pull request #4268 from mansellan/3949
Fix the leaky event sink for VB6 commandbars
2 parents b374b51 + fc5ce70 commit 25797e9

File tree

15 files changed

+205
-39
lines changed

15 files changed

+205
-39
lines changed

‎Rubberduck.VBEEditor/Rubberduck.VBEditor.csproj

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@
7878
<Compile Include="Factories\ISafeComWrapperProvider.cs" />
7979
<Compile Include="Factories\VBEFactory.cs" />
8080
<Compile Include="HashCode.cs" />
81+
<Compile Include="SafeComWrappers\Abstract\IComIndexedProperty.cs" />
82+
<Compile Include="SafeComWrappers\Abstract\IEventSource.cs" />
83+
<Compile Include="SafeComWrappers\VB\Abstract\ICommandBarButtonEvents.cs" />
84+
<Compile Include="SafeComWrappers\VB\Abstract\ICommandBarEvents.cs" />
85+
<Compile Include="SafeComWrappers\VB\Abstract\IEvents.cs" />
8186
<Compile Include="SourceCodeHandling\SourceFileHandlerSourceCodeHandlerAdapter.cs" />
8287
<Compile Include="SourceCodeHandling\CodePaneSourceCodeHandler.cs" />
8388
<Compile Include="SourceCodeHandling\ISourceCodeHandler.cs" />

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
2+
{
3+
public interface IComIndexedProperty<out TItem>
4+
{
5+
TItem this[object index] { get; }
6+
}
7+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
2+
{
3+
public interface IEventSource<out TEventSource>
4+
{
5+
TEventSource EventSource { get; }
6+
}
7+
}

‎Rubberduck.VBEEditor/SafeComWrappers/SafeRedirectedEventedComWrapper.cs

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System.Diagnostics;
22
using System.Runtime.InteropServices;
33
using System.Runtime.InteropServices.ComTypes;
44
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
@@ -13,73 +13,83 @@ public abstract class SafeRedirectedEventedComWrapper<TSource, TEventSource, TEv
1313
private const int NotAdvising = 0;
1414
private readonly object _lock = new object();
1515
private TEventSource _eventSource; // The event source
16+
private TEventInterface _eventSink; // The event sink
1617
private IConnectionPoint _icp; // The connection point
17-
private int _cookie = NotAdvising; // The cookie for the connection
18+
private int _cookie = NotAdvising; // The cookie for the connection
1819

19-
protected SafeRedirectedEventedComWrapper(TSource target, TEventSource eventSource, bool rewrapping = false) : base(target, rewrapping)
20+
protected SafeRedirectedEventedComWrapper(TSource target, bool rewrapping = false)
21+
: base(target, rewrapping)
2022
{
21-
_eventSource = eventSource;
2223
}
2324

2425
protected override void Dispose(bool disposing)
2526
{
2627
DetachEvents();
27-
28-
Marshal.ReleaseComObject(_eventSource);
29-
_eventSource = null;
30-
3128
base.Dispose(disposing);
3229
}
3330

34-
public void AttachEvents()
31+
protected void AttachEvents(IEventSource<TEventSource> eventSource, TEventInterface eventSink)
3532
{
33+
Debug.Assert(eventSource != null, "Unable to attach events - eventSource is null");
34+
Debug.Assert(eventSink != null, "Unable to attach events - eventSink is null");
35+
if (eventSource == null || eventSink == null)
36+
{
37+
return;
38+
}
39+
3640
lock (_lock)
3741
{
3842
if (IsWrappingNullReference)
3943
{
4044
return;
4145
}
42-
43-
if (_cookie != NotAdvising)
44-
{
46+
47+
// Check that events not already attached
48+
if (_eventSource != null || _eventSink != null)
49+
{
4550
return;
4651
}
4752

53+
_eventSource = eventSource.EventSource;
54+
_eventSink = eventSink;
55+
4856
// Call QueryInterface for IConnectionPointContainer
49-
var icpc = (IConnectionPointContainer) _eventSource;
57+
if (!(_eventSource is IConnectionPointContainer icpc))
58+
{
59+
Debug.Assert(false, $"Unable to attach events - supplied type {_eventSource.GetType().Name} is not a connection point container.");
60+
return;
61+
}
5062

5163
// Find the connection point for the source interface
5264
var g = typeof(TEventInterface).GUID;
5365
icpc.FindConnectionPoint(ref g, out _icp);
54-
55-
var sink = this as TEventInterface;
56-
57-
if (sink == null)
58-
{
59-
throw new InvalidOperationException($"The class {this.GetType()} does not implement the required event interface {typeof(TEventInterface)}");
60-
}
61-
62-
_icp.Advise(sink, out _cookie);
66+
_icp.Advise(_eventSink, out _cookie);
6367
}
6468
}
6569

70+
public abstract void AttachEvents();
71+
6672
public void DetachEvents()
6773
{
6874
lock (_lock)
6975
{
70-
if (_icp == null)
76+
if (_icp != null)
7177
{
72-
return;
78+
if (_cookie != NotAdvising)
79+
{
80+
_icp.Unadvise(_cookie);
81+
_cookie = NotAdvising;
82+
}
83+
84+
Marshal.ReleaseComObject(_icp);
85+
_icp = null;
7386
}
7487

75-
if (_cookie != NotAdvising)
88+
if (_eventSource != null)
7689
{
77-
_icp.Unadvise(_cookie);
78-
_cookie = NotAdvising;
90+
Marshal.ReleaseComObject(_eventSource);
91+
_eventSource = null;
7992
}
80-
81-
Marshal.ReleaseComObject(_icp);
82-
_icp = null;
8393
}
8494
}
8595
}

‎Rubberduck.VBEEditor/SafeComWrappers/VB/Abstract/ICommandBarButtonEvents.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
using System;
2+
3+
namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
4+
{
5+
public interface ICommandBarButtonEvents : ISafeComWrapper, IEquatable<ICommandBarButtonEvents>
6+
{
7+
}
8+
}

‎Rubberduck.VBEEditor/SafeComWrappers/VB/Abstract/ICommandBarEvents.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
using System;
2+
3+
namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
4+
{
5+
public interface ICommandBarEvents : ISafeComWrapper, IComIndexedProperty<ICommandBarButtonEvents>, IEquatable<ICommandBarEvents>
6+
{
7+
}
8+
}

‎Rubberduck.VBEEditor/SafeComWrappers/VB/Abstract/IEvents.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
using System;
2+
3+
namespace Rubberduck.VBEditor.SafeComWrappers.Abstract
4+
{
5+
// This interface is not included on IVBE, as it is only safe in VB6
6+
// https://stackoverflow.com/questions/41055765/whats-the-difference-between-commandbarevents-click-and-commandbarbutton-click/41066408#41066408
7+
public interface IEvents : ISafeComWrapper, IEquatable<IEvents>
8+
{
9+
ICommandBarEvents CommandBarEvents { get; }
10+
}
11+
}

‎Rubberduck.VBEditor.VB6/Rubberduck.VBEditor.VB6.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@
7878
<Compile Include="SafeComWrappers\VB\CodeModule.cs" />
7979
<Compile Include="SafeComWrappers\VB\CodePane.cs" />
8080
<Compile Include="SafeComWrappers\VB\CodePanes.cs" />
81+
<Compile Include="SafeComWrappers\VB\CommandBarButtonEvents.cs" />
82+
<Compile Include="SafeComWrappers\VB\CommandBarEvents.cs" />
83+
<Compile Include="SafeComWrappers\VB\Events.cs" />
8184
<Compile Include="SafeComWrappers\VB\LinkedWindows.cs" />
8285
<Compile Include="SafeComWrappers\VB\Properties.cs" />
8386
<Compile Include="SafeComWrappers\VB\Property.cs" />

‎Rubberduck.VBEditor.VB6/SafeComWrappers/Office/CommandBarButton.cs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
extern alias Office_v8;
22
using System;
33
using System.Drawing;
4-
using System.Runtime.InteropServices;
5-
using System.Windows.Forms;
6-
using Microsoft.CSharp.RuntimeBinder;
74
using NLog;
85
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
96
using MSO = Office_v8::Office;
@@ -22,11 +19,10 @@ public class CommandBarButton : SafeRedirectedEventedComWrapper<MSO.CommandBarBu
2219
// Command bar click event is sourced from VBE.Events.CommandBarEvents[index]
2320
// where index is the command bar button COM object.
2421
public CommandBarButton(MSO.CommandBarButton target, IVBE vbe, bool rewrapping = false)
25-
: base(target, ((VB.VBE)vbe.HardReference).Events.CommandBarEvents[target], rewrapping)
22+
: base(target, rewrapping)
2623
{
2724
_control = new CommandBarControl(target, vbe, rewrapping);
2825
_vbe = vbe;
29-
3026
}
3127

3228
private MSO.CommandBarButton Button => Target;
@@ -252,8 +248,9 @@ public event EventHandler<CommandBarButtonClickEventArgs> Click
252248
lock (_eventLock)
253249
{
254250
_click += value;
255-
if (_click != null && _click.GetInvocationList().Length != 0)
251+
if (_click != null && _click.GetInvocationList().Length == 1)
256252
{
253+
// First subscriber attached - attach COM events
257254
AttachEvents();
258255
}
259256
}
@@ -265,8 +262,9 @@ public event EventHandler<CommandBarButtonClickEventArgs> Click
265262
_click -= value;
266263
if (_click == null || _click.GetInvocationList().Length == 0)
267264
{
265+
// Last subscriber detached - detach COM events
268266
DetachEvents();
269-
};
267+
}
270268
}
271269
}
272270
}
@@ -296,5 +294,17 @@ protected override void Dispose(bool disposing)
296294
base.Dispose(disposing);
297295
_control.Dispose();
298296
}
297+
298+
public override void AttachEvents()
299+
{
300+
// Cast to VB6 VBE SafeComWrapper as events are not exposed on IVBE as they are only safe to use in VB6
301+
using (var events = ((VB6.VBE)_vbe).Events)
302+
using (var commandBarEvents = events.CommandBarEvents)
303+
{
304+
// Disposal of buttonEvents is handled by the base class
305+
var buttonEvents = commandBarEvents[Target] as IEventSource<VB.CommandBarEvents>;
306+
AttachEvents(buttonEvents, this);
307+
}
308+
}
299309
}
300310
}

‎Rubberduck.VBEditor.VB6/SafeComWrappers/VB/CommandBarButtonEvents.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
2+
using VB = Microsoft.Vbe.Interop.VB6;
3+
4+
namespace Rubberduck.VBEditor.SafeComWrappers.VB6
5+
{
6+
public class CommandBarButtonEvents : SafeComWrapper<VB.CommandBarEvents>, ICommandBarButtonEvents, IEventSource<VB.CommandBarEvents>
7+
{
8+
public CommandBarButtonEvents(VB.CommandBarEvents target, bool rewrapping = false)
9+
: base(target, rewrapping)
10+
{
11+
}
12+
13+
// Explicit implementation as usage should only be from within a SafeComWrapper
14+
VB.CommandBarEvents IEventSource<VB.CommandBarEvents>.EventSource => Target;
15+
16+
public override bool Equals(ISafeComWrapper<VB.CommandBarEvents> other)
17+
{
18+
return IsEqualIfNull(other) || (other != null && ReferenceEquals(other.Target, Target));
19+
}
20+
21+
public bool Equals(ICommandBarButtonEvents other)
22+
{
23+
return Equals(other as SafeComWrapper<VB.CommandBarEvents>);
24+
}
25+
26+
public override int GetHashCode()
27+
{
28+
return IsWrappingNullReference ? 0 : Target.GetHashCode();
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)