CODEWITHSUNDEEP
c#.netmemory-leaks
FishySwede
FishySwede

Reputation: 1583

Event unsubscribe/resubscribe memory leak avoidance

I see a lot of code similar to this in our code base:

void Method()
{
     this.someObject.SomeEvent -= someHandler;
     this.someObject.SomeEvent += someHandler;
     //... other stuff
}

Is this ever useful in any way? I keep giving the implementer the benefit of the doubt, thinking I might have missed something.

I would understand the intention to avoid memory leaks if the code was similar to:

void Method()
{
     this.someObject.SomeEvent -= someHandler;
     this.someObject = new WhateverClass();
     this.someObject.SomeEvent += someHandler;
     //... other stuff
}

Upvotes: 0

Views: 232

Answers (2)

TheGeneral
TheGeneral

Reputation: 81493

The first example is a benign attempt to stop multiple subscriptions which may cause problems (and a leak), your second example is trying to stop a memory leak

However, you could use a pattern like this which has its benefits, its not a bullet proof way to stop a leak (i.e i just helps on subtraction, that there isn't multiple subscriptions, and that you haven't subscribed more than once), but it negates this whole add/subtraction thing going on

private EventHandler _stuff;

public event EventHandler Stuff
{
   add
   {
      if (_stuff== null || !_stuff.GetInvocationList().Contains(value))
         _stuff+= value;
   }
   // ignore the resharper warning
   remove => _stuff -= value;
}

Though the truth is, if you want to use the event pattern, you should be well aware of when you need to subscribe and unsubscribe, if you don't, you have bigger problems.

Personally, i prefer Decoupled Messages / Event Aggregator (pub/sub type of thing). I rarely purposely create event plumbing these days.

Upvotes: 2

Mr.Matt
Mr.Matt

Reputation: 161

the first approach is only valid when you want to assert no duplicated subscriptions bound to the same event. If you call to Method() many times in your algorithm, and someHandler is only added and never removed from the subscribers, then, your someHandler callback will be called many times. That's the only risk.

Related to memory leaks, your reasoning is perfect. This article explain the memory leak topic. https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/events/how-to-subscribe-to-and-unsubscribe-from-events

Have a nice day

Upvotes: 1

Related Questions