Skip to content

Commit

Permalink
fix(DependencyObject): Fix possible infinite loop on ancestor lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
jeromelaban committed Feb 25, 2022
1 parent b88a6cb commit acfb5ae
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
16 changes: 15 additions & 1 deletion src/Uno.UI.Tests/DependencyProperty/Given_DependencyObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@ public void Should_Have_Bindable_Attribute()
Assert.AreEqual(1, typeof(MyObject).GetCustomAttributes(typeof(BindableAttribute), true).Length);
}

[TestMethod]
public void When_LayoutLoop()
{
var SUT = new ContentControl();
var inner1 = new Grid();
var inner2 = new Grid();

SUT.Content = inner1;
inner1.Children.Add(inner2);
inner2.Children.Add(SUT);

// No exception should be raised for this test, until
// Children.Add validates for cycles.
}

public partial class MyObject : DependencyObject
{
public MyObject(int value)
Expand All @@ -145,7 +160,6 @@ public override bool Equals(object obj)

public override int GetHashCode() => Value.GetHashCode();
}

}

public partial class MyProvider : DependencyObject
Expand Down
26 changes: 24 additions & 2 deletions src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,10 @@ private static bool IsAncestor(DependencyObject? instance, AncestorsDictionary m

if (ancestor != null && !map.TryGetValue(ancestor, out isAncestor))
{
while (instance != null)
// Max iterations for ancestor lookup
var iterations = 1000;

while (instance != null && iterations-- > 0)
{
#if DEBUG
var prevInstance = instance;
Expand All @@ -1584,7 +1587,6 @@ private static bool IsAncestor(DependencyObject? instance, AncestorsDictionary m
{
if (!hashSet.Contains(instance!))
{
// Console.WriteLine($"Added other {(instance as FrameworkElement)?.Name}");
hashSet.Add(instance);
}
else
Expand All @@ -1597,8 +1599,28 @@ private static bool IsAncestor(DependencyObject? instance, AncestorsDictionary m
if (instance == ancestor)
{
isAncestor = true;
break;
}
}

if (iterations < 1)
{
var level =
#if DEBUG
// Make layout cycles visible in debug builds, until Children.Add
// validates for cycles.
LogLevel.Error;
#else
// Layout cycles are ignored in this function for release builds.
LogLevel.Debug;
#endif

if (ancestor.Log().IsEnabled(level))
{
ancestor.Log().Log(level, $"Possible cycle detected, ignoring ancestor [{ancestor}]");
}

isAncestor = false;
}

map.Set(ancestor, isAncestor);
Expand Down

0 comments on commit acfb5ae

Please sign in to comment.