Skip to content

Drain ConsoleLogProcessor on Dispose#85162

Merged
tarekgh merged 1 commit intodotnet:mainfrom
JanKrivanek:bugfix/flush-console-logger-on-dispose
Apr 22, 2023
Merged

Drain ConsoleLogProcessor on Dispose#85162
tarekgh merged 1 commit intodotnet:mainfrom
JanKrivanek:bugfix/flush-console-logger-on-dispose

Conversation

@JanKrivanek
Copy link
Member

@JanKrivanek JanKrivanek commented Apr 21, 2023 •
edited
Loading

Fixes #79812 and #71242

Context

ConsoleLoggerProcessor was aburptly skipping all queued messages on Dispose. This is a regression introduced by #70186
Current change allows ConsoleLogProcessor to drain its queue on Dispose (similarly as it was pre-net7)

Testing

Added tailored test quickly issuing multiple messages - some of those would get lost with previuos behavior
This as well fixed previously disabled flaky test

@ghost ghost assigned JanKrivanek Apr 21, 2023
@ghost
Copy link

ghost commented Apr 21, 2023

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #79812 and #71242

Context

ConsoleLoggerProcessor was aburptly skipping all queued messages on Dispose. This is a regression introduced by #70186
Current change allows ConsoleLogProcessor to drain its queue on Dispose (similarly as it was pre-net7)

Testing

Added tailored test quickly issuing multiple messages - some of those would get lost with previos behavior
This as well fixed previously disabled flaky test

Author: JanKrivanek
Assignees: -
Labels:

area-Extensions-Logging
Milestone: -

@build-analysis build-analysis bot mentioned this pull request Apr 21, 2023
Closed
@tarekgh tarekgh added this to the 8.0.0 milestone Apr 22, 2023
@tarekgh
Copy link
Member

tarekgh commented Apr 22, 2023

Thanks @JanKrivanek for fixing this.

@tarekgh tarekgh merged commit 23d69cb into dotnet:main Apr 22, 2023
@tarekgh tarekgh mentioned this pull request Apr 22, 2023
Closed
@JanKrivanek JanKrivanek deleted the bugfix/flush-console-logger-on-dispose branch April 24, 2023 06:10
@hicksjacobp
Copy link

hicksjacobp commented Apr 24, 2023

While this was fixed in main, is there any plan to release NuGet package updates for older versions (7.0)?

@tarekgh
Copy link
Member

tarekgh commented Apr 24, 2023

Currently there is no plan to port this fix to .NET 7.0. We need to exercise the fix in main for a while to get some confidence there is no side effect of this fix and watch to see if there is a demand on this fix to consider porting it. You'll have the option to reference the .NET 8.0 preview package when it is available in the .NET 7.0 apps.

@ghost ghost locked as resolved and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tarekgh tarekgh tarekgh approved these changes

Assignees

@JanKrivanek JanKrivanek

Labels

area-Extensions-Logging

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

ConsoleLogger Queue optimization can lead to missed log entries

3 participants

@JanKrivanek @tarekgh @hicksjacobp