# Another way to implement weak event handlers in vb.net

Summary: Please let me know, what weaknesses do you see in the described workaround for event handlers preventing garbage collection of registered listener objects...

I'm in the development of and API for a set of devices communicating with each other, and optionally with a PC trough a socket based gateway. The API describes the multitude of different devices, channels as a class hierarchy, and also implements some kind of caching for the derived objects.

I'd like to make the API really simple to use for the developers, so I keep the interfaces as simple as possible: hiding background threading, etc... I ran into the already described problems, when registering event handlers from a windows form interface:

1. The event source object is in the cache, it keeps the reference to the target object, preventing it from garbage collection. It's quite problematic, because the UI creates and destroys controls (providing some kind of user interface for the device objects) on the fly... for example when a new device is discovered, or when view is changed.

2. As the event raiser might be a different thread than the UI, so it requires checking in each of the event handler routines.

I was reading the WeakEvent pattern guidelines - I'm not too happy with that solution: - It's not "standard" - so the programmer has to register for events "manually" - And quite complex to build.

I ended up with Custom Event declarations, like:

Private EHL_Received As New PacketEventHandlerList("PacketReceived")
RemoveHandler(ByVal value As EventHandler)
End RemoveHandler
RaiseEvent(ByVal sender As Object, ByVal e As PacketEventArgs)
End RaiseEvent
End Event


Then, in the "event handler" list, instead of storing real EventHandlers, I collect custom objects: they keep weak references to the targets.

Friend Class PacketEventHandler

Private targetRef As WeakReference = Nothing
Private IsStatic As Boolean = False
Friend IsValid As Boolean = True
Friend Method As System.Reflection.MethodInfo

Public Sub New(ByVal h As EventHandler)
IsStatic = IsNothing(h.Target)
If Not IsStatic Then targetRef = New WeakReference(h.Target)
Me.Method = h.Method
End Sub

Get
If Not IsNothing(targetRef) Then Return targetRef.Target
Return Nothing
End Get
End Property

Public Sub Invoke(ByVal sender As Object, ByVal e As EventArgs)

' If method is static, there is not too much to do
If IsStatic Then
Me.Method.Invoke(Nothing, {sender, e})
Exit Sub
End If

' If the reference is invalid, then the object was probably collectd. Mark handler as invalid
Dim MyTarget As Object = Me.Target
If IsNothing(MyTarget) Then
Me.IsValid = False
Exit Sub
Else
' As we are in a different thread, do the invoke stuff if needed
If TypeOf (MyTarget) Is Windows.Forms.Control Then
Dim ctlTarget As Windows.Forms.Control = MyTarget
If ctlTarget.InvokeRequired Then
ctlTarget.Invoke(New System.Windows.Forms.MethodInvoker(Sub() Me.Invoke(sender, e)))
MyTarget = Nothing
Exit Sub
End If
End If

' For other objects it's simple
Me.Method.Invoke(MyTarget, {sender, e})
MyTarget = Nothing
End If
End Sub
End Class


Note, that each handler object holds reference to one single handler, so a list is created.

Friend Class PacketEventHandlerList
Private Handlers As New List(Of PacketEventHandler)
Private Name As String

Public Sub New(ByVal Name As String)
' Just keep the name for logging purposes
Me.Name = Name
End Sub

Friend Sub Add(ByVal value As EventHandler)
Console.WriteLine("{0} registers {1}", value.Target.ToString, Me.Name)
End Sub

Friend Sub Remove(ByVal value As EventHandler)
Console.WriteLine("{0} unregisters {1}", value.Target.ToString, Me.Name)
For I As Integer = Handlers.Count - 1 To 0 Step -1
Dim MyEvh As PacketEventHandler = Handlers(I)
If Object.ReferenceEquals(MyEvh.Target, value.Target) And value.Method.Equals(value.Method) Then
Handlers.RemoveAt(I)
Console.WriteLine("Successful remove!")
End If
Next
End Sub

Friend Sub Invoke(ByVal sender As Object, ByVal e As EventArgs)
Dim InvalidHandlers As Integer = 0

' Go trough all registered handlers, and invoke them
For Each EVH As PacketEventHandler In Handlers
Try
EVH.Invoke(sender, e)
Catch ex As Exception
LogException(ex)
End Try
If Not EVH.IsValid Then InvalidHandlers += 1
Next

' If there were invalid handlers, just clean them up
If InvalidHandlers > 0 Then
For I As Integer = Handlers.Count - 1 To 0 Step -1
If Not Handlers(I).IsValid Then Handlers.RemoveAt(I)
Next
End If
End Sub

End Class


And, it seems to work.

Minor issue is, that I could't check the RemoveHandler part, as it's not called for some reasons. I'm not sure if I need that at all...

Do you see any problems with this implementation?

## migrated from programmers.stackexchange.comFeb 8 '12 at 11:26

This question came from our site for professionals, academics, and students working within the systems development life cycle.

• This may be more suitable on StackOverflow (or CodeReview?). Please don't cross-post it though - it will get automigrated if enough people agree with this and votes to close here. – Péter Török Feb 8 '12 at 10:51
• The solution proposed seems similar to Udi Dahan's Domain Events pattern: udidahan.com/2009/06/14/domain-events-salvation personally I think Udi's solution is easier to test and doesn't have any magic strings like "PacketReceived" and doesn't require any reflection. – Kane Jun 22 '12 at 1:43

At first glance, PacketEventHandler is a rather confusing name for a class, but then looking at it more thoroughly I see that it's actually a wrapper around an event handler, so I see what made you call it that way, but I'd still try harder to find a better name... although I have none to suggest right now (naming is one of the hardest things to do properly!).

The way you're clearing your InvalidHandlers looks awkward.

' If there were invalid handlers, just clean them up
If InvalidHandlers > 0 Then
For I As Integer = Handlers.Count - 1 To 0 Step -1
If Not Handlers(I).IsValid Then Handlers.RemoveAt(I)
Next
End If


Handlers being a List(Of PacketEventHandler), I'm pretty sure you could simply do this:

Dim Handler As PacketEventHandler
If InvalidHandlers > 0 Then
For Each Handler In Handlers
If Not Handler.IsValid Then Handlers.Remove(Handler)
Next
End If


And reviewing this snippet made me wonder, are you using Option Explicit? I don't see your I declared anywhere - always use Option Explicit. The C# programmer in me would also advise you to use Option Strict just to be on a [type-]safe side (you don't have to specify explicit if you specify strict - the latter implies the former).

This is by no means a full review - but it's all I could see on a first pass. The rest roughly looks good to me.