Skip to content

Commit

Permalink
Fix Repeater memoization memory leak
Browse files Browse the repository at this point in the history
If the ConditionalWeakTable references itself, it will get leaked by
.NET Core GC: dotnet/runtime#12255

Fortunately, we don't really need the ConditinalWeakTable here:
* Repeater should generally be a short-lived object, so
   strong references will also be collected reasonably soon
* Between Load and PreRender, the viewmodels were pinned
   in the DataContext of all children and the children are in Children
   collection. Only after PreRender could those references be useful,
   but we can as well Clear the dictionary at that point.

Co-authored-by: Adam Štěpánek <[email protected]>
  • Loading branch information
exyi and cafour committed Dec 1, 2024
1 parent 867508d commit 80e85bf
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/Framework/Framework/Controls/Repeater.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Linq;
using System.Runtime.CompilerServices;
using System.Collections.Generic;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Controls
{
Expand Down Expand Up @@ -139,6 +140,7 @@ protected internal override void OnLoad(IDotvvmRequestContext context)
protected internal override void OnPreRender(IDotvvmRequestContext context)
{
SetChildren(context, renderClientTemplate: !RenderOnServer, memoizeReferences: false);
childrenCache.Clear(); // not going to need the unreferenced children anymore
base.OnPreRender(context);
}

Expand Down Expand Up @@ -248,7 +250,7 @@ private DotvvmControl CreateEmptyItem(IDotvvmRequestContext context)
return emptyDataContainer;
}

private ConditionalWeakTable<object, DataItemContainer> childrenCache = new ConditionalWeakTable<object, DataItemContainer>();
private readonly Dictionary<object, DataItemContainer> childrenCache = new(ReferenceEqualityComparer<object>.Instance);
private DotvvmControl AddItem(IList<DotvvmControl> c, IDotvvmRequestContext context, object? item = null, int? index = null, bool allowMemoizationRetrieve = false, bool allowMemoizationStore = false)
{
if (allowMemoizationRetrieve && item != null && childrenCache.TryGetValue(item, out var container2) && container2.Parent == null)
Expand Down Expand Up @@ -276,8 +278,12 @@ private DotvvmControl AddItem(IList<DotvvmControl> c, IDotvvmRequestContext cont
// write it to the cache after the content is build. If it would be before that, exception could be suppressed
if (allowMemoizationStore && item != null)
{
// this GetValue call adds the value without raising exception when the value is already added...
childrenCache.GetValue(item, _ => container);
#if DotNetCore
childrenCache.TryAdd(item, container);
#else
if (!childrenCache.ContainsKey(item))
childrenCache.Add(item, container);
#endif
}

return container;
Expand Down Expand Up @@ -306,14 +312,15 @@ private void SetChildren(IDotvvmRequestContext context, bool renderClientTemplat
if (DataSource != null)
{
var index = 0;
var isCommand = context.RequestType == DotvvmRequestType.Command; // on GET request we are not initializing the Repeater twice
foreach (var item in GetIEnumerableFromDataSource()!)
{
if (SeparatorTemplate != null && index > 0)
{
AddSeparator(Children, context);
}
AddItem(Children, context, item, index,
allowMemoizationRetrieve: context.RequestType == DotvvmRequestType.Command && !memoizeReferences, // on GET request we are not initializing the Repeater twice
allowMemoizationRetrieve: isCommand && !memoizeReferences,
allowMemoizationStore: memoizeReferences
);
index++;
Expand Down
16 changes: 16 additions & 0 deletions src/Framework/Framework/Utils/ReferenceEqualityComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System.Collections.Generic;
using System.Runtime.CompilerServices;

namespace DotVVM.Framework.Utils
{
// TODO next version: Replace with System.Collections.Generic.ReferenceEqualityComparer
internal class ReferenceEqualityComparer<T> : IEqualityComparer<T>
where T : class
{
public static ReferenceEqualityComparer<T> Instance { get; } = new ReferenceEqualityComparer<T>();


public bool Equals(T? x, T? y) => ReferenceEquals(x, y);
public int GetHashCode(T obj) => obj == null ? 0 : RuntimeHelpers.GetHashCode(obj);
}
}

0 comments on commit 80e85bf

Please sign in to comment.