Skip to content

Commit ca4d6d1

Browse files
sdmacleasdmaclea
authored
[release/5.0] Port Debugger data breakpoint deadlock fixes to .NET 5.0 (#44563)
* Add ICorDebugHeapValue4 -- CreatePinnedHandle (#44471) * Add ICorDebugHeapValue4 * Add EnableGCNotificationEvents deprecation comment * Drop support for IID_ICorDebugProcess10 and fix thread suspend logic (#44549) * Stop providing IID_ICorDebugProcess10 Prevent older VS versions from setting managed data breakpoints. * Simplify the thread collision logic to prevent deadlock This is a simplification of #44539 as proposed by @kouvel
1 parent 3700448 commit ca4d6d1

File tree

9 files changed

+5447
-5245
lines changed

9 files changed

+5447
-5245
lines changed

‎src/coreclr/src/debug/di/divalue.cpp‎

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,6 @@ HRESULT CordbValue::InternalCreateHandle(CorDebugHandleType handleType,
421421

422422
DebuggerIPCEvent event;
423423
CordbProcess *process;
424-
BOOL fStrong = FALSE;
425424

426425
// @dbgtodo- , as part of inspection, convert this path to throwing.
427426
if (ppHandle == NULL)
@@ -431,13 +430,14 @@ HRESULT CordbValue::InternalCreateHandle(CorDebugHandleType handleType,
431430

432431
*ppHandle = NULL;
433432

434-
if (handleType == HANDLE_STRONG)
433+
switch (handleType)
435434
{
436-
fStrong = TRUE;
437-
}
438-
else
439-
{
440-
_ASSERTE(handleType == HANDLE_WEAK_TRACK_RESURRECTION);
435+
case HANDLE_STRONG:
436+
case HANDLE_WEAK_TRACK_RESURRECTION:
437+
case HANDLE_PINNED:
438+
break;
439+
default:
440+
return E_INVALIDARG;
441441
}
442442

443443

@@ -460,7 +460,7 @@ HRESULT CordbValue::InternalCreateHandle(CorDebugHandleType handleType,
460460

461461
CORDB_ADDRESS addr = GetValueHome() != NULL ? GetValueHome()->GetAddress() : NULL;
462462
event.CreateHandle.objectToken = CORDB_ADDRESS_TO_PTR(addr);
463-
event.CreateHandle.fStrong = fStrong;
463+
event.CreateHandle.handleType = handleType;
464464

465465
// Note: two-way event here...
466466
HRESULT hr = process->SendIPCEvent(&event, sizeof(DebuggerIPCEvent));
@@ -1827,6 +1827,10 @@ HRESULT CordbObjectValue::QueryInterface(REFIID id, void **pInterface)
18271827
{
18281828
*pInterface = static_cast<ICorDebugHeapValue3*>(this);
18291829
}
1830+
else if (id == IID_ICorDebugHeapValue4)
1831+
{
1832+
*pInterface = static_cast<ICorDebugHeapValue4*>(this);
1833+
}
18301834
else if ((id == IID_ICorDebugStringValue) &&
18311835
(m_info.objTypeData.elementType == ELEMENT_TYPE_STRING))
18321836
{
@@ -1963,6 +1967,21 @@ HRESULT CordbObjectValue::CreateHandle(
19631967
return CordbValue::InternalCreateHandle(handleType, ppHandle);
19641968
} // CreateHandle
19651969

1970+
/*
1971+
* Creates a pinned handle for this heap value.
1972+
*
1973+
* Not Implemented In-Proc.
1974+
*/
1975+
HRESULT CordbObjectValue::CreatePinnedHandle(
1976+
ICorDebugHandleValue ** ppHandle)
1977+
{
1978+
PUBLIC_API_ENTRY(this);
1979+
FAIL_IF_NEUTERED(this);
1980+
ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess());
1981+
1982+
return CordbValue::InternalCreateHandle(HANDLE_PINNED, ppHandle);
1983+
} // CreatePinnedHandle
1984+
19661985
// Get class information for this object
19671986
// Arguments:
19681987
// output: ppClass - ICDClass instance for this object
@@ -3325,6 +3344,10 @@ HRESULT CordbBoxValue::QueryInterface(REFIID id, void **pInterface)
33253344
{
33263345
*pInterface = static_cast<ICorDebugHeapValue3*>(this);
33273346
}
3347+
else if (id == IID_ICorDebugHeapValue4)
3348+
{
3349+
*pInterface = static_cast<ICorDebugHeapValue4*>(this);
3350+
}
33283351
else if (id == IID_IUnknown)
33293352
{
33303353
*pInterface = static_cast<IUnknown*>(static_cast<ICorDebugBoxValue*>(this));
@@ -3387,6 +3410,24 @@ HRESULT CordbBoxValue::CreateHandle(
33873410
return CordbValue::InternalCreateHandle(handleType, ppHandle);
33883411
} // CordbBoxValue::CreateHandle
33893412

3413+
// Creates a pinned handle for this heap value.
3414+
// Not Implemented In-Proc.
3415+
// Create a handle for a heap object.
3416+
// @todo: How to prevent this being called by non-heap object?
3417+
// Arguments:
3418+
// output: ppHandle - on success, the newly created handle
3419+
// Return Value: S_OK on success or E_INVALIDARG, E_OUTOFMEMORY, or CORDB_E_HELPER_MAY_DEADLOCK
3420+
HRESULT CordbBoxValue::CreatePinnedHandle(
3421+
ICorDebugHandleValue ** ppHandle)
3422+
{
3423+
PUBLIC_API_ENTRY(this);
3424+
FAIL_IF_NEUTERED(this);
3425+
ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess());
3426+
3427+
return CordbValue::InternalCreateHandle(HANDLE_PINNED, ppHandle);
3428+
} // CreatePinnedHandle
3429+
3430+
33903431
HRESULT CordbBoxValue::GetValue(void *pTo)
33913432
{
33923433
// Can't get a whole copy of a box.
@@ -3565,6 +3606,10 @@ HRESULT CordbArrayValue::QueryInterface(REFIID id, void **pInterface)
35653606
{
35663607
*pInterface = static_cast<ICorDebugHeapValue3*>(this);
35673608
}
3609+
else if (id == IID_ICorDebugHeapValue4)
3610+
{
3611+
*pInterface = static_cast<ICorDebugHeapValue4*>(this);
3612+
}
35683613
else if (id == IID_IUnknown)
35693614
{
35703615
*pInterface = static_cast<IUnknown*>(static_cast<ICorDebugArrayValue*>(this));
@@ -3888,6 +3933,23 @@ HRESULT CordbArrayValue::CreateHandle(
38883933
return CordbValue::InternalCreateHandle(handleType, ppHandle);
38893934
} // CordbArrayValue::CreateHandle
38903935

3936+
/*
3937+
* Creates a pinned handle for this heap value.
3938+
* Not Implemented In-Proc.
3939+
* Arguments:
3940+
* output: ppHandle - on success, the newly created handle
3941+
* Return Value: S_OK on success or E_INVALIDARG, E_OUTOFMEMORY, or CORDB_E_HELPER_MAY_DEADLOCK
3942+
*/
3943+
HRESULT CordbArrayValue::CreatePinnedHandle(
3944+
ICorDebugHandleValue ** ppHandle)
3945+
{
3946+
PUBLIC_API_ENTRY(this);
3947+
FAIL_IF_NEUTERED(this);
3948+
ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess());
3949+
3950+
return CordbValue::InternalCreateHandle(HANDLE_PINNED, ppHandle);
3951+
} // CreatePinnedHandle
3952+
38913953
// get a copy of the array
38923954
// Arguments
38933955
// output: pTo - pointer to a caller-allocated and managed buffer to hold the copy. The caller must guarantee

‎src/coreclr/src/debug/di/process.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2169,10 +2169,6 @@ HRESULT CordbProcess::QueryInterface(REFIID id, void **pInterface)
21692169
{
21702170
*pInterface = static_cast<ICorDebugProcess8*>(this);
21712171
}
2172-
else if (id == IID_ICorDebugProcess10)
2173-
{
2174-
*pInterface = static_cast<ICorDebugProcess10*>(this);
2175-
}
21762172
else if (id == IID_ICorDebugProcess11)
21772173
{
21782174
*pInterface = static_cast<ICorDebugProcess11*>(this);

‎src/coreclr/src/debug/di/rspriv.h‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2935,7 +2935,6 @@ class CordbProcess :
29352935
public ICorDebugProcess5,
29362936
public ICorDebugProcess7,
29372937
public ICorDebugProcess8,
2938-
public ICorDebugProcess10,
29392938
public ICorDebugProcess11,
29402939
public IDacDbiInterface::IAllocator,
29412940
public IDacDbiInterface::IMetaDataLookup,
@@ -3146,7 +3145,7 @@ class CordbProcess :
31463145
COM_METHOD EnableExceptionCallbacksOutsideOfMyCode(BOOL enableExceptionsOutsideOfJMC);
31473146

31483147
//-----------------------------------------------------------
3149-
// ICorDebugProcess10
3148+
// ICorDebugProcess10 (To be removed in .NET 6, in a separate cleanup PR)
31503149
//-----------------------------------------------------------
31513150
COM_METHOD EnableGCNotificationEvents(BOOL fEnable);
31523151

@@ -9168,6 +9167,7 @@ class CordbObjectValue : public CordbValue,
91689167
public ICorDebugValue3,
91699168
public ICorDebugHeapValue2,
91709169
public ICorDebugHeapValue3,
9170+
public ICorDebugHeapValue4,
91719171
public ICorDebugExceptionObjectValue,
91729172
public ICorDebugComObjectValue,
91739173
public ICorDebugDelegateObjectValue
@@ -9243,6 +9243,11 @@ class CordbObjectValue : public CordbValue,
92439243
COM_METHOD GetThreadOwningMonitorLock(ICorDebugThread **ppThread, DWORD *pAcquisitionCount);
92449244
COM_METHOD GetMonitorEventWaitList(ICorDebugThreadEnum **ppThreadEnum);
92459245

9246+
//-----------------------------------------------------------
9247+
// ICorDebugHeapValue4
9248+
//-----------------------------------------------------------
9249+
COM_METHOD CreatePinnedHandle(ICorDebugHandleValue ** ppHandle);
9250+
92469251
//-----------------------------------------------------------
92479252
// ICorDebugObjectValue
92489253
//-----------------------------------------------------------
@@ -9492,7 +9497,8 @@ class CordbBoxValue : public CordbValue,
94929497
public ICorDebugValue2,
94939498
public ICorDebugValue3,
94949499
public ICorDebugHeapValue2,
9495-
public ICorDebugHeapValue3
9500+
public ICorDebugHeapValue3,
9501+
public ICorDebugHeapValue4
94969502
{
94979503
public:
94989504
CordbBoxValue(CordbAppDomain * appdomain,
@@ -9582,6 +9588,11 @@ class CordbBoxValue : public CordbValue,
95829588
COM_METHOD GetThreadOwningMonitorLock(ICorDebugThread **ppThread, DWORD *pAcquisitionCount);
95839589
COM_METHOD GetMonitorEventWaitList(ICorDebugThreadEnum **ppThreadEnum);
95849590

9591+
//-----------------------------------------------------------
9592+
// ICorDebugHeapValue4
9593+
//-----------------------------------------------------------
9594+
COM_METHOD CreatePinnedHandle(ICorDebugHandleValue ** ppHandle);
9595+
95859596
//-----------------------------------------------------------
95869597
// ICorDebugGenericValue
95879598
//-----------------------------------------------------------
@@ -9620,7 +9631,8 @@ class CordbArrayValue : public CordbValue,
96209631
public ICorDebugValue2,
96219632
public ICorDebugValue3,
96229633
public ICorDebugHeapValue2,
9623-
public ICorDebugHeapValue3
9634+
public ICorDebugHeapValue3,
9635+
public ICorDebugHeapValue4
96249636
{
96259637
public:
96269638
CordbArrayValue(CordbAppDomain * appdomain,
@@ -9706,6 +9718,11 @@ class CordbArrayValue : public CordbValue,
97069718
COM_METHOD GetThreadOwningMonitorLock(ICorDebugThread **ppThread, DWORD *pAcquisitionCount);
97079719
COM_METHOD GetMonitorEventWaitList(ICorDebugThreadEnum **ppThreadEnum);
97089720

9721+
//-----------------------------------------------------------
9722+
// ICorDebugHeapValue4
9723+
//-----------------------------------------------------------
9724+
COM_METHOD CreatePinnedHandle(ICorDebugHandleValue ** ppHandle);
9725+
97099726
//-----------------------------------------------------------
97109727
// ICorDebugArrayValue
97119728
//-----------------------------------------------------------

‎src/coreclr/src/debug/ee/debugger.cpp‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11369,7 +11369,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
1136911369
Object * pObject = (Object*)pEvent->CreateHandle.objectToken;
1137011370
OBJECTREF objref = ObjectToOBJECTREF(pObject);
1137111371
AppDomain * pAppDomain = pEvent->vmAppDomain.GetRawPtr();
11372-
BOOL fStrong = pEvent->CreateHandle.fStrong;
11372+
CorDebugHandleType handleType = pEvent->CreateHandle.handleType;
1137311373
OBJECTHANDLE objectHandle;
1137411374

1137511375
// This is a synchronous event (reply required)
@@ -11385,17 +11385,27 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
1138511385

1138611386
if (SUCCEEDED(pEvent->hr))
1138711387
{
11388-
if (fStrong == TRUE)
11389-
{
11390-
// create strong handle
11391-
objectHandle = pAppDomain->CreateStrongHandle(objref);
11392-
}
11393-
else
11394-
{
11388+
switch (handleType)
11389+
{
11390+
case HANDLE_STRONG:
11391+
// create strong handle
11392+
objectHandle = pAppDomain->CreateStrongHandle(objref);
11393+
break;
11394+
case HANDLE_WEAK_TRACK_RESURRECTION:
1139511395
// create the weak long handle
1139611396
objectHandle = pAppDomain->CreateLongWeakHandle(objref);
11397-
}
11398-
pEvent->CreateHandleResult.vmObjectHandle.SetRawPtr(objectHandle);
11397+
break;
11398+
case HANDLE_PINNED:
11399+
// create pinning handle
11400+
objectHandle = pAppDomain->CreatePinningHandle(objref);
11401+
break;
11402+
default:
11403+
pEvent->hr = E_INVALIDARG;
11404+
}
11405+
}
11406+
if (SUCCEEDED(pEvent->hr))
11407+
{
11408+
pEvent->CreateHandleResult.vmObjectHandle.SetRawPtr(objectHandle);
1139911409
}
1140011410
}
1140111411

‎src/coreclr/src/debug/inc/dbgipcevents.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2297,8 +2297,8 @@ struct MSLAYOUT DebuggerIPCEvent
22972297

22982298
struct MSLAYOUT
22992299
{
2300-
void *objectToken;
2301-
BOOL fStrong;
2300+
void *objectToken;
2301+
CorDebugHandleType handleType;
23022302
} CreateHandle;
23032303

23042304
struct MSLAYOUT

‎src/coreclr/src/inc/cordebug.idl‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ interface ICorDebugReferenceValue;
159159
interface ICorDebugHeapValue;
160160
interface ICorDebugHeapValue2;
161161
interface ICorDebugHeapValue3;
162+
interface ICorDebugHeapValue4;
162163
interface ICorDebugHandleValue;
163164
interface ICorDebugObjectValue;
164165
interface ICorDebugStringValue;
@@ -1603,13 +1604,15 @@ typedef enum CorDebugCreateProcessFlags
16031604

16041605

16051606
/* ICorDebugHeapValue::CreateHandle takes a handle flavor.
1606-
* A strong handle will keep an object alive while a weak track resurrection
1607-
* will not.
1607+
* - A strong handle will keep an object alive while allowing GC relocation
1608+
* - A weak handle will not keep an object alive
1609+
* - A pinned handle will keep an object alive and disallow GC relocation
16081610
*/
16091611
typedef enum CorDebugHandleType
16101612
{
16111613
HANDLE_STRONG = 1,
1612-
HANDLE_WEAK_TRACK_RESURRECTION = 2
1614+
HANDLE_WEAK_TRACK_RESURRECTION = 2,
1615+
HANDLE_PINNED = 3
16131616
} CorDebugHandleType;
16141617

16151618
#pragma warning(push)
@@ -3304,6 +3307,9 @@ interface ICorDebugProcess10 : IUnknown
33043307
// Enable or disable the GC notification events. The GC notification events are turned off by default
33053308
// They will be delivered through ICorDebugManagedCallback4
33063309
//
3310+
// This interface is deprecated. The EnableGCNotificationEvents(true) occasionally deadlocked debug sessions
3311+
// in .NET Core 5.0 and later. Please use the IID_ICorDebugHeapValue4 to pin an object and prevent its relocation
3312+
//
33073313
// Parameters
33083314
// fEnable - true to enable the events, false to disable
33093315
//
@@ -6472,6 +6478,27 @@ interface ICorDebugHeapValue3 : IUnknown
64726478
HRESULT GetMonitorEventWaitList([out] ICorDebugThreadEnum **ppThreadEnum);
64736479
};
64746480

6481+
/*
6482+
* ICorDebugHeapValue4
6483+
*/
6484+
6485+
[
6486+
object,
6487+
local,
6488+
uuid(B35DD495-A555-463B-9BE9-C55338486BB8),
6489+
pointer_default(unique)
6490+
]
6491+
interface ICorDebugHeapValue4 : IUnknown
6492+
{
6493+
6494+
/*
6495+
* Creates a handle of the given type for this heap value.
6496+
*
6497+
*/
6498+
HRESULT CreatePinnedHandle([out] ICorDebugHandleValue ** ppHandle);
6499+
6500+
};
6501+
64756502
/*
64766503
* ICorDebugObjectValue is a subclass of ICorDebugValue which applies to
64776504
* values which contain an object.

‎src/coreclr/src/pal/prebuilt/idl/cordebug_i.cpp‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,11 @@
66

77

88
/* File created by MIDL compiler version 8.01.0622 */
9-
/* at Mon Jan 18 19:14:07 2038
10-
*/
11-
/* Compiler settings for runtime/src/coreclr/src/inc/cordebug.idl:
12-
Oicf, W1, Zp8, env=Win32 (32b run), target_arch=X86 8.01.0622
9+
/* Compiler settings for cordebug.idl:
10+
Oicf, W1, Zp8, env=Win32 (32b run), target_arch=X86 8.01.0622
1311
protocol : dce , ms_ext, c_ext, robust
14-
error checks: allocation ref bounds_check enum stub_data
15-
VC __declspec() decoration level:
12+
error checks: allocation ref bounds_check enum stub_data
13+
VC __declspec() decoration level:
1614
__declspec(uuid()), __declspec(selectany), __declspec(novtable)
1715
DECLSPEC_UUID(), MIDL_INTERFACE()
1816
*/
@@ -23,7 +21,7 @@
2321

2422
#ifdef __cplusplus
2523
extern "C"{
26-
#endif
24+
#endif
2725

2826

2927
#include <rpc.h>
@@ -367,6 +365,9 @@ MIDL_DEFINE_GUID(IID, IID_ICorDebugHeapValue2,0xE3AC4D6C,0x9CB7,0x43e6,0x96,0xCC
367365
MIDL_DEFINE_GUID(IID, IID_ICorDebugHeapValue3,0xA69ACAD8,0x2374,0x46e9,0x9F,0xF8,0xB1,0xF1,0x41,0x20,0xD2,0x96);
368366

369367

368+
MIDL_DEFINE_GUID(IID, IID_ICorDebugHeapValue4,0xB35DD495,0xA555,0x463B,0x9B,0xE9,0xC5,0x53,0x38,0x48,0x6B,0xB8);
369+
370+
370371
MIDL_DEFINE_GUID(IID, IID_ICorDebugObjectValue,0x18AD3D6E,0xB7D2,0x11d2,0xBD,0x04,0x00,0x00,0xF8,0x08,0x49,0xBD);
371372

372373

@@ -479,3 +480,6 @@ MIDL_DEFINE_GUID(CLSID, CLSID_EmbeddedCLRCorDebug,0x211f1254,0xbc7e,0x4af5,0xb9,
479480
#ifdef __cplusplus
480481
}
481482
#endif
483+
484+
485+

0 commit comments

Comments
 (0)