Ensure event handlers are unsubscribed in Core Components

[Shazwazza]

Static events are pretty evil because if you don't unsubscribe from them the event handler will pin an object that should be GC'd to the root of the app. This can cause memory leaks but worse can cause really odd things to happen. I'm surprised this hasn't been an issue in our integration tests. There's a chance it is causing problems but we don't visibly see it happening. There's also a chance this is causing problems in websites in rare circumstances like overlapping appdomains but I can't be sure. What is clear is that we absolutely must unsubscribe from these events so this fixes that to ensure there isn't weird problems occuring.

This is a backport from the netcore project because this was absolutely causing problems with integration tests so it's strange we don't see these in the netframework projects.

---

This item has been added to our backlog AB#8094

[marcemarc]

@Shazwazza should we update examples in the docs? to always show unsubscribing if people subscribe to an event, eg: https://our.umbraco.com/documentation/Reference/Events/MemberService-Events/

[Shazwazza]

@marcemarc yes absolutely, this type of thing will absolutely cause problems in tests and we aren't sure yet if this is causing problems in websites but either way, it's imperative that everyone cleans up their event handlers in static events.

[marcemarc]

@Shazwazza ok done!

[warrenbuckley]

All good :)

[nul800sebastiaan]

Cherry picked for 8.7.1 in 3950f32

[ronaldbarendse]

@Shazwazza If we should always unsubscribe all static events, why doesn't Umbraco ensure this is always done by also setting the event handlers to null in the last component that runs (e.g. the CoreInitialComponent and/or WebInitialComponent)?

I've checked the amount of events on startup and after a application pool recycle and restart using ContentService.Saved.GetInvocationList().Length, but it's always reset and doesn't increase. This is probably because static fields are unique per AppDomain and are thus reset on restart...

[Shazwazza]

@ronaldbarendse because cleaning up event subscriptions is a good thing to do to make sure nothing strange happens :) We've talked about nulling out all events too as another precaution too. The reason for this PR is because we discovered some nasty leaks between threads in integration tests that were causing havoc which is down to events being static and not allowing things to be GC'd and then having those event handlers execute with stale resources. To be sure this isn't affecting anything else we unbind from events. Sure, in a web app this might not be so important but in other app types it certainly can be.