Using IDisposable to unsubscribe events

Question

I have a class that handles events from a WinForms control. Based on what the user is doing, I am deferencing one instance of the class and creating a new one to handle the same event. I need to unsubscribe the old instance from the event first - easy enough. I'd like to do this in a non-proprietary manner if possible, and it seems like this is a job for IDisposable. However, most documentation recommends IDisposable only when using unmanaged resources, which does not apply here.

If I implement IDisposable and unsubscribe from the event in Dispose(), am I perverting its intention? Should I instead provide an Unsubscribe() function and call that?

Edit: Here's some dummy code that kind of shows what I'm doing (using IDisposable). My actual implementation is related to some proprietary data binding (long story).

class EventListener : IDisposable
{
    private TextBox m_textBox;

    public EventListener(TextBox textBox)
    {
        m_textBox = textBox;
        textBox.TextChanged += new EventHandler(textBox_TextChanged);
    }

    void textBox_TextChanged(object sender, EventArgs e)
    {
        // do something
    }

    public void Dispose()
    {
        m_textBox.TextChanged -= new EventHandler(textBox_TextChanged);
    }
}

class MyClass
{
    EventListener m_eventListener = null;
    TextBox m_textBox = new TextBox();

    void SetEventListener()
    {
        if (m_eventListener != null) m_eventListener.Dispose();
        m_eventListener = new EventListener(m_textBox);
    }
}

In the actual code, the "EventListener" class is more involved, and each instance is uniquely significant. I use these in a collection, and create/destroy them as the user clicks around.

Conclusion

I'm accepting gbjbaanb's answer, at least for now. I feel that the benefit of using a familiar interface outweighs any possible downside of using it where no unmanaged code is involved (how would a user of this object even know that?).

If anyone disagrees - please post/comment/edit. If a better argument can be made against IDisposable, then I'll change the accepted answer.

see the WeakEvent Pattern that might help you: http://msdn.microsoft.com/en-us/library/aa970850.aspx — gbjbaanb, Jan 17 '09 at 01:36
It is 7 years later and that link says: "We're sorry—the topic that you requested is no longer available. Use the search box to find related information." — Rhyous, Apr 29 '16 at 00:16

score 47 · Accepted Answer · answered Jan 16 '09 at 22:43

47

Yes, go for it. Although some people think IDisposable is implemented only for unmanaged resources, this is not the case - unmanaged resources just happens to be the biggest win, and most obvious reason to implement it. I think its acquired this idea because people couldn't think of any other reason to use it. Its not like a finaliser which is a performance problem and not easy for the GC to handle well.

Put any tidy-up code in your dispose method. It'll be clearer, cleaner and significantly more likely to prevent memory leaks and a damn sight easier to use correctly than trying to remember to un-do your references.

The intention of IDisposable is to make your code work better without you having to do lots of manual work. Use its power in your favour and get over some artificial "design intention" nonsense.

I remember it was difficult enough to persuade Microsoft of the usefulness of deterministic finalisation back when .NET first came out - we won the battle and persuaded them to add it (even if it was only a design pattern at the time), use it!

answered Jan 16 '09 at 22:43

gbjbaanb

51,617
12
104
148

1

I strongly disagree. You are breaking the "contract" in "design by contract" when you do that. – Domenic Jan 17 '09 at 00:05
8

what contract? Nowhere does it say "IDisposable is for unmanaged resources only". Often it says "can be used for" but that's quite a difference. – gbjbaanb Jan 17 '09 at 01:29
4

@Domenic: I agree with gbjbaanb. Even if the docs did say that IDisposable is intended only for the release unmanaged resources, you wouldn't really break any hard contract (as in preconditions, postconditions and class invariants) when you use it for other clean-up. – Jan 17 '09 at 08:24
1

Wouldn't letting go of listeners be considered cleaning up? – Benjamin Autin Aug 05 '09 at 15:30
I've pondered the same question as the OP and have sort of decided somewhere along the line to allow uses other than unmanaged resource cleanup but from what I can tell most documentation strictly directs developers to use IDisposable pattern to dispose of unmanaged or managed resources. I can't find the link but somewhere, managed resources have been described as managed objects that own resources either managed or unmanaged which also implement IDisposable. Whether divergence from these recommendations is considered dangerous or bad practice I'm not yet sure. – jpierson Apr 14 '11 at 05:02
Consider the use of SafeHandle (http://blogs.msdn.com/b/bclteam/archive/2005/03/16/396900.aspx) for an example of how MS didn't think things through well enough :) IMHO the only thing that you can leave to the GC is objects that use memory only. If it contains any other resources, managed or unmanaged, then its ok to use IDisposable to clean the object up when you want to. – gbjbaanb Apr 18 '11 at 21:30
6

The problem I find with this approach is that you end with almost all your classes implementing IDisposable. You add an event handler to a class, thus you implement IDisposable on that class. Then you must implement IDisposable on all classes using the previous class to call Dispose when ending the work with that class. Soon you find yourself with IDisposable in half of your classes which think is not the intended usage of the interface. – Ignacio Soler Garcia Jun 10 '12 at 09:14
Not sure if this could be a solution but what about the Weak Event pattern? – Ignacio Soler Garcia Jun 10 '12 at 09:17
@SoMoS: The only thing that should call `Dispose` on an object is the last entity to have responsibility for it. If an entity which is given a reference to an object will expect to use it longer than the entity which is presently responsible for it, the two entities must agree to have the recipient assume responsibility. Otherwise no transfer or responsibility (and no `Dispose`) is necessary. Also, if an object subscribes to event notifications but doesn't implement `IDisposable`, how is the publisher supposed to know that notifications are no longer required? – supercat Aug 25 '13 at 19:20
I think that another reason to implement IDisposable is having the possibility to use your object inside the 'using' statement: using (var vm = new DisposableViewModel()){ ... } – 3615 Feb 10 '17 at 11:47

score 15 · Answer 2 · answered Jan 16 '09 at 22:34

15

My personal vote would be to have an Unsubscribe method in order to remove the class from events. IDisposable is a pattern intended for deterministic release of unmanaged resources. In this case you not managing any unmanaged resources and therefore should not be implementing IDisposable.

IDisposable can be used to manage event subscriptions but probably shouldn't. For an example I point you to WPF. This is a library rife with events and event handlers. Yet virtually no class in WPF implements IDisposable. I would take that as an indication that events should be managed another way.

answered Jan 16 '09 at 22:34

JaredPar

733,204
149
1,241
1,454

7

WPF has hardly any IDisposable controls because it uses the WeakEvent Pattern to get round leaks: http://msdn.microsoft.com/en-us/library/aa970850.aspx – gbjbaanb Jan 17 '09 at 01:34
2

@gbjbaanb - Right thats what I understood as well although this probably backs up what JaredPar's point was about "an indication that events should be managed another way". One of those ways I suppose could be the WeakEvent Pattern, another would be a custom IUnsubscribable interface for example that could mimic how IDisposable is used. – jpierson Apr 14 '11 at 05:07
@jpierson having a IUnsubscribable interface has a drawback: it would make it impossible to write something like using(var vm = new UnsibscribableViewModel()){ ... }. – 3615 Feb 10 '17 at 11:50
`WeakEventPattern` is **the** way to solve the OP's issue. It has been designed for that very purpose in 3.0 RT. – bokibeg Jun 01 '17 at 15:33
This is way too dogmatic. Why not use IDisposable? You can implement your Unsubscribe method, but it won't get called at the end of a Using statement, or by an IOC container that injected the object, whereas Dispose will. If you need to clean up use IDisposable. – Mick Feb 22 '19 at 02:26

score 8 · Answer 3 · edited Feb 06 '17 at 13:13

8

One thing that bothers me about using IDisposable pattern for unsubscribe from events is Finalization issue.

Dispose() function in IDisposable is supposed to be called by the developer, HOWEVER, if it isn't called by the developer, it is a understood that the GC will call this function (by the standard IDisposable pattern, at least). In your case, however, if you don't call Dispose no one else will - The event remains and the strong reference holds GC from calling the finalizer.

The mere fact that Dispose() won't be called automatically by GC seems to me enough to not to use IDisposable in this case. Perhaps it calls for a new application specific interface that says that this type of object must have a Cleanup function called to be disposed by GC.

edited Feb 06 '17 at 13:13

nawfal

70,104
56
326
368

answered Oct 26 '10 at 10:16

VitalyB

12,397
9
72
94

@Jason Coyne: I would join with Jason Coyne in arguing just the opposite: iDisposable is for things which cannot be satisfactorily handled via garbage collection alone. Indeed, my interpretation of the iDisposable contract would be: the /only/ objects which /shouldn't/ implement iDisposable are those for which GC-based cleanup is sufficient, or possibly those which can't be cleaned up with a single method call. To my mind, the /non-implementation/ of iDisposable makes a stronger statement than the implementation of it. – supercat Nov 01 '10 at 18:34
2

After a while with my code, I tend to agree with you. However, I am still concerned about the ambiguity of IDisposable and wish Microsoft handled the issue better. – VitalyB Nov 02 '10 at 08:08
1

I totally agree with you. IDisposable was implemented from the very beginning to handle COM interop and integration with unmanaged resources. Having provided a solution that guarantees no memory leaks is good, but as you noted, if you're using Dispose() to unsubscribe from events and you don't call the method directly in code (i.e. a "using" statement or otherwise), then a strong reference is held and the object never gets GC'ed. It's a pain in the rear, and definitely something to bring up. – Doug Nov 20 '10 at 05:15
@Doug: The fact that failing to clean up event handlers will cause memory leaks is annoying, but it makes far more sense to say "when possible, it should be possible to clean up any object by try-casting to `IDisposable` and, if that succeeds, calling `Dispose` on it", than to say "objects which can't clean themselves automatically when a cleanup method isn't called shouldn't implement `IDisposable`, especially since using `IDisposable,Dispose` as one's cleanup method greatly increases the likelihood that it will be called. – supercat Aug 03 '12 at 17:37
@VitalyB you can fix this by using a weak reference instead of a strong reference of course. In which case GC can call the finalizer. – Tim Lovell-Smith Aug 20 '13 at 20:25
1

@supercat Using IDisposable to dispose of your object's finalizable resources now instead of letting the GC do it later is a valid use of IDisposable, and in fact an optimization because it frees up space on the finalizer queue when you call SuppressFinalize(), and of course it frees up whatever resources your finalizer frees up. That's why the 'standard' example for IDisposable does this. – Tim Lovell-Smith Aug 20 '13 at 20:27
1

@TimLovell-Smith: It's fine for objects that can do an okay job of cleaning up automatically to offer `IDisposable` so code can do even better. I read the original answer (esp. last paragraph), however, as claiming that the things which can't clean themselves automatically shouldn't implement `IDisposable` because callers might regard the fact that a class implements `IDisposable` as a sign that the class will be able to clean up after itself automatically without the consumer having to call a cleanup method like `Dispose`. – supercat Aug 20 '13 at 20:43

score 5 · Answer 4 · answered Aug 05 '09 at 15:16

I think disposable is for anything that GC can't take care of automatically, and event references count in my book. Here is a helper class I came up with.

public class DisposableEvent<T> : IDisposable
    {

        EventHandler<EventArgs<T>> Target { get; set; }
        public T Args { get; set; }
        bool fired = false;

        public DisposableEvent(EventHandler<EventArgs<T>> target)
        {
            Target = target;
            Target += new EventHandler<EventArgs<T>>(subscriber);
        }

        public bool Wait(int howLongSeconds)
        {
            DateTime start = DateTime.Now;
            while (!fired && (DateTime.Now - start).TotalSeconds < howLongSeconds)
            {
                Thread.Sleep(100);
            }
            return fired;
        }

        void subscriber(object sender, EventArgs<T> e)
        {
            Args = e.Value;
            fired = true;
        }

        public void Dispose()
        {
            Target -= subscriber;
            Target = null;
        }

    }

which lets you write this code :

Class1 class1 = new Class1();
            using (var x = new DisposableEvent<object>(class1.Test))
            {
                if (x.Wait(30))
                {
                    var result = x.Args;
                }
            }

One side effect, you must not use the event keyword on your events, since that prevents passing them as a parameter to the helper constructor, however, that seems to not have any ill effects.

Disposable pattern isn't really because the GC can't do it automatically, disposable pattern is because it makes sense to support the scenario of 'do cleanup as soon as possible', in a using block. Finalizer is required for cleaning up resources the GC doesn't know how to clean up and therefore can't do automatically. — Tim Lovell-Smith, Aug 20 '13 at 20:29

score 5 · Answer 5 · answered Jan 17 '09 at 08:49

5

Another option would be to use weak delegates or something like WPFs weak events, instead of having to unsubscribe explicitly.

P.S. [OT] I consider the decision to only provide strong delegates the single most expensive design mistake of the .NET platform.

answered Jan 17 '09 at 08:49

Not sure about "most expensive", but it's up there. I'm puzzled as to why Microsoft has yet to offer a `WeakDelegate` type, and have `Delegate.Combine` and `Delegate.Remove` omit from the resulting delegate list any weak delegates that have expired. Publisher-side weak events aren't a proper solution, since it's the subscriber rather than the publisher who knows whether a subscription should keep an object alive. – supercat Sep 27 '12 at 21:31

Sebastian Gregor · Answer 6 · 2011-01-05T01:36:40.393

From all that i read about disposables i would argue that they were really mainly invented for solving one problem: freeing unmanaged system resources in a timely manner. But still all examples that i found are not only focussed on the topic of unmanaged resources, but also have another property in common: Dispose is called just to speed up a process that otherwise would have occured later on automatically (GC -> finalizer -> dispose)

Calling a dispose method that unsubscribes from an event however would never occur automatically, even if you would add a finalizer that would call your dispose. (at least not as long as the event owning object exists - and if it would be called you wouldn't benefit from the unsubscription, since the event owning object would also be gone anyway)

So the main difference is that events somehow build an object graph that can't be collected, since the event handling object suddenly becomes referenced of the service that you just wanted to reference/use. You suddenly are forced to call Dispose - no automatic disposal is possible. Dispose would thereby get a subtle other meaning than that found in all examples where a Dispose call - in dirty theory ;) - isn't necessary, since it would get called automaically (at some time)...

Anyway. Since the disposable pattern is something that already is pretty complicated (deals with finalizers that are hard to get right and many guidelines / contracts) and more importantly in most points has nothing to do with the event back referencing topic, i would say it would be easier to get that separeted in our heads by just not using that metaphor for something that could be called "unroot from object graph" / "stop" / "turn off".

What we want to achieve is to disable / stop some behaviour (by unsubscribing from an event). It would be nice to have a standard interface like IStoppable with a Stop() method, that by contract is just focussed on

getting the object (+ all its own stoppables) disconnected from events of any objects that it didn't create by its own
so that it won't get called in implicit event style manner any longer (therefore can be perceived as stopped)
can be collected as soon any traditional references onto that object are gone

Let's call the only interface method that does the unsubscription "Stop()". You would know that the stopped object is in a acceptable state but only is stopped. Maybe a simple property "Stopped" would also be a nice to have.

It even would make sense to have an interface "IRestartable" that inherits from IStoppable and additionally has a method "Restart()" if you just want to pause a certain behaviour that certainly will be needed again in the future, or to store a deleted model object in a history for later undo recovery.

After all writing i have to confess to have just seen an example of IDisposable somewhere over here: http://msdn.microsoft.com/en-us/library/dd783449%28v=VS.100%29.aspx But anyway until i get every detail and the original motivation of IObservable i would say it is not the best use case example

since again it is a pretty complicated system around it and we only have a small problem over here
and it might be that one of the motivations of that that whole new system is to get rid of events in the first place, which would result in a sort of stack overflow concerning th e original question

But it seems they are on some right track. Anyway: they should have used my interface "IStoppable" ;) since i strongly believe there is a difference in

Dispose: "you should call that method or something might leak if the GC happens to late" ....

and

Stop: "you have to call this method to stop a certain behaviour"

I disagree. There is a strong convention that if an object requires cleanup, its owner can clean it up by trying to cast to IDisposable and, if that works, calling Dispose. One shouldn't have to guess whether an object might require some other type of cleanup. While it's true that many disposable objects will manage to clean up for themselves if abandoned, the implication that an object which implements IDisposable will clean up for itself when abandoned is far weaker than the implication that an object which *doesn't* implement IDisposable will do so. — supercat, May 04 '11 at 22:12
Incidentally, I think the MS default IDisposable pattern is silly (the only objects with cleanup finalizers should be those whose purpose is to encapsulate a single cleanup responsibility; if a class does something other than handle a single cleanup responsibility, then by definition any derived classes will as well, and thus derived classes should not have finalizers for cleanup (they may have finalizers generate "Object wrongfully abandoned" log entries). I would also note that the name "Dispose" is a misnomer, since the purpose isn't to dispose of an object, but rather... — supercat, May 04 '11 at 22:15
...to allow it to carry out any responsibilities it has (typically the cleanup of other objects) before it is abandoned. — supercat, May 04 '11 at 22:16

score 3 · Answer 7 · answered Jan 16 '09 at 22:39

3

IDisposable is firmly about resources, and the source of enough problems to not muddy the waters further I think.

I'm voting for an Unsubscribe method on your own Interface too.

answered Jan 16 '09 at 22:39

annakata

74,572
17
113
180

score 3 · Answer 8 · answered Jan 16 '09 at 23:20

3

One option may be not to unsubscribe at all - just to change what the subscription means. If the event handler could be made smart enough to know what it's meant to do based on the context, you don't need to unsubscribe in the first place.

That may or may not be a good idea in your particular case - I don't think we've really got enough information - but it's worth considering.

answered Jan 16 '09 at 23:20

Jon Skeet

1,421,763
867
9,128
9,194

3

In my case I think it means the object would live forever and I'd have a memory leak, unfortunately. – Jon B Jan 16 '09 at 23:43
@Jon B - I think this is was happens with most non-UI events. +1 – Doug Nov 20 '10 at 05:17
Just in case John's explanation wasn't clear I think what he is suggesting is that in some cases like this objects can be recycled instead of thrown away for a new instance. This allows the existing event subscriptions to be used without having to detach. Think of it like a thread pool or connection pool for your objects where there are a pool of possible ones that can be recycled. It's not a solution for all situations but probably in more than cases than not if you just change how your thinking about it. – jpierson Apr 14 '11 at 05:23

score 1 · Answer 9 · edited Jul 10 '18 at 10:45

No, you're not preventing the intention of IDisposable. IDisposable is intended as an all-purpose way to ensure that when you're done using an object, you can proactively clean up everything tied to that object. It doesn't have to be only unmanaged resources, it can include managed resources too. And an event subscription is just another managed resource!

A similar scenario that frequently arises in practice is that you will implement IDisposable on your type, purely in order to ensure you can call Dispose() on another managed object. This is not a perversion either, it is just tidy resource management!

Using IDisposable to unsubscribe events

9 Answers9

Linked

Related