Remove no-op holder stack validation. (#22182)

3 files changed, 22 insertions, 65 deletions

diff --git a/src/inc/holder.h b/src/inc/holder.h
index 49c90b73de..c346c736ac 100644
--- a/src/inc/holder.h
+++ b/src/inc/holder.h
@@ -61,17 +61,8 @@
         _NAME & operator=(_NAME const &);
 #endif
 
-// The types of stack validation we support in holders.
-enum HolderStackValidation
-{
-    HSV_NoValidation,
-    HSV_ValidateMinimumStackReq,
-    HSV_ValidateNormalStackReq,        
-};
-
 #ifdef _DEBUG
 
-
 //------------------------------------------------------------------------------------------------
 // This is used to make Visual Studio autoexp.dat work sensibly with holders again.
 // The problem is that certain codebases (particulary Fusion) implement key data structures
@@ -239,8 +230,7 @@ template
         typename TYPE,
         typename BASE,
         UINT_PTR DEFAULTVALUE = 0,
-        BOOL IS_NULL(TYPE, TYPE) = CompareDefault<TYPE>,
-        HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq
+        BOOL IS_NULL(TYPE, TYPE) = CompareDefault<TYPE>
     >
 class BaseHolder : protected BASE
 {
@@ -304,15 +294,7 @@ class BaseHolder : protected BASE
         if (m_acquired)
         {
             _ASSERTE(!IsNull());
-        
-            if (VALIDATION_TYPE != HSV_NoValidation)
-            {
-                this->DoRelease();
-            }
-            else
-            {
-                this->DoRelease();
-            }
+            this->DoRelease();
             m_acquired = FALSE;
         }
     }
@@ -347,7 +329,7 @@ class BaseHolder : protected BASE
     HIDE_GENERATED_METHODS(BaseHolder)
 };  // BaseHolder<>
 
-template <void (*ACQUIRE)(), void (*RELEASEF)(), HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq>
+template <void (*ACQUIRE)(), void (*RELEASEF)()>
 class StateHolder
 {
   private:
@@ -383,14 +365,7 @@ class StateHolder
 
         if (m_acquired)
         {
-            if (VALIDATION_TYPE != HSV_NoValidation)
-            {
-                RELEASEF();
-            }
-            else
-            {
-                RELEASEF();
-            }
+            RELEASEF();
             m_acquired = FALSE;
         }
     }
@@ -414,7 +389,7 @@ class StateHolder
 };  // class StateHolder<>
 
 // Holder for the case where the acquire function can fail.
-template <typename VALUE, BOOL (*ACQUIRE)(VALUE value), void (*RELEASEF)(VALUE value), HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq>
+template <typename VALUE, BOOL (*ACQUIRE)(VALUE value), void (*RELEASEF)(VALUE value)>
 class ConditionalStateHolder
 {
   private:
@@ -452,14 +427,7 @@ class ConditionalStateHolder
 
         if (m_acquired)
         {
-            if (VALIDATION_TYPE != HSV_NoValidation)
-            {
-                RELEASEF(m_value);
-            }
-            else
-            {
-                RELEASEF(m_value);
-            }
+            RELEASEF(m_value);
             m_acquired = FALSE;
         }
     }
@@ -501,10 +469,10 @@ class ConditionalStateHolder
 // the value it contains.
 //-----------------------------------------------------------------------------
 template <typename TYPE, typename BASE,
-          UINT_PTR DEFAULTVALUE = 0, BOOL IS_NULL(TYPE, TYPE) = CompareDefault<TYPE>, HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq> 
-class BaseWrapper : public BaseHolder<TYPE, BASE, DEFAULTVALUE, IS_NULL, VALIDATION_TYPE>
+          UINT_PTR DEFAULTVALUE = 0, BOOL IS_NULL(TYPE, TYPE) = CompareDefault<TYPE>>
+class BaseWrapper : public BaseHolder<TYPE, BASE, DEFAULTVALUE, IS_NULL>
 {
-    typedef BaseHolder<TYPE, BASE, DEFAULTVALUE, IS_NULL, VALIDATION_TYPE> BaseT;
+    typedef BaseHolder<TYPE, BASE, DEFAULTVALUE, IS_NULL> BaseT;
 
 
 #ifdef __GNUC__
@@ -731,7 +699,7 @@ FORCEINLINE void SafeArrayDoNothing(SAFEARRAY* p)
 // function pointers
 //-----------------------------------------------------------------------------
 
-template <typename TYPE, void (*ACQUIREF)(TYPE), void (*RELEASEF)(TYPE), HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq>  
+template <typename TYPE, void (*ACQUIREF)(TYPE), void (*RELEASEF)(TYPE)>
 class FunctionBase : protected HolderBase<TYPE>
 {
   protected:
@@ -748,16 +716,7 @@ class FunctionBase : protected HolderBase<TYPE>
 
     void DoRelease()
     {
-        // <TODO> Consider removing this stack validation since it is redundant with the
-        // one that is already being done in BaseHolder & BaseWrapper. </TODO>
-        if (VALIDATION_TYPE != HSV_NoValidation)
-        {
-            RELEASEF(this->m_value);
-        }
-        else
-        {
-            RELEASEF(this->m_value);
-        }
+        RELEASEF(this->m_value);
     }
 };  // class Function<>
 
@@ -768,17 +727,16 @@ template
         void (*RELEASEF)(TYPE),
         UINT_PTR DEFAULTVALUE = 0, 
         BOOL IS_NULL(TYPE, TYPE) = CompareDefault<TYPE>,
-        HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq,
         // For legacy compat (see EEJitManager::WriterLockHolder), where default ctor
         // causes ACQUIREF(DEFAULTVALUE), but ACQUIREF ignores the argument and
         // operates on static or global value instead.
         bool DEFAULT_CTOR_ACQUIRE = true
     >
-class Holder : public BaseHolder<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF, VALIDATION_TYPE>,
-                                 DEFAULTVALUE, IS_NULL, VALIDATION_TYPE>
+class Holder : public BaseHolder<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF>,
+                                 DEFAULTVALUE, IS_NULL>
 {
-    typedef BaseHolder<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF, VALIDATION_TYPE>,
-                                 DEFAULTVALUE, IS_NULL, VALIDATION_TYPE> BaseT;
+    typedef BaseHolder<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF>,
+                                 DEFAULTVALUE, IS_NULL> BaseT;
 
   public:
     FORCEINLINE Holder()
@@ -818,17 +776,16 @@ template
         void (*RELEASEF)(TYPE),
         UINT_PTR DEFAULTVALUE = 0,
         BOOL IS_NULL(TYPE, TYPE) = CompareDefault<TYPE>,
-        HolderStackValidation VALIDATION_TYPE = HSV_ValidateNormalStackReq,
         // For legacy compat (see EEJitManager::WriterLockHolder), where default ctor
         // causes ACQUIREF(DEFAULTVALUE), but ACQUIREF ignores the argument and
         // operates on static or global value instead.
         bool DEFAULT_CTOR_ACQUIRE = true
     >
-class Wrapper : public BaseWrapper<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF, VALIDATION_TYPE>,
-                                   DEFAULTVALUE, IS_NULL, VALIDATION_TYPE>
+class Wrapper : public BaseWrapper<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF>,
+                                   DEFAULTVALUE, IS_NULL>
 {
-    typedef BaseWrapper<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF, VALIDATION_TYPE>,
-                                   DEFAULTVALUE, IS_NULL, VALIDATION_TYPE> BaseT;
+    typedef BaseWrapper<TYPE, FunctionBase<TYPE, ACQUIREF, RELEASEF>,
+                                   DEFAULTVALUE, IS_NULL> BaseT;
 
   public:
     FORCEINLINE Wrapper()
@@ -1215,7 +1172,7 @@ typedef Wrapper< bool *, BoolSet, BoolUnset > BoolFlagStateHolder;
 FORCEINLINE void CounterIncrease(RAW_KEYWORD(volatile) LONG* p) {InterlockedIncrement(p);};
 FORCEINLINE void CounterDecrease(RAW_KEYWORD(volatile) LONG* p) {InterlockedDecrement(p);};
 
-typedef Wrapper<RAW_KEYWORD(volatile) LONG*, CounterIncrease, CounterDecrease, (UINT_PTR)0, CompareDefault<RAW_KEYWORD(volatile) LONG*>, HSV_NoValidation> CounterHolder;
+typedef Wrapper<RAW_KEYWORD(volatile) LONG*, CounterIncrease, CounterDecrease, (UINT_PTR)0, CompareDefault<RAW_KEYWORD(volatile) LONG*>> CounterHolder;
 
 
 #ifndef FEATURE_PAL
diff --git a/src/inc/profilepriv.inl b/src/inc/profilepriv.inl
index c612b3d9a5..ac52521b36 100644
--- a/src/inc/profilepriv.inl
+++ b/src/inc/profilepriv.inl
@@ -743,7 +743,7 @@ inline BOOL CORProfilerDisableTieredCompilation()
 // See code:ProfilingAPIUtility::InitializeProfiling#LoadUnloadCallbackSynchronization.
 // 
 typedef Wrapper<Thread *, ProfilingAPIUtility::IncEvacuationCounter, ProfilingAPIUtility::DecEvacuationCounter, 
-    (UINT_PTR)0, CompareDefault<Thread *>, HSV_NoValidation> EvacuationCounterHolder;
+    (UINT_PTR)0, CompareDefault<Thread *>> EvacuationCounterHolder;
 
 
 //---------------------------------------------------------------------------------------
diff --git a/src/inc/stresslog.h b/src/inc/stresslog.h
index d109778a65..dd8c029b0a 100644
--- a/src/inc/stresslog.h
+++ b/src/inc/stresslog.h
@@ -397,7 +397,7 @@ typedef USHORT
     static StressLog theLog;    // We only have one log, and this is it
 };
 
-typedef Holder<CRITSEC_COOKIE, StressLog::Enter, StressLog::Leave, NULL, CompareDefault<CRITSEC_COOKIE>, HSV_NoValidation> StressLogLockHolder;
+typedef Holder<CRITSEC_COOKIE, StressLog::Enter, StressLog::Leave, NULL, CompareDefault<CRITSEC_COOKIE>> StressLogLockHolder;
 
 #if defined(DACCESS_COMPILE)
 inline BOOL StressLog::LogOn(unsigned facility, unsigned level)