Skip to content

[vs18.5] Fix race condition in LoggingService.ProcessLoggingEvent after shutdown#13549

Merged
SimaTian merged 3 commits intovs18.5from
backport/pr-13450-to-vs18.5
Apr 17, 2026
Merged

[vs18.5] Fix race condition in LoggingService.ProcessLoggingEvent after shutdown#13549
SimaTian merged 3 commits intovs18.5from
backport/pr-13450-to-vs18.5

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Backport of #13450 to vs18.5

/cc @rainersigwald @YuliiaKovalova

YuliiaKovalova and others added 3 commits April 15, 2026 22:11
ProcessLoggingEvent accessed _eventQueue, _dequeueEvent, and _enqueueEvent
directly without checking if the service had been shut down. When an external
callback (e.g., Process.Exited from a ProjectCachePlugin) attempted to log
after ShutdownComponent() had nullified these fields, a NullReferenceException
(access violation 0xC0000005) would occur.

The fix:
1. Early return when _serviceState == LoggingServiceState.Shutdown, matching
   the guard pattern already used by RegisterLogger, RegisterDistributedLogger,
   InitializeNodeLoggers, and InitializeComponent.
2. Capture field references to local variables before use, matching the
   existing pattern in LoggingEventProc (the queue pump thread).
3. Null-check the captured locals as a second defense against the race
   between the _serviceState check and field reads during concurrent shutdown.
…tion, Shouldly consistency

- Wrap wait-handle operations (WaitOne/Set) in try/catch for
  ObjectDisposedException, since shutdown can dispose handles after they
  are captured into locals but before they are used.
- Assert logThread.Join result to prevent silent test hangs.
- Switch Assert.Equal to Shouldly ShouldBe for consistency with the rest
  of the test file.
@rainersigwald rainersigwald requested a review from a team as a code owner April 15, 2026 22:13
@SimaTian SimaTian merged commit dc3c0fb into vs18.5 Apr 17, 2026
11 checks passed
@SimaTian SimaTian deleted the backport/pr-13450-to-vs18.5 branch April 17, 2026 07:38
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.

4 participants