Fix ObjectDisposedException issues for renderers OnElementPropertyChanged

[hartez]

Description of Change

This issue occurs under a very specific set of circumstances:

• A Forms element has a bound property
• The element's renderer has been created
• The element and the renderer are not attached to the view hierarchy
• The only reference to the element is a weak reference via the binding
• The garbage collector has just run and determined that the renderer is collectible
• The source of the binding for the element's bound property updates

For garbage collection to occur, the link between the managed and native objects referenced by the renderer needs to be severed. So the object's Handle is set to IntPtr.Zero. However, before the actual collection can occur, the source property changes.

When the source property changes, the weak reference to the element with the bound property is checked, and that creates a strong reference. The property changed handler fires, which ultimately causes the renderer's OnElementPropertyChanged method to run. However, at this point the renderer's native object is no longer eligible for anything; any attempt to reference it throws the ObjectDisposedException.

This change adds a check for a zeroed Handle before running the rest of the OnElementPropertyChanged method, which avoids the exception. The element and renderer can later be succesfully collected by the GC.

The element and the renderer are not attached to the view hierarchy

This circumstance is rare. It's trivial to recreate it artificially (see the unit tests included with this PR), but in practice it mostly happens when properties are bound to a (relatively) long-lived source; see issue #9431 for an example where CollectionView items have properies bound to the Page itself.

Issues Resolved

• Fixes [Bug] ObjectDisposedException (BoxView inside CollectionView) #9431
• Fixes [Android] Disposed EntryCell throws ObjectDisposed exception after updating an object that the EntryCell was previously bound to #5560
• Fixes Cannot access a disposed object. Object name: 'Android.Widget.TextView'. #8607
• Fixes Android Crash - ObjectDisposedException Rendering Shadow with FramerRenderer #6587
• Fixes [Bug] Unhandled Environment exception. - System.ObjectDisposedException: Cannot access a disposed object. #9412
• Fixes [Bug] System.ObjectDisposedException: Cannot access a disposed object. Object name: 'Xamarin.Forms.Platform.Android.FastRenderers.LabelRenderer'. or JniPeerMembers.AssertSelf (Java.Interop.IJavaPeerable self) #9845

API Changes

None

Platforms Affected

• Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Automated tests.

PR Checklist

• Targets the correct branch
• Tests are passing (or failures are unrelated)

[hartez]

Curses! Forgot to deal with the legacy renderers....

[hartez]

Okay, added checks for the old renderers.

[PureWeen]

So, I know when chatting you had mentioned you wanted to make this method call as light as possible because how often PropertyChanged is called.

That being said then should we try only calling this for relevant properties?

I realize the code for that is a bit heavy but it would let us check for a few more things without the performance cost. Maybe the method calls at each line could use an action of sorts to get called

CallPropertyChangedMethod(UpdateMainText, this);

And then CallPropertyChangedMethod would check this to see if it should still call UpdateMainText and then at that point the check and any additional behavior could be generalized.

[PureWeen]

All the IsDisposed checks should also be added to the tracker as well.

[hartez]

All the IsDisposed checks should also be added to the tracker as well.

Done.