[Android] Fix NRE in packager dispose #561

Merged
merged 1 commit into from Dec 1, 2016

Conversation

Projects
None yet
5 participants
@adrianknight89
Contributor

adrianknight89 commented Nov 24, 2016

Description of Change

When VisualElementPackager is disposing itself, it doesn't check if its element is null. You could run the attached repro in the bug description and see that the user is nullifying Element and then disposing the packager.

protected override void Dispose(bool disposing)
{
	if (disposing)
	{
		this.SetElement((VisualElement) null);
		if (this.Packager != null) {

			// exception
			this.Packager.Dispose ();
			this.Packager = null;
		}

		this.Tracker.Dispose();
		this.Tracker = (VisualElementTracker) null;
	}

	base.Dispose(disposing);
}

This change is in line with the UAP counterpart. iOS side doesn't use Element after setting it to null.

P.S. I'm not sure why iOS seems to be the only platform that sets Element to null. Was this on purpose?

Bugs Fixed

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@hartez

This comment has been minimized.

Show comment
Hide comment
@hartez

hartez Dec 1, 2016

Member

This doesn't really fix the underlying problem in 39228; the problem there was the order in which they cleaned up resources. But the null check does help prevent crashes in future custom renderers, so I'm happy to add it.

Member

hartez commented Dec 1, 2016

This doesn't really fix the underlying problem in 39228; the problem there was the order in which they cleaned up resources. But the null check does help prevent crashes in future custom renderers, so I'm happy to add it.

@hartez hartez merged commit 26d5b2b into xamarin:master Dec 1, 2016

3 of 7 checks passed

Android-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Snapshot dependency …
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: OSX Debug : Snapshot dependency failed: ... Windows Debug
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: Windows Debug : Unable to find runner parameter nunit_path required to run th…
Details
iOS9-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Snapshot depende…
Details
Android-UITests-C9 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 349, i…
Details
OSX-Debug-C9 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: OSX Debug : Running
Details
Windows-Debug-C9 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 9 :: Windows Debug : Tests passed: 3585, ignored: 10
Details

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment