From 07e31c63183b63970aa06f52e3440c50fc0c0f8b Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 9 Jan 2018 15:08:27 +0000 Subject: [PATCH] Unit tests relating to Renderer GC behavior --- src/Microsoft.Blazor/Rendering/Renderer.cs | 4 + .../RenderTreeBuilderTest.cs | 9 ++ test/Microsoft.Blazor.Test/RendererTest.cs | 123 ++++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 test/Microsoft.Blazor.Test/RendererTest.cs diff --git a/src/Microsoft.Blazor/Rendering/Renderer.cs b/src/Microsoft.Blazor/Rendering/Renderer.cs index bb51ff4398..436c7d3f4e 100644 --- a/src/Microsoft.Blazor/Rendering/Renderer.cs +++ b/src/Microsoft.Blazor/Rendering/Renderer.cs @@ -4,6 +4,7 @@ using Microsoft.Blazor.Components; using Microsoft.Blazor.RenderTree; using System; +using System.Runtime.CompilerServices; namespace Microsoft.Blazor.Rendering { @@ -19,6 +20,8 @@ namespace Microsoft.Blazor.Rendering // these reference descendant components and associated ComponentState instances. private readonly WeakValueDictionary _componentStateById = new WeakValueDictionary(); + private readonly ConditionalWeakTable _componentStateByComponent + = new ConditionalWeakTable(); private int _nextComponentId = 0; // TODO: change to 'long' when Mono .NET->JS interop supports it /// @@ -34,6 +37,7 @@ namespace Microsoft.Blazor.Rendering var componentId = _nextComponentId++; var componentState = new ComponentState(this, componentId, component); _componentStateById.Add(componentId, componentState); + _componentStateByComponent.Add(component, componentState); // Ensure the componentState lives for at least as long as the component return componentId; } } diff --git a/test/Microsoft.Blazor.Test/RenderTreeBuilderTest.cs b/test/Microsoft.Blazor.Test/RenderTreeBuilderTest.cs index 1243462771..34872f68e7 100644 --- a/test/Microsoft.Blazor.Test/RenderTreeBuilderTest.cs +++ b/test/Microsoft.Blazor.Test/RenderTreeBuilderTest.cs @@ -12,6 +12,15 @@ namespace Microsoft.Blazor.Test { public class RenderTreeBuilderTest { + [Fact] + public void RequiresNonnullRenderer() + { + Assert.Throws(() => + { + new RenderTreeBuilder(null); + }); + } + [Fact] public void StartsEmpty() { diff --git a/test/Microsoft.Blazor.Test/RendererTest.cs b/test/Microsoft.Blazor.Test/RendererTest.cs new file mode 100644 index 0000000000..bb428640d7 --- /dev/null +++ b/test/Microsoft.Blazor.Test/RendererTest.cs @@ -0,0 +1,123 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.Blazor.Components; +using Microsoft.Blazor.Rendering; +using Microsoft.Blazor.RenderTree; +using Xunit; + +namespace Microsoft.Blazor.Test +{ + public class RendererTest + { + [Fact] + public void ComponentsAreNotPinnedInMemory() + { + // It's important that the Renderer base class does not itself pin in memory + // any of the component instances that were attached to it (or by extension, + // their descendants). This is because as the set of active components changes + // over time, we need the GC to be able to release unused ones, and there isn't + // any other mechanism for explicitly destroying components when they stop + // being referenced. + var renderer = new NoOpRenderer(); + + AssertCanBeCollected(() => + { + var component = new TestComponent(null); + renderer.AssignComponentId(component); + return component; + }); + } + + [Fact] + public void CannotRenderComponentsIfGCed() + { + // Arrange + var renderer = new NoOpRenderer(); + + // Act + var componentId = new Func(() => + { + var component = new TestComponent(builder => + throw new NotImplementedException("Should not be invoked")); + + return renderer.AssignComponentId(component); + })(); + + // Since there are no unit test references to 'component' here, the GC + // should be able to collect it + GC.Collect(); + GC.WaitForPendingFinalizers(); + + // Assert + Assert.ThrowsAny(() => + { + renderer.RenderComponent(componentId); + }); + } + + [Fact] + public void CanRenderComponentsIfNotGCed() + { + // Arrange + var renderer = new NoOpRenderer(); + var didRender = false; + + // Act + var component = new TestComponent(builder => + { + didRender = true; + }); + var componentId = renderer.AssignComponentId(component); + + // Unlike the preceding test, we still have a reference to the component + // instance on the stack here, so the following should not cause it to + // be collected. Then when we call RenderComponent, there should be no error. + GC.Collect(); + GC.WaitForPendingFinalizers(); + + renderer.RenderComponent(componentId); + + // Assert + Assert.True(didRender); + } + + private class NoOpRenderer : Renderer + { + public new int AssignComponentId(IComponent component) + => base.AssignComponentId(component); + + public new void RenderComponent(int componentId) + => base.RenderComponent(componentId); + + protected override void UpdateDisplay(int componentId, ArraySegment renderTree) + { + } + } + + private class TestComponent : IComponent + { + private Action _renderAction; + + public TestComponent(Action renderAction) + { + _renderAction = renderAction; + } + + public void BuildRenderTree(RenderTreeBuilder builder) + => _renderAction(builder); + } + + void AssertCanBeCollected(Func targetFactory) + { + // We have to construct the WeakReference in a separate scope + // otherwise its target won't be collected on this GC cycle + var weakRef = new Func( + () => new WeakReference(targetFactory()))(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + Assert.Null(weakRef.Target); + } + } +}