Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed lazy loading thread safety #35529

Merged
merged 6 commits into from
Feb 1, 2025
Merged

Fixed lazy loading thread safety #35529

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3102a3
Summary of the changes
henriquewr Jan 25, 2025
241c105
Fixed a deadlock
henriquewr Jan 28, 2025
cc355ba
Added tests for async lazy loading
henriquewr Jan 29, 2025
f7be7d3
Fixed tests for lazy loading disabled
henriquewr Jan 30, 2025
e406dc0
Changed lazy load to use depth instead of thread id
henriquewr Jan 30, 2025
10a595a
Added comments
henriquewr Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 78 additions & 42 deletions src/EFCore/Infrastructure/Internal/LazyLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Concurrent;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Internal;

namespace Microsoft.EntityFrameworkCore.Infrastructure.Internal;
Expand All @@ -20,7 +21,8 @@ public class LazyLoader : ILazyLoader, IInjectableService
private bool _disposed;
private bool _detached;
private IDictionary<string, bool>? _loadedStates;
private readonly ConcurrentDictionary<(object Entity, string NavigationName), bool> _isLoading = new(NavEntryEqualityComparer.Instance);
private readonly Lock _isLoadingLock = new Lock();
private readonly ConcurrentDictionary<(object Entity, string NavigationName), TaskCompletionSource> _isLoading = new(NavEntryEqualityComparer.Instance);
private HashSet<string>? _nonLazyNavigations;

/// <summary>
Expand Down Expand Up @@ -107,31 +109,48 @@ public virtual void Load(object entity, [CallerMemberName] string navigationName
Check.NotEmpty(navigationName, nameof(navigationName));

var navEntry = (entity, navigationName);
if (_isLoading.TryAdd(navEntry, true))

var willWait = true;
TaskCompletionSource taskCompletionSource;

lock (_isLoadingLock)
{
taskCompletionSource = _isLoading.GetOrAdd(navEntry, x =>
{
willWait = false;
return new TaskCompletionSource();
});
}

if (willWait)
{
try
taskCompletionSource.Task.Wait();
return;
}

try
{
// ShouldLoad is called after _isLoading.Add because it could attempt to load the property. See #13138.
if (ShouldLoad(entity, navigationName, out var entry))
{
// ShouldLoad is called after _isLoading.Add because it could attempt to load the property. See #13138.
if (ShouldLoad(entity, navigationName, out var entry))
try
{
try
{
entry.Load(
_queryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution
? LoadOptions.ForceIdentityResolution
: LoadOptions.None);
}
catch
{
entry.IsLoaded = false;
throw;
}
entry.Load(
_queryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution
? LoadOptions.ForceIdentityResolution
: LoadOptions.None);
}
catch
{
entry.IsLoaded = false;
throw;
}
}
finally
{
_isLoading.TryRemove(navEntry, out _);
}
}
finally
{
taskCompletionSource.SetResult();
_isLoading.TryRemove(navEntry, out _);
}
}

Expand All @@ -150,32 +169,49 @@ public virtual async Task LoadAsync(
Check.NotEmpty(navigationName, nameof(navigationName));

var navEntry = (entity, navigationName);
if (_isLoading.TryAdd(navEntry, true))

var willWait = true;
TaskCompletionSource taskCompletionSource;

lock (_isLoadingLock)
{
taskCompletionSource = _isLoading.GetOrAdd(navEntry, x =>
{
willWait = false;
return new TaskCompletionSource();
});
}

if (willWait)
{
try
await taskCompletionSource.Task.WaitAsync(cancellationToken).ConfigureAwait(false);
return;
}

try
{
// ShouldLoad is called after _isLoading.Add because it could attempt to load the property. See #13138.
if (ShouldLoad(entity, navigationName, out var entry))
{
// ShouldLoad is called after _isLoading.Add because it could attempt to load the property. See #13138.
if (ShouldLoad(entity, navigationName, out var entry))
try
{
try
{
await entry.LoadAsync(
_queryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution
? LoadOptions.ForceIdentityResolution
: LoadOptions.None,
cancellationToken).ConfigureAwait(false);
}
catch
{
entry.IsLoaded = false;
throw;
}
await entry.LoadAsync(
_queryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution
? LoadOptions.ForceIdentityResolution
: LoadOptions.None,
cancellationToken).ConfigureAwait(false);
}
catch
{
entry.IsLoaded = false;
throw;
}
}
finally
{
_isLoading.TryRemove(navEntry, out _);
}
}
finally
{
taskCompletionSource.SetResult();
_isLoading.TryRemove(navEntry, out _);
}
}

Expand Down
Loading