Skip to content

Fix race condition monitor thread in RegistryMonitor#1604

Merged
gregg-miskelly merged 8 commits into
microsoft:mainfrom
SachinM123:threadRaceFix
Jul 2, 2026
Merged

Fix race condition monitor thread in RegistryMonitor#1604
gregg-miskelly merged 8 commits into
microsoft:mainfrom
SachinM123:threadRaceFix

Conversation

@SachinM123

@SachinM123 SachinM123 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes a race condition in Microsoft.DebugEngineHost.RegisteryMonitor where if Stop was called before the monitor thread started, it would never shutdown.

This fixes #1593

Add locking to Start and Monitor methods to prevent race conditions with _stoppedEvent. Log monitor thread lifecycle events for better diagnostics. Ensure proper disposal of resources under lock.
@SachinM123

Copy link
Copy Markdown
Collaborator Author

@microsoft-github-policy-service agree

@gregg-miskelly gregg-miskelly left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few small things to fix

Comment thread src/DebugEngineHost/RegistryMonitor.cs Outdated
Comment thread src/DebugEngineHost/RegistryMonitor.cs Outdated
Comment thread src/DebugEngineHost/RegistryMonitor.cs Outdated
Comment thread src/DebugEngineHost/RegistryMonitor.cs Outdated
Comment thread src/DebugEngineHost/RegistryMonitor.cs Outdated
…tAny(new WaitHandle[] { stoppedEvent!, registryChangedEvent });', remove unnessary null check
…ndle.WaitAny(new WaitHandle[] { stoppedEvent!, registryChangedEvent });', remove unnessary null check"

This reverts commit 87affe8.
Remove redundant _stoppedEvent initialization, assuming it is set before use. Simplify WaitHandle.WaitAny call by removing unnecessary null-forgiving operator, relying on prior null check.
Comment thread src/DebugEngineHost/RegistryMonitor.cs Outdated
@gregg-miskelly gregg-miskelly requested a review from WardenGnaw July 2, 2026 05:46
@gregg-miskelly

Copy link
Copy Markdown
Member

@WardenGnaw Do you want to have a quick look before I merge this?

@gregg-miskelly gregg-miskelly merged commit a5d90d8 into microsoft:main Jul 2, 2026
6 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.

Race condition in src/DebugEngineHost/RegistryMonitor.cs could leave thread running

3 participants