From cc7340cad8bc4d8768540cadb38175d2484e8db9 Mon Sep 17 00:00:00 2001 From: Barbara Rosiak Date: Wed, 3 Jun 2026 17:30:37 -0700 Subject: [PATCH 1/2] Implement GetThreadOwningMonitorLock for cDAC --- .../Dbi/DacDbiImpl.cs | 66 +++++++++++++++++- .../managed/cdac/tests/DacDbiImplTests.cs | 69 +++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 0bcbc3fededf64..c65eb225764faf 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -3131,7 +3131,71 @@ public int GetHandleAddressFromVmHandle(ulong vmHandle, ulong* pRetVal) } public int GetThreadOwningMonitorLock(ulong vmObject, DacDbiMonitorLockInfo* pRetVal) - => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetThreadOwningMonitorLock(vmObject, pRetVal) : HResults.E_NOTIMPL; + { + int hr = HResults.S_OK; + try + { + DacDbiMonitorLockInfo info = default; + info.lockOwner = TargetPointer.Null; + info.acquisitionCount = 0; + + uint threadId = 0; + uint recursionCount = 0; + TargetPointer syncBlock = _target.Contracts.Object.GetSyncBlockAddress(vmObject); + // Note: GetSyncBlockAddress returns TargetPointer.Null when no sync block index is in the header. + // Passing Null to TryGetLockInfo will throw, matching the native DAC. + // Should we add an explicit null guard here instead? + bool isLockHeld = _target.Contracts.SyncBlock.TryGetLockInfo(syncBlock, out threadId, out recursionCount); + + if (!isLockHeld) + { + *pRetVal = info; + } + else + { + var threadStore = _target.Contracts.Thread.GetThreadStoreData(); + TargetPointer threadPtr = threadStore.FirstThread; + while (threadPtr != TargetPointer.Null) + { + ThreadData threadData = _target.Contracts.Thread.GetThreadData(threadPtr); + if (threadData.Id == threadId) + { + info.lockOwner = threadPtr; + info.acquisitionCount = recursionCount + 1; // The runtime tracks recursion count starting at 0, but diagnostics users expect it to start at 1. + break; + } + threadPtr = threadData.NextThread; + } +#if DEBUG + if (threadPtr == TargetPointer.Null) + { + Debug.Assert(false, "A thread should have been found"); + } +#endif + *pRetVal = info; + } + } + catch (System.Exception ex) + { + hr = ex.HResult; + } +#if DEBUG + if (_legacy is not null) + { + DacDbiMonitorLockInfo pRetValLocal; + int hrLocal = _legacy.GetThreadOwningMonitorLock(vmObject, &pRetValLocal); + Debug.ValidateHResult(hr, hrLocal); + if (hr == HResults.S_OK) + { + Debug.Assert(pRetVal->lockOwner == pRetValLocal.lockOwner, + $"lockOwner mismatch: cDAC={pRetVal->lockOwner}, DAC={pRetValLocal.lockOwner}"); + Debug.Assert(pRetVal->acquisitionCount == pRetValLocal.acquisitionCount, + $"acquisitionCount mismatch: cDAC={pRetVal->acquisitionCount}, DAC={pRetValLocal.acquisitionCount}"); + } + } +#endif + return hr; + } public int EnumerateMonitorEventWaitList(ulong vmObject, nint fpCallback, nint pUserData) => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.EnumerateMonitorEventWaitList(vmObject, fpCallback, pUserData) : HResults.E_NOTIMPL; diff --git a/src/native/managed/cdac/tests/DacDbiImplTests.cs b/src/native/managed/cdac/tests/DacDbiImplTests.cs index 5cb4e34d3e46cb..b669d833f4adad 100644 --- a/src/native/managed/cdac/tests/DacDbiImplTests.cs +++ b/src/native/managed/cdac/tests/DacDbiImplTests.cs @@ -719,6 +719,75 @@ public void AreOptimizationsDisabled_WithMethodDesc(MockTarget.Architecture arch Assert.Equal(deoptimized ? Interop.BOOL.TRUE : Interop.BOOL.FALSE, result); } + private delegate void TryGetLockInfoCallback(TargetPointer syncBlock, out uint owningThreadId, out uint recursion); + + public static IEnumerable GetThreadOwningMonitorLockData() + { + foreach (var arch in new MockTarget.StdArch()) + { + yield return new object[] { arch[0], false, 0u, 0ul, 0u }; + yield return new object[] { arch[0], true, 3u, 0x7000ul, 4u }; + } + } + + [Theory] + [MemberData(nameof(GetThreadOwningMonitorLockData))] + public void GetThreadOwningMonitorLock(MockTarget.Architecture arch, bool isLockHeld, uint recursionCount, ulong expectedOwner, uint expectedAcquisitionCount) + { + const ulong ObjectAddr = 0x5000; + const uint OwnerThreadId = 42; + TargetPointer syncBlockAddr = new(0x6000); + TargetPointer ownerThreadPtr = new(0x7000); + TargetPointer otherThreadPtr = new(0x8000); + + var mockObject = new Mock(); + mockObject.Setup(o => o.GetSyncBlockAddress(new TargetPointer(ObjectAddr))) + .Returns(syncBlockAddr); + + var mockSyncBlock = new Mock(); + var lockSetup = mockSyncBlock + .Setup(s => s.TryGetLockInfo(syncBlockAddr, out It.Ref.IsAny, out It.Ref.IsAny)); + if (isLockHeld) + { + lockSetup + .Callback(new TryGetLockInfoCallback((TargetPointer _, out uint threadId, out uint recursion) => + { + threadId = OwnerThreadId; + recursion = recursionCount; + })) + .Returns(true); + } + else + { + lockSetup.Returns(false); + } + + var builder = new TestPlaceholderTarget.Builder(arch) + .UseReader((_, _) => -1) + .AddMockContract(mockObject) + .AddMockContract(mockSyncBlock); + + if (isLockHeld) + { + var mockThread = new Mock(); + mockThread.Setup(t => t.GetThreadStoreData()) + .Returns(new ThreadStoreData(2, otherThreadPtr, TargetPointer.Null, TargetPointer.Null)); + mockThread.Setup(t => t.GetThreadData(otherThreadPtr)) + .Returns(new ThreadData(otherThreadPtr, 99, default, default, default, default, default, default, default, default, default, default, default, default, ownerThreadPtr, default)); + mockThread.Setup(t => t.GetThreadData(ownerThreadPtr)) + .Returns(new ThreadData(ownerThreadPtr, OwnerThreadId, default, default, default, default, default, default, default, default, default, default, default, default, TargetPointer.Null, default)); + builder.AddMockContract(mockThread); + } + + var dacDbi = new DacDbiImpl(builder.Build(), legacyObj: null); + + DacDbiMonitorLockInfo result; + int hr = dacDbi.GetThreadOwningMonitorLock(ObjectAddr, &result); + Assert.Equal(System.HResults.S_OK, hr); + Assert.Equal(expectedOwner, result.lockOwner); + Assert.Equal(expectedAcquisitionCount, result.acquisitionCount); + } + [Theory] [ClassData(typeof(MockTarget.StdArch))] public void AreOptimizationsDisabled_NullMethodDesc_ReturnsFalse(MockTarget.Architecture arch) From d2ca84c24b133f40da35558d947cdba64428ae47 Mon Sep 17 00:00:00 2001 From: Barbara Rosiak Date: Thu, 4 Jun 2026 12:15:16 -0700 Subject: [PATCH 2/2] Address pr comments --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 3 +- .../Dbi/DacDbiImpl.cs | 32 +++------- .../cdac/tests/UnitTests/DacDbiImplTests.cs | 61 +++++++++---------- 3 files changed, 39 insertions(+), 57 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index db9423528f089f..54d31a3cdeb424 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -6052,7 +6052,8 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetThreadOwningMonitorLock(VMPTR_ DWORD threadId = 0; DWORD recursionCount = 0; - BOOL isLockHeld = pObj->GetHeader()->PassiveGetSyncBlock()->TryGetLockInfo(&threadId, &recursionCount); + SyncBlock* psb = pObj->GetHeader()->PassiveGetSyncBlock(); + BOOL isLockHeld = psb != NULL && psb->TryGetLockInfo(&threadId, &recursionCount); if (!isLockHeld) { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index c65eb225764faf..5748401c6b3b70 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -3133,45 +3133,27 @@ public int GetHandleAddressFromVmHandle(ulong vmHandle, ulong* pRetVal) public int GetThreadOwningMonitorLock(ulong vmObject, DacDbiMonitorLockInfo* pRetVal) { int hr = HResults.S_OK; + *pRetVal = default; try { DacDbiMonitorLockInfo info = default; - info.lockOwner = TargetPointer.Null; - info.acquisitionCount = 0; - uint threadId = 0; uint recursionCount = 0; TargetPointer syncBlock = _target.Contracts.Object.GetSyncBlockAddress(vmObject); - // Note: GetSyncBlockAddress returns TargetPointer.Null when no sync block index is in the header. - // Passing Null to TryGetLockInfo will throw, matching the native DAC. - // Should we add an explicit null guard here instead? - bool isLockHeld = _target.Contracts.SyncBlock.TryGetLockInfo(syncBlock, out threadId, out recursionCount); - if (!isLockHeld) + if (syncBlock == TargetPointer.Null || !_target.Contracts.SyncBlock.TryGetLockInfo(syncBlock, out threadId, out recursionCount)) { *pRetVal = info; } else { - var threadStore = _target.Contracts.Thread.GetThreadStoreData(); - TargetPointer threadPtr = threadStore.FirstThread; - while (threadPtr != TargetPointer.Null) - { - ThreadData threadData = _target.Contracts.Thread.GetThreadData(threadPtr); - if (threadData.Id == threadId) - { - info.lockOwner = threadPtr; - info.acquisitionCount = recursionCount + 1; // The runtime tracks recursion count starting at 0, but diagnostics users expect it to start at 1. - break; - } - threadPtr = threadData.NextThread; - } -#if DEBUG - if (threadPtr == TargetPointer.Null) + TargetPointer threadPtr = _target.Contracts.Thread.IdToThread(threadId); + Debug.Assert(threadPtr != TargetPointer.Null, "A thread should have been found"); + if (threadPtr != TargetPointer.Null) { - Debug.Assert(false, "A thread should have been found"); + info.lockOwner = threadPtr; + info.acquisitionCount = recursionCount + 1; // The runtime tracks recursion count starting at 0, but diagnostics users expect it to start at 1. } -#endif *pRetVal = info; } } diff --git a/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs b/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs index 396289f0017257..291e498a3e6c97 100644 --- a/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs @@ -726,57 +726,56 @@ public static IEnumerable GetThreadOwningMonitorLockData() { foreach (var arch in new MockTarget.StdArch()) { - yield return new object[] { arch[0], false, 0u, 0ul, 0u }; - yield return new object[] { arch[0], true, 3u, 0x7000ul, 4u }; + yield return new object[] { arch[0], false, false, 0u, 0ul, 0u }; + yield return new object[] { arch[0], true, false, 0u, 0ul, 0u }; + yield return new object[] { arch[0], true, true, 3u, 0x7000ul, 4u }; } } [Theory] [MemberData(nameof(GetThreadOwningMonitorLockData))] - public void GetThreadOwningMonitorLock(MockTarget.Architecture arch, bool isLockHeld, uint recursionCount, ulong expectedOwner, uint expectedAcquisitionCount) + public void GetThreadOwningMonitorLock(MockTarget.Architecture arch, bool hasSyncBlock, bool isLockHeld, uint recursionCount, ulong expectedOwner, uint expectedAcquisitionCount) { const ulong ObjectAddr = 0x5000; const uint OwnerThreadId = 42; TargetPointer syncBlockAddr = new(0x6000); TargetPointer ownerThreadPtr = new(0x7000); - TargetPointer otherThreadPtr = new(0x8000); var mockObject = new Mock(); mockObject.Setup(o => o.GetSyncBlockAddress(new TargetPointer(ObjectAddr))) - .Returns(syncBlockAddr); - - var mockSyncBlock = new Mock(); - var lockSetup = mockSyncBlock - .Setup(s => s.TryGetLockInfo(syncBlockAddr, out It.Ref.IsAny, out It.Ref.IsAny)); - if (isLockHeld) - { - lockSetup - .Callback(new TryGetLockInfoCallback((TargetPointer _, out uint threadId, out uint recursion) => - { - threadId = OwnerThreadId; - recursion = recursionCount; - })) - .Returns(true); - } - else - { - lockSetup.Returns(false); - } + .Returns(hasSyncBlock ? syncBlockAddr : TargetPointer.Null); var builder = new TestPlaceholderTarget.Builder(arch) .UseReader((_, _) => -1) - .AddMockContract(mockObject) - .AddMockContract(mockSyncBlock); + .AddMockContract(mockObject); + + if (hasSyncBlock) + { + var mockSyncBlock = new Mock(); + var lockSetup = mockSyncBlock + .Setup(s => s.TryGetLockInfo(syncBlockAddr, out It.Ref.IsAny, out It.Ref.IsAny)); + if (isLockHeld) + { + lockSetup + .Callback(new TryGetLockInfoCallback((TargetPointer _, out uint threadId, out uint recursion) => + { + threadId = OwnerThreadId; + recursion = recursionCount; + })) + .Returns(true); + } + else + { + lockSetup.Returns(false); + } + builder.AddMockContract(mockSyncBlock); + } if (isLockHeld) { var mockThread = new Mock(); - mockThread.Setup(t => t.GetThreadStoreData()) - .Returns(new ThreadStoreData(2, otherThreadPtr, TargetPointer.Null, TargetPointer.Null)); - mockThread.Setup(t => t.GetThreadData(otherThreadPtr)) - .Returns(new ThreadData(otherThreadPtr, 99, default, default, default, default, default, default, default, default, default, default, default, default, ownerThreadPtr, default)); - mockThread.Setup(t => t.GetThreadData(ownerThreadPtr)) - .Returns(new ThreadData(ownerThreadPtr, OwnerThreadId, default, default, default, default, default, default, default, default, default, default, default, default, TargetPointer.Null, default)); + mockThread.Setup(t => t.IdToThread(OwnerThreadId)) + .Returns(ownerThreadPtr); builder.AddMockContract(mockThread); }