Event unsubscribe/resubscribe memory leak avoidance

Question

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
}

If you're running into this sort of problem, you may want to consider System.Reactive - see https://stackoverflow.com/a/29986634/1155329 for a simple usage. To kill remove a subscription, you dispose of it, which makes it easier to avoid memory leaks. You can also throw a bunch of Disposables into a CompositeDisposable, and remove many subscriptions in one line of code. — MineR, Jun 28 '18 at 06:15

TheGeneral · Accepted Answer · 2018-06-28T05:56:03.423

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.

score 1 · Answer 2 · answered Jun 28 '18 at 05:43

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

Event unsubscribe/resubscribe memory leak avoidance

2 Answers2