Skip to content

Fix in-memory corruption of nested owned entities after SaveChanges when navigation is replaced #37787

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

Merged
AndriySvyryd merged 8 commits into from
Feb 26, 2026
Merged

Fix in-memory corruption of nested owned entities after SaveChanges when navigation is replaced #37787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8ba63f4
Initial plan
Copilot Feb 24, 2026
7fbe6ad
Fix in-memory corruption of nested owned entities after SaveChanges w…
Copilot Feb 24, 2026
e7a080a
Improve comment explaining CascadeDelete skip logic
Copilot Feb 24, 2026
3b85272
Use SharedIdentityEntry checks instead of FindPrincipal per reviewer …
Copilot Feb 25, 2026
6a6718f
Update test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTe…
AndriySvyryd Feb 25, 2026
5d59a9c
Cache node.SourceEntry?.GetInfrastructure() in local variable in Enti…
Copilot Feb 25, 2026
ace8853
Add missing test overrides for Replace_json_reference_root_preserves_…
Copilot Feb 26, 2026
0232ce2
Add correct SQL baseline for Replace_json_reference_root_preserves_ne…
Copilot Feb 26, 2026
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
15 changes: 11 additions & 4 deletions src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ private bool PaintAction(
SetReferenceLoaded(node);

var internalEntityEntry = node.GetInfrastructure();
if (internalEntityEntry.EntityState != EntityState.Detached
var sourceEntry = node.SourceEntry?.GetInfrastructure();

if ((internalEntityEntry.EntityState != EntityState.Detached
&& (internalEntityEntry.EntityState != EntityState.Deleted
|| sourceEntry?.SharedIdentityEntry == null))
|| (_visited != null && _visited.Contains(internalEntityEntry.Entity)))
{
Comment thread
AndriySvyryd marked this conversation as resolved.
return false;
Expand All @@ -114,7 +118,7 @@ private bool PaintAction(

if (internalEntityEntry.StateManager.ResolveToExistingEntry(
internalEntityEntry,
node.InboundNavigation, node.SourceEntry?.GetInfrastructure()))
node.InboundNavigation, sourceEntry))
{
(_visited ??= new HashSet<object>(ReferenceEqualityComparer.Instance)).Add(internalEntityEntry.Entity);
}
Expand All @@ -139,7 +143,10 @@ private async Task<bool> PaintActionAsync(
SetReferenceLoaded(node);

var internalEntityEntry = node.GetInfrastructure();
if (internalEntityEntry.EntityState != EntityState.Detached
var sourceEntry = node.SourceEntry?.GetInfrastructure();
if ((internalEntityEntry.EntityState != EntityState.Detached
&& (internalEntityEntry.EntityState != EntityState.Deleted
|| sourceEntry?.SharedIdentityEntry == null))
|| (_visited != null && _visited.Contains(internalEntityEntry.Entity)))
{
return false;
Expand All @@ -151,7 +158,7 @@ private async Task<bool> PaintActionAsync(

if (internalEntityEntry.StateManager.ResolveToExistingEntry(
internalEntityEntry,
node.InboundNavigation, node.SourceEntry?.GetInfrastructure()))
node.InboundNavigation, sourceEntry))
{
(_visited ??= []).Add(internalEntityEntry.Entity);
}
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/ChangeTracking/Internal/StateManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,14 @@ public virtual void CascadeChanges(bool force)
/// </summary>
public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumerable<IForeignKey>? foreignKeys = null)
{
// When an owned entity is replaced (e.g., via record 'with' expression), the old entry is
// marked Deleted and a new entry with the same key is linked via SharedIdentityEntry.
// Skip cascade from the old entry since the replacement handles its own dependents.
if (entry.SharedIdentityEntry != null)
Comment thread
AndriySvyryd marked this conversation as resolved.
{
return;
}

var doCascadeDelete = force || CascadeDeleteTiming != CascadeTiming.Never;
var principalIsDetached = entry.EntityState == EntityState.Detached;

Expand Down
61 changes: 61 additions & 0 deletions test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3671,6 +3671,67 @@ public virtual Task Edit_single_property_with_non_ascii_characters()
Assert.Equal("测试1", result.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf.SomethingSomething);
});

[ConditionalFact]
public virtual Task Replace_json_reference_root_preserves_nested_owned_entities_in_memory()
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
CreateContext,
UseTransaction,
async context =>
{
var query = await context.JsonEntitiesBasic.ToListAsync();
var entity = query.Single();

// Save original leaf value
var originalLeaf = entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf;
var originalLeafValue = originalLeaf.SomethingSomething;

// Replace the owned reference with a new instance that shares nested reference navigations
var oldRoot = entity.OwnedReferenceRoot;
entity.OwnedReferenceRoot = new JsonOwnedRoot
{
Name = "Modified",
Number = oldRoot.Number,
Names = oldRoot.Names,
Numbers = oldRoot.Numbers,
OwnedReferenceBranch = new JsonOwnedBranch
{
Id = oldRoot.OwnedReferenceBranch.Id,
Date = oldRoot.OwnedReferenceBranch.Date,
Enum = oldRoot.OwnedReferenceBranch.Enum,
Fraction = oldRoot.OwnedReferenceBranch.Fraction,
Comment thread
AndriySvyryd marked this conversation as resolved.
NullableEnum = oldRoot.OwnedReferenceBranch.NullableEnum,
Enums = oldRoot.OwnedReferenceBranch.Enums,
NullableEnums = oldRoot.OwnedReferenceBranch.NullableEnums,
OwnedReferenceLeaf = originalLeaf,
OwnedCollectionLeaf = [],
},
OwnedCollectionBranch = [],
};

// Before DetectChanges, leaf should be accessible
Assert.Same(originalLeaf, entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf);

context.ChangeTracker.DetectChanges();

// After DetectChanges, leaf should still be accessible
Assert.NotNull(entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf);

ClearLog();
await context.SaveChangesAsync();

// After SaveChanges, nested owned entities should still be accessible in memory
Assert.NotNull(entity.OwnedReferenceRoot.OwnedReferenceBranch);
Assert.NotNull(entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf);
Assert.Equal(originalLeafValue, entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf.SomethingSomething);
},
async context =>
{
var result = await context.Set<JsonEntityBasic>().SingleAsync();
Assert.Equal("Modified", result.OwnedReferenceRoot.Name);
Assert.NotNull(result.OwnedReferenceRoot.OwnedReferenceBranch);
Assert.NotNull(result.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf);
});

public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
=> facade.UseTransaction(transaction.GetDbTransaction());

Expand Down
22 changes: 22 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateJsonTypeSqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3274,6 +3274,28 @@ FROM [JsonEntitiesAllTypes] AS [j]
""");
}

public override async Task Replace_json_reference_root_preserves_nested_owned_entities_in_memory()
{
await base.Replace_json_reference_root_preserves_nested_owned_entities_in_memory();

AssertSql(
"""
@p0='{"Id":0,"Name":"Modified","Names":["e1_r1","e1_r2"],"Number":10,"Numbers":[-2147483648,-1,0,1,2147483647],"OwnedCollectionBranch":[],"OwnedReferenceBranch":{"Date":"2100-01-01T00:00:00","Enum":-1,"Enums":[-1,-1,2],"Fraction":10.0,"Id":88,"NullableEnum":null,"NullableEnums":[null,-1,2],"OwnedCollectionLeaf":[],"OwnedReferenceLeaf":{"SomethingSomething":"e1_r_r_r"}}}' (Nullable = false) (Size = 367)
@p1='1'

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
UPDATE [JsonEntitiesBasic] SET [OwnedReferenceRoot] = @p0
OUTPUT 1
WHERE [Id] = @p1;
""",
//
"""
SELECT TOP(2) [j].[Id], [j].[EntityBasicId], [j].[Name], [j].[OwnedCollectionRoot], [j].[OwnedReferenceRoot]
FROM [JsonEntitiesBasic] AS [j]
""");
}

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();

Expand Down
21 changes: 21 additions & 0 deletions test/EFCore.Sqlite.FunctionalTests/Update/JsonUpdateSqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2408,6 +2408,27 @@ public override Task Edit_single_property_collection_of_collection_of_nullable_i
public override Task Edit_single_property_collection_of_collection_of_single()
=> Assert.ThrowsAsync<ArgumentOutOfRangeException>(base.Edit_single_property_collection_of_collection_of_single);

public override async Task Replace_json_reference_root_preserves_nested_owned_entities_in_memory()
{
await base.Replace_json_reference_root_preserves_nested_owned_entities_in_memory();

AssertSql(
"""
@p0='{"Id":0,"Name":"Modified","Names":["e1_r1","e1_r2"],"Number":10,"Numbers":[-2147483648,-1,0,1,2147483647],"OwnedCollectionBranch":[],"OwnedReferenceBranch":{"Date":"2100-01-01 00:00:00","Enum":-1,"Enums":[-1,-1,2],"Fraction":"10.0","Id":88,"NullableEnum":null,"NullableEnums":[null,-1,2],"OwnedCollectionLeaf":[],"OwnedReferenceLeaf":{"SomethingSomething":"e1_r_r_r"}}}' (Nullable = false) (Size = 369)
@p1='1'

UPDATE "JsonEntitiesBasic" SET "OwnedReferenceRoot" = @p0
WHERE "Id" = @p1
RETURNING 1;
""",
//
"""
SELECT "j"."Id", "j"."EntityBasicId", "j"."Name", "j"."OwnedCollectionRoot", "j"."OwnedReferenceRoot"
FROM "JsonEntitiesBasic" AS "j"
LIMIT 2
""");
}

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();

Expand Down
Loading