Skip to content

Commit

Permalink
Fix buggy uses of NonCapturingLazyInitializer (#23881)
Browse files Browse the repository at this point in the history
  • Loading branch information
roji authored Jan 15, 2021
1 parent 09e8bad commit 6d97708
Show file tree
Hide file tree
Showing 18 changed files with 96 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private static void IncludeCollection<TEntity, TIncludingEntity, TIncludedEntity
{
if (entity is TIncludingEntity includingEntity)
{
var collectionAccessor = navigation.GetCollectionAccessor();
var collectionAccessor = navigation.GetCollectionAccessor()!;
collectionAccessor.GetOrCreate(includingEntity, forMaterialization: true);

if (setLoaded)
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Update/ModificationCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public virtual EntityState EntityState
/// </summary>
public virtual IReadOnlyList<ColumnModification> ColumnModifications
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _columnModifications, this, command => command.GenerateColumnModifications());
ref _columnModifications, this, static command => command.GenerateColumnModifications());

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore/ChangeTracking/ValueComparer`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public override int GetHashCode(object instance)
/// <returns> <see langword="true" /> if they are equal; <see langword="false" /> otherwise. </returns>
public virtual bool Equals(T left, T right)
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _equals, this, c => c.EqualsExpression.Compile())(left, right);
ref _equals, this, static c => c.EqualsExpression.Compile())(left, right);

/// <summary>
/// Returns the hash code for the given instance.
Expand All @@ -275,7 +275,7 @@ public virtual bool Equals(T left, T right)
/// <returns> The hash code. </returns>
public virtual int GetHashCode(T instance)
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _hashCode, this, c => c.HashCodeExpression.Compile())(instance);
ref _hashCode, this, static c => c.HashCodeExpression.Compile())(instance);

/// <summary>
/// <para>
Expand Down Expand Up @@ -308,7 +308,7 @@ public override object Snapshot(object instance)
/// <returns> The snapshot. </returns>
public virtual T Snapshot([CanBeNull] T instance)
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _snapshot, this, c => c.SnapshotExpression.Compile())(instance);
ref _snapshot, this, static c => c.SnapshotExpression.Compile())(instance);

/// <summary>
/// The type.
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Internal/InternalDbSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private EntityQueryable<TEntity> EntityQueryable
return NonCapturingLazyInitializer.EnsureInitialized(
ref _entityQueryable,
this,
internalSet => internalSet.CreateEntityQueryable());
static internalSet => internalSet.CreateEntityQueryable());
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/INavigation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ bool IsOnDependent
/// </summary>
/// <returns> The accessor. </returns>
[DebuggerStepThrough]
new IClrCollectionAccessor GetCollectionAccessor()
new IClrCollectionAccessor? GetCollectionAccessor()
=> ((Navigation)this).CollectionAccessor;

/// <summary>
Expand Down Expand Up @@ -114,7 +114,7 @@ bool INavigationBase.IsCollection
/// </summary>
/// <returns> The accessor. </returns>
[DebuggerStepThrough]
IClrCollectionAccessor INavigationBase.GetCollectionAccessor()
IClrCollectionAccessor? INavigationBase.GetCollectionAccessor()
=> GetCollectionAccessor();
}
}
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/INavigationBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ bool IsEagerLoaded
/// navigation.
/// </summary>
/// <returns> The accessor. </returns>
IClrCollectionAccessor GetCollectionAccessor();
IClrCollectionAccessor? GetCollectionAccessor();

/// <summary>
/// <para>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/ISkipNavigation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ INavigationBase INavigationBase.Inverse
/// navigation.
/// </summary>
/// <returns> The accessor. </returns>
IClrCollectionAccessor INavigationBase.GetCollectionAccessor()
IClrCollectionAccessor? INavigationBase.GetCollectionAccessor()
=> ((SkipNavigation)this).CollectionAccessor;
}
}
18 changes: 9 additions & 9 deletions src/EFCore/Metadata/Internal/EntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2544,7 +2544,7 @@ public virtual IEnumerable<Property> GetProperties()
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual PropertyCounts Counts
=> NonCapturingLazyInitializer.EnsureInitialized(ref _counts, this, entityType => entityType.CalculateCounts());
=> NonCapturingLazyInitializer.EnsureInitialized(ref _counts, this, static entityType => entityType.CalculateCounts());

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2555,7 +2555,7 @@ public virtual PropertyCounts Counts
public virtual Func<InternalEntityEntry, ISnapshot> RelationshipSnapshotFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _relationshipSnapshotFactory, this,
entityType => new RelationshipSnapshotFactoryFactory().Create(entityType));
static entityType => new RelationshipSnapshotFactoryFactory().Create(entityType));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2566,7 +2566,7 @@ public virtual Func<InternalEntityEntry, ISnapshot> RelationshipSnapshotFactory
public virtual Func<InternalEntityEntry, ISnapshot> OriginalValuesFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _originalValuesFactory, this,
entityType => new OriginalValuesFactoryFactory().Create(entityType));
static entityType => new OriginalValuesFactoryFactory().Create(entityType));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2577,7 +2577,7 @@ public virtual Func<InternalEntityEntry, ISnapshot> OriginalValuesFactory
public virtual Func<InternalEntityEntry, ISnapshot> StoreGeneratedValuesFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _storeGeneratedValuesFactory, this,
entityType => new SidecarValuesFactoryFactory().Create(entityType));
static entityType => new SidecarValuesFactoryFactory().Create(entityType));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2588,7 +2588,7 @@ public virtual Func<InternalEntityEntry, ISnapshot> StoreGeneratedValuesFactory
public virtual Func<InternalEntityEntry, ISnapshot> TemporaryValuesFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _temporaryValuesFactory, this,
entityType => new TemporaryValuesFactoryFactory().Create(entityType));
static entityType => new TemporaryValuesFactoryFactory().Create(entityType));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2599,7 +2599,7 @@ public virtual Func<InternalEntityEntry, ISnapshot> TemporaryValuesFactory
public virtual Func<ValueBuffer, ISnapshot> ShadowValuesFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _shadowValuesFactory, this,
entityType => new ShadowValuesFactoryFactory().Create(entityType));
static entityType => new ShadowValuesFactoryFactory().Create(entityType));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2610,7 +2610,7 @@ public virtual Func<ValueBuffer, ISnapshot> ShadowValuesFactory
public virtual Func<ISnapshot> EmptyShadowValuesFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _emptyShadowValuesFactory, this,
entityType => new EmptyShadowValuesFactoryFactory().CreateEmpty(entityType));
static entityType => new EmptyShadowValuesFactoryFactory().CreateEmpty(entityType));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -2621,7 +2621,7 @@ public virtual Func<ISnapshot> EmptyShadowValuesFactory
public virtual Func<MaterializationContext, object> InstanceFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _instanceFactory, this,
entityType =>
static entityType =>
{
var binding = (InstantiationBinding?)entityType[CoreAnnotationNames.ServiceOnlyConstructorBinding];
if (binding == null)
Expand All @@ -2631,7 +2631,7 @@ public virtual Func<MaterializationContext, object> InstanceFactory
var contextParam = Expression.Parameter(typeof(MaterializationContext), "mc");
_instanceFactory = Expression.Lambda<Func<MaterializationContext, object>>(
entityType._instanceFactory = Expression.Lambda<Func<MaterializationContext, object>>(
binding.CreateConstructorExpression(
new ParameterBindingInfo(entityType, contextParam)),
contextParam)
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/Index.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ protected override IConventionAnnotation OnAnnotationSet(
/// </summary>
public virtual IDependentKeyValueFactory<TKey> GetNullableValueFactory<TKey>()
=> (IDependentKeyValueFactory<TKey>)NonCapturingLazyInitializer.EnsureInitialized(
ref _nullableValueFactory, this, i => new CompositeValueFactory(i.Properties));
ref _nullableValueFactory, this, static i => new CompositeValueFactory(i.Properties));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Internal/Key.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public virtual IEnumerable<ForeignKey> GetReferencingForeignKeys()
/// </summary>
public virtual Func<bool, IIdentityMap> IdentityMapFactory
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _identityMapFactory, this, k => new IdentityMapFactoryFactory().Create(k));
ref _identityMapFactory, this, static k => new IdentityMapFactoryFactory().Create(k));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -147,7 +147,7 @@ public virtual Func<bool, IIdentityMap> IdentityMapFactory
/// </summary>
public virtual IPrincipalKeyValueFactory<TKey> GetPrincipalKeyValueFactory<TKey>()
=> (IPrincipalKeyValueFactory<TKey>)NonCapturingLazyInitializer.EnsureInitialized(
ref _principalKeyValueFactory, this, k => new KeyValueFactoryFactory().Create<TKey>(k));
ref _principalKeyValueFactory, this, static k => new KeyValueFactoryFactory().Create<TKey>(k));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
8 changes: 6 additions & 2 deletions src/EFCore/Metadata/Internal/Navigation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class Navigation : PropertyBase, IMutableNavigation, IConventionNavigatio
{
// Warning: Never access these fields directly as access needs to be thread-safe
private IClrCollectionAccessor? _collectionAccessor;
private bool _collectionAccessorInitialized;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down Expand Up @@ -271,9 +272,12 @@ public virtual Navigation? Inverse
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual IClrCollectionAccessor CollectionAccessor
public virtual IClrCollectionAccessor? CollectionAccessor
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _collectionAccessor, this, n => new ClrCollectionAccessorFactory().Create(n));
ref _collectionAccessor,
ref _collectionAccessorInitialized,
this,
static n => new ClrCollectionAccessorFactory().Create(n));

/// <summary>
/// Runs the conventions when an annotation was set or removed.
Expand Down
12 changes: 6 additions & 6 deletions src/EFCore/Metadata/Internal/PropertyBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ public virtual PropertyIndexes PropertyIndexes
get
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _indexes, this,
property =>
static property =>
{
var _ = (property.DeclaringType as EntityType)?.Counts;
});
Expand Down Expand Up @@ -352,7 +352,7 @@ private void UpdateFieldInfoConfigurationSource(ConfigurationSource configuratio
/// </summary>
public virtual IClrPropertyGetter Getter
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _getter, this, p => new ClrPropertyGetterFactory().Create(p));
ref _getter, this, static p => new ClrPropertyGetterFactory().Create(p));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -362,7 +362,7 @@ public virtual IClrPropertyGetter Getter
/// </summary>
public virtual IClrPropertySetter Setter
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _setter, this, p => new ClrPropertySetterFactory().Create(p));
ref _setter, this, static p => new ClrPropertySetterFactory().Create(p));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -372,7 +372,7 @@ public virtual IClrPropertySetter Setter
/// </summary>
public virtual IClrPropertySetter MaterializationSetter
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _materializationSetter, this, p => new ClrPropertyMaterializationSetterFactory().Create(p));
ref _materializationSetter, this, static p => new ClrPropertyMaterializationSetterFactory().Create(p));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -381,7 +381,7 @@ public virtual IClrPropertySetter MaterializationSetter
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual PropertyAccessors Accessors
=> NonCapturingLazyInitializer.EnsureInitialized(ref _accessors, this, p => new PropertyAccessorsFactory().Create(p));
=> NonCapturingLazyInitializer.EnsureInitialized(ref _accessors, this, static p => new PropertyAccessorsFactory().Create(p));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -391,7 +391,7 @@ public virtual PropertyAccessors Accessors
/// </summary>
public virtual IComparer<IUpdateEntry> CurrentValueComparer
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _currentValueComparer, this, p => new CurrentValueComparerFactory().Create(p));
ref _currentValueComparer, this, static p => new CurrentValueComparerFactory().Create(p));

private static readonly MethodInfo _containsKeyMethod =
typeof(IDictionary<string, object>).GetRequiredMethod(nameof(IDictionary<string, object>.ContainsKey), new[] { typeof(string) });
Expand Down
10 changes: 7 additions & 3 deletions src/EFCore/Metadata/Internal/SkipNavigation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class SkipNavigation : PropertyBase, IMutableSkipNavigation, IConventionS

// Warning: Never access these fields directly as access needs to be thread-safe
private IClrCollectionAccessor? _collectionAccessor;
private bool _collectionAccessorInitialized;
private ICollectionLoader? _manyToManyLoader;

/// <summary>
Expand Down Expand Up @@ -323,9 +324,12 @@ protected override IConventionAnnotation OnAnnotationSet(
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual IClrCollectionAccessor CollectionAccessor
public virtual IClrCollectionAccessor? CollectionAccessor
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _collectionAccessor, this, n => new ClrCollectionAccessorFactory().Create(n));
ref _collectionAccessor,
ref _collectionAccessorInitialized,
this,
static n => new ClrCollectionAccessorFactory().Create(n));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -335,7 +339,7 @@ public virtual IClrCollectionAccessor CollectionAccessor
/// </summary>
public virtual ICollectionLoader ManyToManyLoader
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _manyToManyLoader, this, n => new ManyToManyLoaderFactory().Create(this));
ref _manyToManyLoader, this, static n => new ManyToManyLoaderFactory().Create(n));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/ServiceParameterBinding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public abstract Expression BindToParameter(
/// </summary>
public virtual Func<MaterializationContext, IEntityType, object, object> ServiceDelegate
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _serviceDelegate, this, b =>
ref _serviceDelegate, this, static b =>
{
var materializationContextParam = Expression.Parameter(typeof(MaterializationContext));
var entityTypeParam = Expression.Parameter(typeof(IEntityType));
Expand Down
7 changes: 4 additions & 3 deletions src/EFCore/Query/Internal/CompiledQueryBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,15 @@ protected abstract Func<QueryContext, TResult> CreateCompiledQuery(
private Func<QueryContext, TResult> EnsureExecutor(TContext context)
=> NonCapturingLazyInitializer.EnsureInitialized(
ref _executor,
this,
context,
_queryExpression,
(c, q) =>
static (t, c, q) =>
{
var queryCompiler = context.GetService<IQueryCompiler>();
var queryCompiler = c.GetService<IQueryCompiler>();
var expression = new QueryExpressionRewriter(c, q.Parameters).Visit(q.Body);
return CreateCompiledQuery(queryCompiler, expression);
return t.CreateCompiledQuery(queryCompiler, expression);
});

private sealed class QueryExpressionRewriter : ExpressionVisitor
Expand Down
Loading

0 comments on commit 6d97708

Please sign in to comment.