Skip to content

Commit fd9ddfa

Browse files
barosiakjkotasCopilot
authored
[cDAC] Implement GetPartialUserState for cDAC (#127848)
## Summary Replaces the legacy-delegation stub in DacDbiImpl.GetPartialUserState with a managed implementation using the IThread contract, mirroring the native C++ logic in dacdbiimpl.cpp that maps thread state flags to CorDebugUserState. ## Changes - Rename TS_Interruptible to TS_WaitSleepJoin and remove TSNC_DebuggerSleepWaitJoin from ThreadStateNC across all VM consumers - Add WaitSleepJoin to ThreadState enum (from m_State) - Implement GetPartialUserState in DacDbiImpl with DEBUG cross-validation - Add CorDebugUserState typed enum to IDacDbiInterface - Update data descriptor, contract docs, and VM annotations - Add unit and dump tests --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 10c9a33 commit fd9ddfa

14 files changed

Lines changed: 133 additions & 25 deletions

File tree

docs/design/datacontracts/Thread.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ enum ThreadState
3232
Unstarted = 0x00000400, // Thread has never been started
3333
Stopped = 0x00010000, // Thread has started to shut down
3434
ThreadPoolWorker = 0x01000000, // is this a threadpool worker thread?
35+
WaitSleepJoin = 0x02000000, // Thread is in a Sleep(), Wait(), Join()
3536
Detached = unchecked((int)0x80000000), // Thread was detached
3637
}
3738

src/coreclr/debug/daccess/dacdbiimpl.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5691,13 +5691,7 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetPartialUserState(VMPTR_Thread
56915691
result |= USER_STOPPED;
56925692
}
56935693

5694-
// Don't report Thread::TS_AbortRequested
5695-
5696-
// The interruptible flag is unreliable (see issue 699245)
5697-
// The Debugger_SleepWaitJoin is always accurate when it is present, but it is still
5698-
// just a band-aid fix to cover some of the race conditions interruptible has.
5699-
5700-
if (ts & Thread::TS_Interruptible || pThread->HasThreadStateNC(Thread::TSNC_DebuggerSleepWaitJoin))
5694+
if (ts & Thread::TS_WaitSleepJoin)
57015695
{
57025696
result |= USER_WAIT_SLEEP_JOIN;
57035697
}

src/coreclr/vm/comsynchronizable.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ extern "C" INT32 QCALLTYPE ThreadNative_GetThreadState(QCall::ThreadHandle threa
420420
if (state & Thread::TS_AbortRequested)
421421
res |= ThreadNative::ThreadAbortRequested;
422422

423-
if (state & Thread::TS_Interruptible)
423+
if (state & Thread::TS_WaitSleepJoin)
424424
res |= ThreadNative::ThreadWaitSleepJoin;
425425

426426
return res;
@@ -436,8 +436,7 @@ extern "C" void QCALLTYPE ThreadNative_SetWaitSleepJoinState(QCall::ThreadHandle
436436
CONTRACTL_END;
437437

438438
// Set the state bits.
439-
thread->SetThreadState(Thread::TS_Interruptible);
440-
thread->SetThreadStateNC(Thread::TSNC_DebuggerSleepWaitJoin);
439+
thread->SetThreadState(Thread::TS_WaitSleepJoin);
441440
}
442441

443442
extern "C" void QCALLTYPE ThreadNative_ClearWaitSleepJoinState(QCall::ThreadHandle thread)
@@ -450,8 +449,7 @@ extern "C" void QCALLTYPE ThreadNative_ClearWaitSleepJoinState(QCall::ThreadHand
450449
CONTRACTL_END;
451450

452451
// Clear the state bits.
453-
thread->ResetThreadState(Thread::TS_Interruptible);
454-
thread->ResetThreadStateNC(Thread::TSNC_DebuggerSleepWaitJoin);
452+
thread->ResetThreadState(Thread::TS_WaitSleepJoin);
455453
}
456454

457455
#ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT

src/coreclr/vm/eedbginterfaceimpl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1437,7 +1437,7 @@ CorDebugUserState EEDbgInterfaceImpl::GetPartialUserState(Thread *pThread)
14371437
ret |= (unsigned)USER_STOPPED;
14381438
}
14391439

1440-
if (ts & Thread::TS_Interruptible)
1440+
if (ts & Thread::TS_WaitSleepJoin)
14411441
{
14421442
ret |= (unsigned)USER_WAIT_SLEEP_JOIN;
14431443
}

src/coreclr/vm/threads.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2990,7 +2990,7 @@ void Thread::UserInterrupt(ThreadInterruptMode mode)
29902990
InterlockedOr(&m_UserInterrupt, mode);
29912991

29922992
if (HasValidThreadHandle() &&
2993-
HasThreadState (TS_Interruptible))
2993+
HasThreadState (TS_WaitSleepJoin))
29942994
{
29952995
HANDLE handle = GetThreadHandle();
29962996
if (handle != INVALID_HANDLE_VALUE)
@@ -4461,7 +4461,7 @@ void Thread::HandleThreadInterrupt ()
44614461
}
44624462
if ((m_UserInterrupt & TI_Interrupt) != 0)
44634463
{
4464-
ResetThreadState ((ThreadState)(TS_Interrupted | TS_Interruptible));
4464+
ResetThreadState ((ThreadState)(TS_Interrupted | TS_WaitSleepJoin));
44654465
InterlockedAnd (&m_UserInterrupt, ~TI_Interrupt);
44664466

44674467
COMPlusThrow(kThreadInterruptedException);

src/coreclr/vm/threads.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ class Thread
549549
// unused = 0x00800000,
550550
TS_TPWorkerThread = 0x01000000, // is this a threadpool worker thread? [cDAC] [Thread]: Contract depends on this value.
551551

552-
TS_Interruptible = 0x02000000, // sitting in a Sleep(), Wait(), Join()
552+
TS_WaitSleepJoin = 0x02000000, // sitting in a Sleep(), Wait(), Join(). [cDAC] [Thread]: Contract depends on this value.
553553
TS_Interrupted = 0x04000000, // was awakened by an interrupt APC. !!! This can be moved to TSNC
554554

555555
// unused
@@ -614,9 +614,7 @@ class Thread
614614
//
615615
// Once we are completely independent of the OS UEF, we could remove this.
616616
TSNC_SkipManagedPersonalityRoutine = 0x02000000, // Ignore the ProcessCLRException calls when propagating exception to external native code
617-
TSNC_DebuggerSleepWaitJoin = 0x04000000, // Indicates to the debugger that this thread is in a sleep wait or join state
618-
// This almost mirrors the TS_Interruptible state however that flag can change
619-
// during GC-preemptive mode whereas this one cannot.
617+
// unused = 0x04000000,
620618
// unused = 0x08000000,
621619
TSNC_TSLTakenForStartup = 0x10000000, // The ThreadStoreLock (TSL) is held by another mechanism during
622620
// thread startup so can be skipped.

src/coreclr/vm/threadsuspend.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,7 +1538,7 @@ Thread::UserAbort(EEPolicy::ThreadAbortTypes abortType, DWORD timeout)
15381538

15391539
// If the thread is in sleep, wait, or join interrupt it
15401540
// However, we do NOT want to interrupt if the thread is already processing an exception
1541-
if (m_State & TS_Interruptible)
1541+
if (m_State & TS_WaitSleepJoin)
15421542
{
15431543
UserInterrupt(TI_Abort); // if the user wakes up because of this, it will read the
15441544
// abort requested bit and initiate the abort
@@ -2221,7 +2221,7 @@ void Thread::HandleThreadAbort ()
22212221

22222222
if (ReadyForAbort())
22232223
{
2224-
ResetThreadState ((ThreadState)(TS_Interrupted | TS_Interruptible));
2224+
ResetThreadState ((ThreadState)(TS_Interrupted | TS_WaitSleepJoin));
22252225
// We are going to abort. Abort satisfies Thread.Interrupt requirement.
22262226
InterlockedExchange (&m_UserInterrupt, 0);
22272227

@@ -2267,7 +2267,7 @@ void Thread::PreWorkForThreadAbort()
22672267
SetAbortInitiated();
22682268
// if an abort and interrupt happen at the same time (e.g. on a sleeping thread),
22692269
// the abort is favored. But we do need to reset the interrupt bits.
2270-
ResetThreadState((ThreadState)(TS_Interruptible | TS_Interrupted));
2270+
ResetThreadState((ThreadState)(TS_WaitSleepJoin | TS_Interrupted));
22712271
ResetUserInterrupted();
22722272
}
22732273

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IThread.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public enum ThreadState
3333
Unstarted = 0x00000400, // Thread has never been started
3434
Stopped = 0x00010000, // Thread has started to shut down
3535
ThreadPoolWorker = 0x01000000, // Thread is a thread pool worker thread
36+
WaitSleepJoin = 0x02000000, // Thread is in a Sleep(), Wait(), Join()
3637
Detached = unchecked((int)0x80000000), // Thread was detached
3738
}
3839

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ private enum ThreadState_1
2727
Unstarted = 0x400,
2828
Stopped = 0x10000,
2929
ThreadPoolWorker = 0x1000000,
30+
WaitSleepJoin = 0x2000000,
3031
Detached = unchecked((int)0x80000000)
3132
}
3233

@@ -74,6 +75,8 @@ private static Contracts.ThreadState GetThreadState(ThreadState_1 state)
7475
result |= Contracts.ThreadState.Unstarted;
7576
if (state.HasFlag(ThreadState_1.Stopped))
7677
result |= Contracts.ThreadState.Stopped;
78+
if (state.HasFlag(ThreadState_1.WaitSleepJoin))
79+
result |= Contracts.ThreadState.WaitSleepJoin;
7780
if (state.HasFlag(ThreadState_1.ThreadPoolWorker))
7881
result |= Contracts.ThreadState.ThreadPoolWorker;
7982
if (state.HasFlag(ThreadState_1.Detached))

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -744,8 +744,50 @@ public int HasUnhandledException(ulong vmThread, Interop.BOOL* pResult)
744744
public int GetUserState(ulong vmThread, int* pRetVal)
745745
=> LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetUserState(vmThread, pRetVal) : HResults.E_NOTIMPL;
746746

747-
public int GetPartialUserState(ulong vmThread, int* pRetVal)
748-
=> LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetPartialUserState(vmThread, pRetVal) : HResults.E_NOTIMPL;
747+
public int GetPartialUserState(ulong vmThread, CorDebugUserState* pRetVal)
748+
{
749+
*pRetVal = default;
750+
int hr = HResults.S_OK;
751+
try
752+
{
753+
TargetPointer threadPtr = new TargetPointer(vmThread);
754+
Contracts.ThreadData threadData = _target.Contracts.Thread.GetThreadData(threadPtr);
755+
Contracts.ThreadState threadState = threadData.State;
756+
757+
CorDebugUserState result = default;
758+
if ((threadState & Contracts.ThreadState.Background) != 0)
759+
result |= CorDebugUserState.USER_BACKGROUND;
760+
761+
if ((threadState & Contracts.ThreadState.Unstarted) != 0)
762+
result |= CorDebugUserState.USER_UNSTARTED;
763+
764+
if ((threadState & Contracts.ThreadState.Stopped) != 0)
765+
result |= CorDebugUserState.USER_STOPPED;
766+
767+
if ((threadState & Contracts.ThreadState.WaitSleepJoin) != 0)
768+
result |= CorDebugUserState.USER_WAIT_SLEEP_JOIN;
769+
770+
if ((threadState & Contracts.ThreadState.ThreadPoolWorker) != 0)
771+
result |= CorDebugUserState.USER_THREADPOOL;
772+
773+
*pRetVal = result;
774+
}
775+
catch (System.Exception ex)
776+
{
777+
hr = ex.HResult;
778+
}
779+
#if DEBUG
780+
if (_legacy is not null)
781+
{
782+
CorDebugUserState retValLocal;
783+
int hrLocal = _legacy.GetPartialUserState(vmThread, &retValLocal);
784+
Debug.ValidateHResult(hr, hrLocal);
785+
if (hr == HResults.S_OK)
786+
Debug.Assert(*pRetVal == retValLocal, $"cDAC: {*pRetVal}, DAC: {retValLocal}");
787+
}
788+
#endif
789+
return hr;
790+
}
749791

750792
public int GetConnectionID(ulong vmThread, uint* pRetVal)
751793
{
@@ -2034,4 +2076,5 @@ public int GetAsyncLocals(ulong vmMethod, ulong codeAddr, uint state, nint pAsyn
20342076

20352077
public int GetGenericArgTokenIndex(ulong vmMethod, uint* pIndex)
20362078
=> LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetGenericArgTokenIndex(vmMethod, pIndex) : HResults.E_NOTIMPL;
2079+
20372080
}

0 commit comments

Comments
 (0)