[rc2] Resolve query logger from QueryContext in shaper generation #36758
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of closing over the current scoped logger, causing a leak. Closes dotnet#36464
roji
changed the title
cincuranet
approved these changes
Sep 12, 2025
AndriySvyryd
approved these changes
Sep 12, 2025
artl93
approved these changes
Sep 12, 2025
roji
added a commit
to roji/efcore
that referenced
this pull request
Sep 14, 2025
…6758) Instead of closing over the current scoped logger, causing a leak. Closes dotnet#36464 (cherry picked from commit b7996be)
roji
added a commit
to roji/efcore
that referenced
this pull request
Sep 14, 2025
…6758) Instead of closing over the current scoped logger, causing a leak. Closes dotnet#36464 (cherry picked from commit b7996be)
roji
added a commit
to roji/efcore
that referenced
this pull request
Sep 14, 2025
…6758) Instead of closing over the current scoped logger, causing a leak. Closes dotnet#36464 (cherry picked from commit b7996be)
roji
added a commit
that referenced
this pull request
Sep 15, 2025
roji
added a commit
that referenced
this pull request
Sep 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of closing over the current scoped logger, causing a leak.
Closes #36464
Description
When generating the shaper (code which reads back results from the database), the current logic wrongly captures the current query logger (which is a scoped service); this means that whatever logger is in use when the query is first compiled gets referenced from the shaper, and stored in our singleton query cache potentially forever (or until evicted). The logger has references ultimately leading to the EF change tracker and through it, meaning that these user objects are kept alive forever (a form of leak).
Customer impact
With specific usage, arbitrary large user objects may be kept alive forever, and never garbage-collected. In addition, the wrong query logger instance may get used (the one from when the query was first compiled, instead of the current one for the query being executed), which could lead to incorrect logging.
We have performed patch servicing for this kind of bug before (because of the leak potential).
How found
Customer report
Regression
Yes, from EF 7 to EF 8; introduced in #31100.
Testing
This is unfortunately difficult/possible to test automatically, as it's about an unwanted reference in the shaper keeping objects alive. Tested manually to ensure that the shaper no longer contains the reference.
Risk
Low. The change is simple, and we have ample code coverage for the affected codepaths.