Remove "Host Breakable" lock concept

docs/design/coreclr/botr/profilability.md

@@ -67,9 +67,6 @@ Each and every callback wrapper must have some common gunk at the top.  Here's a
 
         // Yay!
         CAN_TAKE_LOCK;
-
-        // Yay!
-        ASSERT_NO_EE_LOCKS_HELD();
     }
     CONTRACTL_END;
     CLR_TO_PROFILER_ENTRYPOINT((LF_CORPROF,

src/coreclr/inc/clrinternal.idl

@@ -61,7 +61,7 @@ typedef enum {
     CRST_UNSAFE_COOPGC    = 0x4,  // AVOID THIS! Lock must be taken in cooperative mode.
     CRST_UNSAFE_ANYMODE   = 0x8,  // AVOID THIS! Lock can be taken in either GC mode.
     CRST_DEBUGGER_THREAD  = 0x10, // This lock can be taken on the debugger's helper thread.
-    CRST_HOST_BREAKABLE   = 0x20, // This lock is held while running managed code.  It can be terminated by a host.
+    // CRST_UNUSED        = 0x20,
     // CRST_UNUSED        = 0x40,
     CRST_TAKEN_DURING_SHUTDOWN = 0x80, // This lock is taken during the shutdown sequence in EEShutdown(helper)
     CRST_GC_NOTRIGGER_WHEN_TAKEN = 0x100,

src/coreclr/inc/contract.h

@@ -297,15 +297,9 @@ struct TakenLockInfo
 
 enum DbgStateLockType
 {
-    // EE locks (used to sync EE structures).  These do not include
-    // CRST_HOST_BREAKABLE Crsts, and are thus not held while managed
-    // code runs
+    // EE locks (used to sync EE structures).
     kDbgStateLockType_EE,
 
-    // CRST_HOST_BREAKABLE Crsts.  These can be held while arbitrary
-    // managed code runs.
-    kDbgStateLockType_HostBreakableCrst,
-
     // User locks (e.g., Monitor.Enter, ReaderWriterLock class)
     kDbgStateLockType_User,
 
@@ -1963,10 +1957,6 @@ inline ClrDebugState *GetClrDebugState(BOOL fAlloc)
     LOCK_TAKEN_MULTIPLE(kDbgStateLockType_EE, 1, pvLock)
 #define EE_LOCK_RELEASED(pvLock)                \
     LOCK_RELEASED_MULTIPLE(kDbgStateLockType_EE, 1, pvLock)
-#define HOST_BREAKABLE_CRST_TAKEN(pvLock)       \
-    LOCK_TAKEN_MULTIPLE(kDbgStateLockType_HostBreakableCrst, 1, pvLock)
-#define HOST_BREAKABLE_CRST_RELEASED(pvLock)    \
-    LOCK_RELEASED_MULTIPLE(kDbgStateLockType_HostBreakableCrst, 1, pvLock)
 #define USER_LOCK_TAKEN(pvLock)                 \
     LOCK_TAKEN_MULTIPLE(kDbgStateLockType_User, 1, pvLock)
 #define USER_LOCK_RELEASED(pvLock)              \
@@ -1978,8 +1968,6 @@ inline ClrDebugState *GetClrDebugState(BOOL fAlloc)
 #define LOCK_RELEASED_MULTIPLE(dbgStateLockType, cExits, pvLock)
 #define EE_LOCK_TAKEN(pvLock)
 #define EE_LOCK_RELEASED(pvLock)
-#define HOST_BREAKABLE_CRST_TAKEN(pvLock)
-#define HOST_BREAKABLE_CRST_RELEASED(pvLock)
 #define USER_LOCK_TAKEN(pvLock)
 #define USER_LOCK_RELEASED(pvLock)
 

src/coreclr/inc/contract.inl

@@ -417,9 +417,9 @@ inline UINT DbgStateLockData::GetCombinedLockCount()
 {
     // If this fires, the set of lock types must have changed.  You'll need to
     // fix the sum below to include all lock types
-    _ASSERTE(kDbgStateLockType_Count == 3);
+    _ASSERTE(kDbgStateLockType_Count == 2);
 
-    return m_rgcLocksTaken[0] + m_rgcLocksTaken[1] + m_rgcLocksTaken[2];
+    return m_rgcLocksTaken[0] + m_rgcLocksTaken[1];
 }
 
 inline void DbgStateLockState::SetStartingValues()
@@ -619,15 +619,12 @@ inline UINT GetDbgStateLockCount(DbgStateLockType dbgStateLockType)
 
 #define ASSERT_NO_USER_LOCKS_HELD()   \
     _ASSERTE(GetDbgStateLockCount(kDbgStateLockType_User) == 0)
-#define ASSERT_NO_HOST_BREAKABLE_CRSTS_HELD()   \
-    _ASSERTE(GetDbgStateLockCount(kDbgStateLockType_HostBreakableCrst) == 0)
 #define ASSERT_NO_EE_LOCKS_HELD()   \
     _ASSERTE(GetDbgStateLockCount(kDbgStateLockType_EE) == 0)
 
 #else  // ENABLE_CONTRACTS_IMPL
 
 #define ASSERT_NO_USER_LOCKS_HELD()
-#define ASSERT_NO_HOST_BREAKABLE_CRSTS_HELD()
 #define ASSERT_NO_EE_LOCKS_HELD()
 
 #endif  // ENABLE_CONTRACTS_IMPL

src/coreclr/pal/prebuilt/inc/clrinternal.h

@@ -73,7 +73,6 @@ enum __MIDL___MIDL_itf_clrinternal_0000_0000_0001
         CRST_UNSAFE_COOPGC	= 0x4,
         CRST_UNSAFE_ANYMODE	= 0x8,
         CRST_DEBUGGER_THREAD	= 0x10,
-        CRST_HOST_BREAKABLE	= 0x20,
         CRST_TAKEN_DURING_SHUTDOWN	= 0x80,
         CRST_GC_NOTRIGGER_WHEN_TAKEN	= 0x100,
         CRST_DEBUG_ONLY_CHECK_FORBID_SUSPEND_THREAD	= 0x200

src/coreclr/vm/appdomain.cpp

@@ -1675,12 +1675,12 @@ void AppDomain::Init()
     //   involves other locks that arent part of this special deadlock-breaking semantics, then
     //   we continue to block.
     //
-    m_JITLock.Init(CrstJit, CrstFlags(CRST_REENTRANCY | CRST_UNSAFE_SAMELEVEL), TRUE);
-    m_ClassInitLock.Init(CrstClassInit, CrstFlags(CRST_REENTRANCY | CRST_UNSAFE_SAMELEVEL), TRUE);
-    m_ILStubGenLock.Init(CrstILStubGen, CrstFlags(CRST_REENTRANCY), TRUE);
-    m_NativeTypeLoadLock.Init(CrstInteropData, CrstFlags(CRST_REENTRANCY), TRUE);
+    m_JITLock.Init(CrstJit, CrstFlags(CRST_REENTRANCY | CRST_UNSAFE_SAMELEVEL));
+    m_ClassInitLock.Init(CrstClassInit, CrstFlags(CRST_REENTRANCY | CRST_UNSAFE_SAMELEVEL));
+    m_ILStubGenLock.Init(CrstILStubGen, CrstFlags(CRST_REENTRANCY));
+    m_NativeTypeLoadLock.Init(CrstInteropData, CrstFlags(CRST_REENTRANCY));
     m_crstGenericDictionaryExpansionLock.Init(CrstGenericDictionaryExpansion);
-    m_FileLoadLock.Init(CrstAssemblyLoader, CrstFlags(CRST_HOST_BREAKABLE), TRUE);
+    m_FileLoadLock.Init(CrstAssemblyLoader, CrstFlags(CRST_DEFAULT));
     m_DomainCacheCrst.Init(CrstAppDomainCache);
 
     // Has to switch thread to GC_NOTRIGGER while being held

src/coreclr/vm/callhelpers.cpp

@@ -96,9 +96,8 @@ void CallDescrWorker(CallDescrData * pCallDescrData)
     static_assert_no_msg(sizeof(curThread->dangerousObjRefs) == sizeof(ObjRefTable));
     memcpy(ObjRefTable, curThread->dangerousObjRefs, sizeof(ObjRefTable));
 
-    // If the current thread owns spinlock or unbreakable lock, it cannot call managed code.
-    _ASSERTE(!curThread->HasUnbreakableLock() &&
-             (curThread->m_StateNC & Thread::TSNC_OwnsSpinLock) == 0);
+    // If the current thread owns spinlock it cannot call managed code.
+    _ASSERTE((curThread->m_StateNC & Thread::TSNC_OwnsSpinLock) == 0);
 
 #ifdef TARGET_ARM
     _ASSERTE(IsThumbCode(pCallDescrData->pTarget));

src/coreclr/vm/comcallablewrapper.cpp

@@ -3862,7 +3862,7 @@ void ComCallWrapperTemplate::Init()
 {
     WRAPPER_NO_CONTRACT;
 
-    g_CreateWrapperTemplateCrst.Init(CrstWrapperTemplate, (CrstFlags)(CRST_REENTRANCY | CRST_HOST_BREAKABLE));
+    g_CreateWrapperTemplateCrst.Init(CrstWrapperTemplate, (CrstFlags)(CRST_REENTRANCY));
 }
 
 ComCallWrapperTemplate::ComCallWrapperTemplate()

src/coreclr/vm/crst.cpp

@@ -66,10 +66,6 @@ void CrstBase::Destroy()
     _ASSERTE(holderthreadid.IsUnknown() || IsAtProcessExit() || g_fEEShutDown);
 #endif
 
-    // If a lock is host breakable, a host is required to block the release call until
-    // deadlock detection is finished.
-    GCPreemp __gcHolder((m_dwFlags & CRST_HOST_BREAKABLE) == CRST_HOST_BREAKABLE);
-
     minipal_mutex_destroy(&m_lock._mtx);
 
     LOG((LF_SYNC, INFO3, "CrstBase::Destroy %p\n", this));
@@ -154,15 +150,6 @@ void CrstBase::Enter(INDEBUG(NoLevelCheckFlag noLevelCheckFlag/* = CRST_LEVEL_CH
     //
     // What's worse, the implied contract differs for different flavors of crst.
     //
-    // THROWS/FAULT
-    //
-    //     A crst can be HOST_BREAKBALE or not. A HOST_BREAKABLE crst can throw on an attempt to enter
-    //     (due to deadlock breaking by the host.) A non-breakable crst will never
-    //     throw or OOM or fail an enter.
-    //
-    //
-    //
-    //
     // GC/MODE
     //     Orthogonally, a crst can be one of the following flavors. We only want to see the
     //     "normal" type used in new code. Other types, kept for legacy reasons, are listed in
@@ -196,30 +183,6 @@ void CrstBase::Enter(INDEBUG(NoLevelCheckFlag noLevelCheckFlag/* = CRST_LEVEL_CH
     ClrDebugState *pClrDebugState = CheckClrDebugState();
     if (pClrDebugState)
     {
-        if (m_dwFlags & CRST_HOST_BREAKABLE)
-        {
-            if (pClrDebugState->IsFaultForbid() &&
-                !(pClrDebugState->ViolationMask() & (FaultViolation|FaultNotFatal|BadDebugState)))
-            {
-                CONTRACT_ASSERT("You cannot enter a HOST_BREAKABLE lock in a FAULTFORBID region.",
-                                Contract::FAULT_Forbid,
-                                Contract::FAULT_Mask,
-                                __FUNCTION__,
-                                __FILE__,
-                                __LINE__);
-            }
-
-            if (!(pClrDebugState->CheckOkayToThrowNoAssert()))
-            {
-                CONTRACT_ASSERT("You cannot enter a HOST_BREAKABLE lock in a NOTHROW region.",
-                                Contract::THROWS_No,
-                                Contract::THROWS_Mask,
-                                __FUNCTION__,
-                                __FILE__,
-                                __LINE__);
-            }
-        }
-
         // If we might want to toggle the GC mode, then we better not be in a GC_NOTRIGGERS region
         if (!(m_dwFlags & (CRST_UNSAFE_COOPGC | CRST_UNSAFE_ANYMODE | CRST_GC_NOTRIGGER_WHEN_TAKEN)))
         {
@@ -424,14 +387,7 @@ void CrstBase::PostEnter()
     STATIC_CONTRACT_NOTHROW;
     STATIC_CONTRACT_GC_NOTRIGGER;
 
-    if ((m_dwFlags & CRST_HOST_BREAKABLE) != 0)
-    {
-        HOST_BREAKABLE_CRST_TAKEN(this);
-    }
-    else
-    {
-        EE_LOCK_TAKEN(this);
-    }
+    EE_LOCK_TAKEN(this);
 
     _ASSERTE((m_entercount == 0 && m_holderthreadid.IsUnknown()) ||
              m_holderthreadid.IsCurrentThread() ||
@@ -459,12 +415,9 @@ void CrstBase::PostEnter()
     }
 
     Thread * pThread = GetThreadNULLOk();
-    if ((m_dwFlags & CRST_HOST_BREAKABLE) == 0)
+    if (pThread)
     {
-        if (pThread)
-        {
-            pThread->IncUnbreakableLockCount();
-        }
+        pThread->IncLockCount();
     }
 
     if ((ThreadStore::s_pThreadStore != NULL)
@@ -516,12 +469,9 @@ void CrstBase::PreLeave()
 
     Thread * pThread = GetThreadNULLOk();
 
-    if ((m_dwFlags & CRST_HOST_BREAKABLE) == 0)
+    if (pThread)
     {
-        if (pThread)
-        {
-            pThread->DecUnbreakableLockCount();
-        }
+        pThread->DecLockCount();
     }
 
     if (m_countNoTriggerGC > 0 && !ThreadStore::s_pThreadStore->IsCrstForThreadStore(this))
@@ -533,14 +483,7 @@ void CrstBase::PreLeave()
         }
     }
 
-    if ((m_dwFlags & CRST_HOST_BREAKABLE) != 0)
-    {
-        HOST_BREAKABLE_CRST_RELEASED(this);
-    }
-    else
-    {
-        EE_LOCK_RELEASED(this);
-    }
+    EE_LOCK_RELEASED(this);
 
     // Are we in the shutdown sequence and in phase 2 of it?
     if (IsAtProcessExit() && (g_fEEShutDown & ShutDown_Phase2))
@@ -581,7 +524,6 @@ void CrstBase::DebugInit(CrstType crstType, CrstFlags flags)
                           CRST_UNSAFE_COOPGC |
                           CRST_UNSAFE_ANYMODE |
                           CRST_DEBUGGER_THREAD |
-                          CRST_HOST_BREAKABLE |
                           CRST_INITIALIZED |
                           CRST_TAKEN_DURING_SHUTDOWN |
                           CRST_GC_NOTRIGGER_WHEN_TAKEN |

src/coreclr/vm/dispatchinfo.cpp

@@ -1038,7 +1038,7 @@ void DispatchMemberInfo::SetUpDispParamAttributes(int iParam, MarshalInfo* Info)
 DispatchInfo::DispatchInfo(MethodTable *pMT)
 : m_pMT(pMT)
 , m_pFirstMemberInfo(NULL)
-, m_lock(CrstInterop, (CrstFlags)(CRST_HOST_BREAKABLE | CRST_REENTRANCY))
+, m_lock(CrstInterop, (CrstFlags)(CRST_REENTRANCY))
 , m_CurrentDispID(0x10000)
 , m_bAllowMembersNotInComMTMemberMap(FALSE)
 , m_bInvokeUsingInvokeMember(FALSE)