-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix duplicate entity tracking error when re-attaching detached entities with client-generated keys #37054
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.
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
Conversation
…generated keys Co-authored-by: AndriySvyryd <[email protected]>
When an entity is transitioned from a tracked state to Detached, it should remain in the _detachedReferenceMap. Previously, the condition prevented this, causing GetOrCreateEntry to create duplicate entries for the same entity instance. Co-authored-by: AndriySvyryd <[email protected]>
When an entity is detached and then re-tracked through navigation fixup, attempting to set the state on the original entry would fail with a duplicate tracking error. This fix detects when the entity is already tracked by a different entry and updates that entry's state instead of trying to track the stale entry. Co-authored-by: AndriySvyryd <[email protected]>
- Improved code comments to better explain the scenario and EF Core's behavior - Removed debug assertion that was failing (this is expected behavior, not an error) Co-authored-by: AndriySvyryd <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a potential issue with entity tracking when reattaching graphs with client-generated keys. The fix prevents duplicate tracking errors when an entity is detached and then re-tracked via navigation fixup (DetectChanges) before manually setting its state.
Key Changes:
- Added detection logic in
InternalEntityEntry.OnStateChangingto check if an entity is already tracked by a different entry - Added a regression test demonstrating the scenario where this issue could occur
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs | Added logic to detect and handle already-tracked entities during state transitions from Detached |
| test/EFCore.Tests/ChangeTracking/GraphTrackingTest.cs | Added test case for reattaching graphs with client-generated keys |
| { | ||
| // The entity is already tracked by a different entry. | ||
| // Update the state of the existing entry instead of trying to track this stale entry. | ||
| existingEntry.SetEntityState(newState); |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The early return at line 216 bypasses the rest of the OnStateChanging flow, which means the subsequent call to StateManager.StartTracking(this) at line 219 is skipped. However, this also means that OnStateChanged (line 229) will never be called for this stale entry. This could result in state change events not being fired, change counters not being updated, and potentially other side effects. Consider whether OnStateChanged or other cleanup should be called before the early return, or if the current entry needs different handling (e.g., should this entry be marked as detached or disposed?).
| existingEntry.SetEntityState(newState); | |
| existingEntry.SetEntityState(newState); | |
| OnStateChanged(newState); |
| private class FooGuid | ||
| { | ||
| public Guid Id { get; set; } | ||
| public string Name { get; set; } |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nullable reference type annotations are missing for reference type properties. According to the coding guidelines, nullable reference types should be used. These properties should either be marked as nullable (e.g., public string? Name { get; set; }) or initialized to prevent potential null reference warnings.
| public string Name { get; set; } | |
| public string Name { get; set; } = string.Empty; |
| public string Name { get; set; } | ||
| public FooGuid Foo { get; set; } | ||
| public Guid FooId { get; set; } | ||
| public BarGuid RelatedBar { get; set; } |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nullable reference type annotations are missing for reference type properties. According to the coding guidelines, nullable reference types should be used. These properties should either be marked as nullable (e.g., public string? Name { get; set; }) or initialized to prevent potential null reference warnings.
| public BarGuid RelatedBar { get; set; } | |
| public BarGuid? RelatedBar { get; set; } |
When entities with client-generated keys (Guid PKs, InMemory provider) are manually detached and re-attached, setting the state on the original entry fails with "Duplicate InternalEntityEntry" if the entity was already re-tracked through navigation fixup.
Changes
StartTracking. If so, forward the state change to the active entry instead of failing.Can_reattach_graph_with_client_generated_keysto validate the scenario.The fix allows user code that manually manages entity state to work correctly when EF Core has already re-tracked entities through navigation fixup.
Original prompt
This section details on the original issue you should resolve
<issue_title>InMemoryProvider causing 'The instance of entity type 'x' cannot be tracked(...)' when manually re-attaching graph</issue_title>
<issue_description>### Bug description
I'm building an extension for EF Core and at some point, I need to detach entities from the db context and then re-attach them manually.
This works, but when migrating to this extension in my existing code base (in which I have hundereds of tests against the in-memory provider), I got a lot of test cases failed at what seemed to be random (e.g. sometimes simply not adding unrelated entity to the db context instance would make the test green). Turns out that the tests fails only when entities are returned from change tracker in a specific order. I know there is no inherent order to those, but below is specific example where there is exception.
Changing pretty much anything (the order in which the entities are added to the db context instance or the order in which they are re-attached) makes the problem go away.
The issue happens only when using the InMemoryProvider (or, to be more specific - doesn't happen when using Sql server provider).
Your code
Stack traces
Verbose output
EF Core version
9.0.9
Database provider
Microsoft.EntityFrameworkCore.InMemory
Target framework
.net 8.0
Operating system
No response
IDE
No response</issue_description>
<agent_instructions>Start by creating a test based on the SqlServer repro in the last comment</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@roji Am not quite sure how the above could be specific to InMemory, as it seems to be doing pure change tracking operations - any thoughts @AndriySvyryd?If it really is specific to InMemory in some way, then note that we're no longer investing in that provider and discourage its use by users - we're therefore be unlikely to investigate and fix this.</comment_new>
<comment_new>@roji
If that's the case, you should be able to reproduce this on SQL Server/SQLite by having Guid PKs - EF generates those on the client by default as well. If you can show that happening on those providers that would make this issue more important.</comment_new>
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.