Skip to content

Commit

Permalink
Avoid stack overflow when lazy loading
Browse files Browse the repository at this point in the history
Fixes #13138

(Note that it's harder to hit this in 3+ because field mapping is the default.)
  • Loading branch information
ajcvickers committed Dec 23, 2019
1 parent 8e018ca commit b3a89d0
Show file tree
Hide file tree
Showing 3 changed files with 227 additions and 166 deletions.
8 changes: 5 additions & 3 deletions src/EFCore/Internal/LazyLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,7 @@ public virtual Task LoadAsync(
: Task.CompletedTask;
}

private bool ShouldLoad(
object entity, string navigationName,
out NavigationEntry navigationEntry)
private bool ShouldLoad(object entity, string navigationName, out NavigationEntry navigationEntry)
{
if (_loadedStates != null
&& _loadedStates.TryGetValue(navigationName, out var loaded)
Expand All @@ -141,6 +139,9 @@ private bool ShouldLoad(
}
else if (Context.ChangeTracker.LazyLoadingEnabled)
{
// Set early to avoid recursive loading overflow
SetLoaded(entity, navigationName, loaded: true);

var entityEntry = Context.Entry(entity); // Will use local-DetectChanges, if enabled.
var tempNavigationEntry = entityEntry.Navigation(navigationName);

Expand All @@ -153,6 +154,7 @@ private bool ShouldLoad(
Logger.NavigationLazyLoading(Context, entity, navigationName);

navigationEntry = tempNavigationEntry;

return true;
}
}
Expand Down
61 changes: 60 additions & 1 deletion test/EFCore.Specification.Tests/LazyLoadProxyTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,39 @@ public abstract class LazyLoadProxyTestBase<TFixture> : IClassFixture<TFixture>

protected TFixture Fixture { get; }

[ConditionalTheory] // Issue #13138
[InlineData(EntityState.Unchanged)]
[InlineData(EntityState.Modified)]
[InlineData(EntityState.Deleted)]
public virtual void Lazy_load_one_to_one_reference_with_recursive_property(EntityState state)
{
using (var context = CreateContext(lazyLoadingEnabled: true))
{
var child = context.Set<WithRecursiveProperty>().Single();

var referenceEntry = context.Entry(child).Reference(e => e.Parent);

context.Entry(child).State = state;

Assert.True(referenceEntry.IsLoaded);

Assert.NotNull(child.Parent);

Assert.True(referenceEntry.IsLoaded);

context.ChangeTracker.LazyLoadingEnabled = false;

Assert.Equal(2, context.ChangeTracker.Entries().Count());

var parent = context.ChangeTracker.Entries<Parent>().Single().Entity;

Assert.Equal(parent.Id, child.IdLoadedFromParent);

Assert.Same(parent, child.Parent);
Assert.Same(child, parent.WithRecursiveProperty);
}
}

[ConditionalTheory]
[InlineData(EntityState.Unchanged, false)]
[InlineData(EntityState.Modified, false)]
Expand Down Expand Up @@ -2134,8 +2167,33 @@ public class Parent
public virtual SingleShadowFk SingleShadowFk { get; set; }
public virtual IEnumerable<ChildCompositeKey> ChildrenCompositeKey { get; set; }
public virtual SingleCompositeKey SingleCompositeKey { get; set; }
public virtual WithRecursiveProperty WithRecursiveProperty { get; set; }
}

public class WithRecursiveProperty
{
private int _backing;

[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id { get; set; }

public int? ParentId { get; set; }
public virtual Parent Parent { get; set; }

public int IdLoadedFromParent
{
get
{
if (Parent != null)
{
_backing = Parent.Id;
}

return _backing;
}
set => _backing = value;
}
}
public class Child
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
Expand Down Expand Up @@ -2474,7 +2532,8 @@ protected override void Seed(DbContext context)
{
new ChildCompositeKey { Id = 51 }, new ChildCompositeKey { Id = 52 }
},
SingleCompositeKey = new SingleCompositeKey { Id = 62 }
SingleCompositeKey = new SingleCompositeKey { Id = 62 },
WithRecursiveProperty = new WithRecursiveProperty { Id = 8086 }
});

context.Add(
Expand Down
Loading

0 comments on commit b3a89d0

Please sign in to comment.