Skip to content

Commit

Permalink
Make compiled model and runtime initialization thread-safe
Browse files Browse the repository at this point in the history 
Fixes #25205
  • Loading branch information
@AndriySvyryd
AndriySvyryd authored Aug 18, 2021
1 parent 0c644f3 commit f8ef489
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 146 deletions.
28 changes: 11 additions & 17 deletions src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,24 +164,18 @@ private string CreateModel(
using (mainBuilder.Indent())
{
mainBuilder
.Append("private static ").Append(className).AppendLine(nullable ? "? _instance;" : " _instance;")
.Append("public static IModel Instance")
.AppendLines(@"
.Append("static ").Append(className).Append("()")
.AppendLines(
@"
{
get
{
if (_instance == null)
{
_instance = new " + className + @"();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
}");

mainBuilder
var model = new " + className + @"();
model.Initialize();
model.Customize();
_instance = model;
}")
.AppendLine()
.Append("private static ").Append(className).AppendLine(nullable ? "? _instance;" : " _instance;")
.AppendLine("public static IModel Instance => _instance;")
.AppendLine()
.AppendLine("partial void Initialize();")
.AppendLine()
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Infrastructure/RelationalModelRuntimeInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected override void InitializeModel(IModel model, bool designTime, bool prev
{
if (prevalidation)
{
model.AddRuntimeAnnotation(RelationalAnnotationNames.ModelDependencies, RelationalDependencies.RelationalModelDependencies);
model.SetRuntimeAnnotation(RelationalAnnotationNames.ModelDependencies, RelationalDependencies.RelationalModelDependencies);
}
else
{
Expand Down
70 changes: 34 additions & 36 deletions src/EFCore/Infrastructure/ModelRuntimeInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure
/// </summary>
public class ModelRuntimeInitializer : IModelRuntimeInitializer
{
private static readonly object _syncObject = new();

/// <summary>
/// Creates a new <see cref="ModelRuntimeInitializer" /> instance.
/// </summary>
Expand Down Expand Up @@ -56,59 +58,55 @@ public virtual IModel Initialize(
if (model is Model mutableModel
&& !mutableModel.IsReadOnly)
{
model = mutableModel.FinalizeModel();
lock (_syncObject)
{
if (!mutableModel.IsReadOnly)
{
model = mutableModel.FinalizeModel();
}
}
}

if (model.ModelDependencies == null)
{
model = model.GetOrAddRuntimeAnnotationValue(
CoreAnnotationNames.ReadOnlyModel,
static args =>
// Make sure InitializeModel really only gets called once, since it may not be thread safe or idempotent.
lock (_syncObject)
{
if (model.ModelDependencies == null)
{
var (initializer, model, designTime, validationLogger) = args;
model.ModelDependencies = Dependencies.ModelDependencies;

model.ModelDependencies = initializer.Dependencies.ModelDependencies;

initializer.InitializeModel(model, designTime, prevalidation: true);
InitializeModel(model, designTime, prevalidation: true);

if (validationLogger != null
&& model is IConventionModel)
{
initializer.Dependencies.ModelValidator.Validate(model, validationLogger);
}

initializer.InitializeModel(model, designTime, prevalidation: false);

if (!designTime
&& model is Model mutableModel)
{
model = mutableModel.OnModelFinalized();
Dependencies.ModelValidator.Validate(model, validationLogger);
}

return model;
},
(this, model, designTime, validationLogger));

if (designTime)
{
model.RemoveRuntimeAnnotation(CoreAnnotationNames.ReadOnlyModel);
InitializeModel(model, designTime, prevalidation: false);
}
}
}
else if (!designTime)

if (designTime)
{
model = model.GetOrAddRuntimeAnnotationValue(
CoreAnnotationNames.ReadOnlyModel,
static model =>
return model;
}

model = model.GetOrAddRuntimeAnnotationValue(
CoreAnnotationNames.ReadOnlyModel,
static model =>
{
if (model is Model mutableModel)
{
if (model is Model mutableModel)
{
model = mutableModel.OnModelFinalized();
}
// This assumes OnModelFinalized is thread-safe
model = mutableModel.OnModelFinalized();
}

return model!;
},
model);
}
return model!;
},
model);

return model;
}
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Conventions/IModelFinalizedConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
public interface IModelFinalizedConvention : IConvention
{
/// <summary>
/// Called after a model is finalized and can no longer be mutated.
/// <para> Called after a model is finalized and can no longer be mutated. </para>
/// <para> The implementation must be thread-safe. </para>
/// </summary>
/// <param name="model"> The model. </param>
IModel ProcessModelFinalized(IModel model);
Expand Down
147 changes: 56 additions & 91 deletions test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,17 @@ namespace TestNamespace
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.EmptyContext))]
partial class EmptyContextModel : RuntimeModel
{
private static EmptyContextModel _instance;
public static IModel Instance
static EmptyContextModel()
{
get
{
if (_instance == null)
{
_instance = new EmptyContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new EmptyContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static EmptyContextModel _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down Expand Up @@ -476,22 +471,17 @@ namespace TestNamespace
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.BigContext))]
partial class BigContextModel : RuntimeModel
{
private static BigContextModel? _instance;
public static IModel Instance
static BigContextModel()
{
get
{
if (_instance == null)
{
_instance = new BigContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new BigContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static BigContextModel? _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down Expand Up @@ -1853,22 +1843,17 @@ namespace TestNamespace
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.DbFunctionContext))]
partial class DbFunctionContextModel : RuntimeModel
{
private static DbFunctionContextModel _instance;
public static IModel Instance
static DbFunctionContextModel()
{
get
{
if (_instance == null)
{
_instance = new DbFunctionContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new DbFunctionContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static DbFunctionContextModel _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down Expand Up @@ -2343,22 +2328,17 @@ namespace TestNamespace
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.SequencesContext))]
partial class SequencesContextModel : RuntimeModel
{
private static SequencesContextModel _instance;
public static IModel Instance
static SequencesContextModel()
{
get
{
if (_instance == null)
{
_instance = new SequencesContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new SequencesContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static SequencesContextModel _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down Expand Up @@ -2551,22 +2531,17 @@ namespace TestNamespace
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.ConstraintsContext))]
partial class ConstraintsContextModel : RuntimeModel
{
private static ConstraintsContextModel _instance;
public static IModel Instance
static ConstraintsContextModel()
{
get
{
if (_instance == null)
{
_instance = new ConstraintsContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new ConstraintsContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static ConstraintsContextModel _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down Expand Up @@ -2707,22 +2682,17 @@ namespace Microsoft.EntityFrameworkCore.Metadata
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.SqliteContext))]
partial class SqliteContextModel : RuntimeModel
{
private static SqliteContextModel _instance;
public static IModel Instance
static SqliteContextModel()
{
get
{
if (_instance == null)
{
_instance = new SqliteContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new SqliteContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static SqliteContextModel _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down Expand Up @@ -2887,22 +2857,17 @@ namespace TestNamespace
[DbContext(typeof(CSharpRuntimeModelCodeGeneratorTest.CosmosContext))]
partial class CosmosContextModel : RuntimeModel
{
private static CosmosContextModel _instance;
public static IModel Instance
static CosmosContextModel()
{
get
{
if (_instance == null)
{
_instance = new CosmosContextModel();
_instance.Initialize();
_instance.Customize();
}
return _instance;
}
var model = new CosmosContextModel();
model.Initialize();
model.Customize();
_instance = model;
}
private static CosmosContextModel _instance;
public static IModel Instance => _instance;
partial void Initialize();
partial void Customize();
Expand Down
Loading

0 comments on commit f8ef489

Please sign in to comment.