Skip to content

[cDAC] Implement GetThreadOwningMonitorLock for cDAC#128966

Merged
barosiak merged 3 commits into
dotnet:mainfrom
barosiak:barosiak/GetThreadOwningMonitorLock
Jun 5, 2026
Merged

[cDAC] Implement GetThreadOwningMonitorLock for cDAC#128966
barosiak merged 3 commits into
dotnet:mainfrom
barosiak:barosiak/GetThreadOwningMonitorLock

Conversation

@barosiak
Copy link
Copy Markdown
Member

@barosiak barosiak commented Jun 4, 2026

Summary

Implement GetThreadOwningMonitorLock in DacDbiImpl.

Changes

  • DacDbiImpl.cs - Implement GetThreadOwningMonitorLock with #if DEBUG legacy validation.
  • DacDbiImplTests.cs - Add 8 tests for lock-not-held and lock-held scenarios.

@barosiak barosiak self-assigned this Jun 4, 2026
Copilot AI review requested due to automatic review settings June 4, 2026 00:39
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements the IDacDbiInterface.GetThreadOwningMonitorLock path in the managed cDAC (DacDbiImpl) and adds xUnit coverage for “lock held” vs “lock not held” scenarios using mocked IObject/ISyncBlock/IThread contracts.

Changes:

  • Implement GetThreadOwningMonitorLock by reading sync-block lock info, then resolving the owning Thread by ThreadData.Id, with #if DEBUG parity validation against the legacy DAC implementation.
  • Add a new [Theory] to validate returned DacDbiMonitorLockInfo.lockOwner and acquisitionCount for held/unheld cases across standard mock architectures.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Adds the cDAC implementation of GetThreadOwningMonitorLock with legacy parity validation under #if DEBUG.
src/native/managed/cdac/tests/DacDbiImplTests.cs Adds xUnit coverage for lock-held vs lock-not-held behavior across architectures using mocked contracts.

Comment thread src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs Outdated
Copy link
Copy Markdown
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM assuming we fix that pre-existing DAC bug.

Copilot AI review requested due to automatic review settings June 4, 2026 19:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

LGTM!

@barosiak
Copy link
Copy Markdown
Member Author

barosiak commented Jun 5, 2026

/ba-g unrelated failures, timeouts

@barosiak barosiak merged commit 56df10e into dotnet:main Jun 5, 2026
120 of 125 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants