Skip to content

[Microsoft.Gen.Logging] Clear thread-local state when logging throws - #7682

Merged
evgenyfedorov2 merged 2 commits into
dotnet:mainfrom
Rimobul:lsokolovsky/issue-7634
Aug 6, 2026
Merged

[Microsoft.Gen.Logging] Clear thread-local state when logging throws#7682
evgenyfedorov2 merged 2 commits into
dotnet:mainfrom
Rimobul:lsokolovsky/issue-7634

Conversation

@Rimobul

@Rimobul Rimobul commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #7634

Clear LoggerMessageHelper.ThreadLocalState in a finally block to prevent stale structured fields from leaking into subsequent log calls.

Add regression coverage for throwing ILogger implementations.

Microsoft Reviewers: Open in CodeFlow

Fixes dotnet#7634

Clear LoggerMessageHelper.ThreadLocalState in a finally block to prevent stale structured fields from leaking into subsequent log calls.

Add regression coverage for throwing ILogger implementations.
Copilot AI review requested due to automatic review settings August 6, 2026 08:35
@Rimobul
Rimobul requested a review from a team as a code owner August 6, 2026 08:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Microsoft.Gen.Logging source generator to always clear LoggerMessageHelper.ThreadLocalState even when ILogger.Log throws, preventing stale structured fields from leaking into subsequent log calls on the same thread.

Changes:

  • Wrap generated thread-local state usage in try/finally and clear state in the finally block.
  • Add new generated logging test methods to reproduce the leak scenario.
  • Add a regression test that uses a throwing ILogger and then validates the next log does not observe stale RequestId.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/Generators/Microsoft.Gen.Logging/TestClasses/ArgTestExtensions.cs Adds new [LoggerMessage] methods used by tests to exercise structured state reuse across calls.
test/Generators/Microsoft.Gen.Logging/Generated/LogMethodTests.cs Adds a regression test (throwing logger + capturing logger) to validate state cleanup behavior.
src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs Changes emitted code to clear thread-local state in a finally block.

Comment thread src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs
@evgenyfedorov2
evgenyfedorov2 merged commit 8f264ab into dotnet:main Aug 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Microsoft.Gen.Logging  does not clear  ThreadLocalState  when  ILogger.Log  throws

5 participants