2

I am using weak events when I can't deterministically unsubscribe (otherwise I would prefer += and -= instead of weak event):

class SomeType
{
    public SomeType(...)
    {
        // object doesn't know when it will be removed
        WeakEventManager(SomeSource, EventArgs).AddHandler(someSourceInstance,
            nameof(SomeSource.SomeEvent), (s, e) => { ... });
    }
 }

This way if object is garbage collected, then event handler will not be called. Perfect.

However. If object is not yet garbage collected (but there are no more strong references), then event handler will still be called.

My question is rather general: what should I do when using weak events? Should I expect invalid call in event handler when using weak events? Or should I force GC to avoid that case (kind of deterministic "clean up")? Something else?

Community
  • 1
  • 1
Sinatr
  • 20,892
  • 15
  • 90
  • 319
  • 1
    You've got it backwards -- weak events allow subscribers to be garbage collected if they would only be kept alive through event handlers. They're not there to get rid of the the event handlers just because the subscriber is eligible for GC, that's a side effect. You can call that an "invalid call", but it's not. So the short answer is "yes, you should expect such calls"; the longer is "what are you even doing that this an issue, and are you missing code that can make your intentions explicit"? – Jeroen Mostert Apr 21 '16 at 10:37
  • @JeroenMostert, I had memory leakage because of missing `-=`. It wasn't *easy* (I didn't say *impossible*) to provide unsubscribing, therefore I tried to use weak events. They solve memory leakage problem perfectly, but another problem arise... therefore my question. How should I make intention "this object required this method to be called" clear? `IDisposable`? – Sinatr Apr 21 '16 at 11:31
  • 1
    `IDisposable` is one way, yes. It's the main mechanism C# has for deterministic cleanup. While technically only intended for use in releasing unmanaged resources, it's widely used in frameworks for deterministic cleanup even when no unmanaged resources are involved. It has the benefit of language support (`using`) and clearly signalling to the developer that this object should be released explicitly. However, even better than `IDisposable` is figuring out the rules of ownership for your objects, and making the owners responsible to "shut down" the object, if at all possible. – Jeroen Mostert Apr 21 '16 at 11:37
  • @JeroenMostert, that's the problem, there are no rules. Object will be used as item in some list, then removed. I am not happy with `IDisposable`, because the object which creates and maintain list can also be simply deleted. It can in turn also implement `IDisposable`, but that sounds like everything should implement it *just in case* there might be a requirement to clean up. – Sinatr Apr 21 '16 at 11:44
  • 2
    Yes, that's how "ownership" works -- if the level directly above can't lay down the law, it will have to kick the buck upwards, and so on and so forth. You can't make garbage collection do the heavy lifting for you -- it's designed to conserve memory (where necessary!), not implement a lifetime system that you can piggyback on. In particular, if properly written, your application should run correctly even if there was no garbage collection because we have (say) terabytes of memory and never need to free anything. If it doesn't, you've got a design error. – Jeroen Mostert Apr 21 '16 at 11:48
  • 1
    Why does it matter if the event handler runs? If you can make it "not matter", your problem is solved. If that's not possible, you've got an implicit functional requirement somewhere that you're not expressing in code. – Jeroen Mostert Apr 21 '16 at 11:49
  • That was a good point against GC abuse with terabytes, thanks. Still, it's an interesting topic. I can't find answer (and don't want to keep asking you), therefore I asked [another question](http://stackoverflow.com/q/36768867/1997232). – Sinatr Apr 21 '16 at 11:59

1 Answers1

3

You should always expect an event handler may be called after you're unregistered, even with "strong" events. There's nothing invalid about such a call.

The simplest scenario is obvious when you look at how event handlers are executed:

protected void OnMyEvent(object sender, EventArgs e)
{
  var ev = MyEvent;
  if (ev != null) ev(this, EventArgs.Empty);
}

If a delegate is unregistered between ev = MyEvent and ev.Invoke, it will still receive the notification at the earliest opportunity. Have I mentioned that concurrent programming is hard yet?

But in your case, the problem really is "why doesn't the object know when to unregister?" Answer that, and you'll have your solution. Why would invoking an event handler that targets an object that is no longer strong referenced anywhere be an illegal operation? It's not like the object is partially collected or anything - it just wasn't collected yet.

Luaan
  • 62,244
  • 7
  • 97
  • 116
  • There is no pattern to use. `IDisposable`? To only unsubscribe? In `C++` there are destructors you rely on. I can make a method `CallMeWhenYouDontNeedInstance()`, but I am not sure (nor I can ensure a user of such object will call it). This was the main point of using weak events: to avoid such complications. – Sinatr Apr 21 '16 at 10:44
  • @Sinatr You can't rely on destructors in C++ any more than you can rely on someone calling your cleanup method, be it `Dispose` (which would be wrong, pedantically speaking) or not. In almost all cases, objects should have clear ownership, and when the owner is done, it should dispose of the object they own. I understand what you're trying to do and why, but generally speaking, this kind of pattern is hard to follow and will be prone to errors no matter how many hacks you add ("Look ma! I'm unregistering an event in a finalizer!"). If you have an option to use deterministic cleanup, do it. – Luaan Apr 21 '16 at 10:53
  • If event source lives longer than the object, then indeed you can get handler called, even if object is *about to be disposed* (unless you unsubscribe prior making it eligible for GC). I haven't thought what my normal event handler also had a chance to have invalid state. – Sinatr Apr 21 '16 at 12:12
  • @Sinatr It can't be eligible for GC before you unsubscribe, unless the event source is also eligible for GC (which means there's no point in unsubscribing). It's actually possible for the event source to be collected before both the subscribed object and the notification (e.g. when the notification is posted on a synchronization context). Really, concurrent code is hard. There's no such state as "about to be collected" with the exception of finalizer code (which shouldn't deal with managed resources at all) - if you have a reference, you're not GCable. This holds even with weak references. – Luaan Apr 21 '16 at 13:04