Merge pull request #11033 from adityamandaleeka/handle_table_local_gc_store

Move handle object manipulation functions to interface

Author: adityamandaleeka

src/gc/gc.cpp

@@ -34218,7 +34218,7 @@ bool GCHeap::StressHeap(gc_alloc_context * context)
                     if (g_pConfig->AppDomainLeaks() && str->SetAppDomainNoThrow())
                     {
 #endif
-                        StoreObjectInHandle(m_StressObjs[i], ObjectToOBJECTREF(str));
+                        HndAssignHandle(m_StressObjs[i], ObjectToOBJECTREF(str));
 #if CHECK_APP_DOMAIN_LEAKS
                     }
 #endif
@@ -34251,7 +34251,7 @@ bool GCHeap::StressHeap(gc_alloc_context * context)
             {
                 // Let the string itself become garbage.
                 // will be realloced next time around
-                StoreObjectInHandle(m_StressObjs[m_CurStressObj], 0);
+                HndAssignHandle(m_StressObjs[m_CurStressObj], 0);
             }
         }
     }

src/gc/gchandletable.cpp

@@ -132,3 +132,18 @@ void* GCHandleManager::GetExtraInfoFromHandle(OBJECTHANDLE handle)
 {
     return (void*)::HndGetHandleExtraInfo(handle);
 }
+
+void GCHandleManager::StoreObjectInHandle(OBJECTHANDLE handle, Object* object)
+{
+    ::HndAssignHandle(handle, ObjectToOBJECTREF(object));
+}
+
+bool GCHandleManager::StoreObjectInHandleIfNull(OBJECTHANDLE handle, Object* object)
+{
+    return !!::HndFirstAssignHandle(handle, ObjectToOBJECTREF(object));
+}
+
+Object* GCHandleManager::InterlockedCompareExchangeObjectInHandle(OBJECTHANDLE handle, Object* object, Object* comparandObject)
+{
+    return (Object*)::HndInterlockedCompareExchangeHandle(handle, ObjectToOBJECTREF(object), ObjectToOBJECTREF(comparandObject));
+}

src/gc/gchandletableimpl.h

@@ -54,6 +54,12 @@ class GCHandleManager : public IGCHandleManager
     virtual void DestroyHandleOfUnknownType(OBJECTHANDLE handle);
 
     virtual void* GetExtraInfoFromHandle(OBJECTHANDLE handle);
+
+    virtual void StoreObjectInHandle(OBJECTHANDLE handle, Object* object);
+
+    virtual bool StoreObjectInHandleIfNull(OBJECTHANDLE handle, Object* object);
+
+    virtual Object* InterlockedCompareExchangeObjectInHandle(OBJECTHANDLE handle, Object* object, Object* comparandObject);
 };
 
 #endif  // GCHANDLETABLE_H_

src/gc/gcinterface.h

@@ -444,6 +444,12 @@ class IGCHandleManager {
     virtual void DestroyHandleOfUnknownType(OBJECTHANDLE handle) = 0;
 
     virtual void* GetExtraInfoFromHandle(OBJECTHANDLE handle) = 0;
+
+    virtual void StoreObjectInHandle(OBJECTHANDLE handle, Object* object) = 0;
+
+    virtual bool StoreObjectInHandleIfNull(OBJECTHANDLE handle, Object* object) = 0;
+
+    virtual Object* InterlockedCompareExchangeObjectInHandle(OBJECTHANDLE handle, Object* object, Object* comparandObject) = 0;
 };
 
 // IGCHeap is the interface that the VM will use when interacting with the GC.

src/gc/handletable.inl

@@ -22,13 +22,6 @@ inline void HndAssignHandle(OBJECTHANDLE handle, OBJECTREF objref)
     // sanity
     _ASSERTE(handle);
 
-#ifdef _DEBUG_IMPL
-    // handle should not be in unloaded domain
-    ValidateAppDomainForHandle(handle);
-
-    // Make sure the objref is valid before it is assigned to a handle
-    ValidateAssignObjrefForHandle(objref, HndGetHandleTableADIndex(HndGetHandleTable(handle)));
-#endif
     // unwrap the objectref we were given
     _UNCHECKED_OBJECTREF value = OBJECTREF_TO_UNCHECKED_OBJECTREF(objref);
 
@@ -49,13 +42,6 @@ inline void* HndInterlockedCompareExchangeHandle(OBJECTHANDLE handle, OBJECTREF
     // sanity
     _ASSERTE(handle);
 
-#ifdef _DEBUG_IMPL
-    // handle should not be in unloaded domain
-    ValidateAppDomainForHandle(handle);
-
-    // Make sure the objref is valid before it is assigned to a handle
-    ValidateAssignObjrefForHandle(objref, HndGetHandleTableADIndex(HndGetHandleTable(handle)));
-#endif
     // unwrap the objectref we were given
     _UNCHECKED_OBJECTREF value = OBJECTREF_TO_UNCHECKED_OBJECTREF(objref);
     _UNCHECKED_OBJECTREF oldValue = OBJECTREF_TO_UNCHECKED_OBJECTREF(oldObjref);
@@ -88,13 +74,6 @@ inline BOOL HndFirstAssignHandle(OBJECTHANDLE handle, OBJECTREF objref)
     // sanity
     _ASSERTE(handle);
 
-#ifdef _DEBUG_IMPL
-    // handle should not be in unloaded domain
-    ValidateAppDomainForHandle(handle);
-
-    // Make sure the objref is valid before it is assigned to a handle
-    ValidateAssignObjrefForHandle(objref, HndGetHandleTableADIndex(HndGetHandleTable(handle)));
-#endif
     // unwrap the objectref we were given
     _UNCHECKED_OBJECTREF value = OBJECTREF_TO_UNCHECKED_OBJECTREF(objref);
     _UNCHECKED_OBJECTREF null = NULL;

src/gc/objecthandle.h

@@ -21,16 +21,6 @@
 #include <weakreference.h>
 #endif // FEATURE_COMINTEROP
 
-/*
- * Convenience macros for accessing handles.  StoreFirstObjectInHandle is like
- * StoreObjectInHandle, except it only succeeds if transitioning from NULL to
- * non-NULL.  In other words, if this handle is being initialized for the first
- * time.
- */
-#define StoreObjectInHandle(handle, object)        HndAssignHandle(handle, object)
-#define InterlockedCompareExchangeObjectInHandle(handle, object, oldObj)        HndInterlockedCompareExchangeHandle(handle, object, oldObj)
-#define StoreFirstObjectInHandle(handle, object)   HndFirstAssignHandle(handle, object)
-
 typedef DPTR(struct HandleTableMap) PTR_HandleTableMap;
 typedef DPTR(struct HandleTableBucket) PTR_HandleTableBucket;
 typedef DPTR(PTR_HandleTableBucket) PTR_PTR_HandleTableBucket;
@@ -90,17 +80,6 @@ void GCHandleValidatePinnedObject(OBJECTREF obj);
 
 int GetCurrentThreadHomeHeapNumber();
 
-inline void ResetOBJECTHANDLE(OBJECTHANDLE handle)
-{
-    WRAPPER_NO_CONTRACT;
-
-    StoreObjectInHandle(handle, NULL);
-}
-
-#ifndef FEATURE_REDHAWK
-typedef Holder<OBJECTHANDLE,DoNothing<OBJECTHANDLE>,ResetOBJECTHANDLE> ObjectInHandleHolder;
-#endif
-
 /*
  * Table maintenance routines
  */

src/vm/appdomain.cpp

@@ -4679,7 +4679,8 @@ OBJECTREF AppDomain::GetExposedObject()
         obj = (APPDOMAINREF) AllocateObject(pMT);
         obj->SetDomain(this);
 
-        if(StoreFirstObjectInHandle(m_ExposedObject, (OBJECTREF) obj) == FALSE) {
+        if (!StoreFirstObjectInHandle(m_ExposedObject, (OBJECTREF) obj))
+        {
             obj = (APPDOMAINREF) GetRawExposedObject();
             _ASSERTE(obj);
         }

src/vm/gchandleutilities.h

@@ -30,15 +30,19 @@ class GCHandleUtilities
     GCHandleUtilities() = delete;
 };
 
-void ValidateHandleAndAppDomain(OBJECTHANDLE handle);
+void ValidateObjectAndAppDomain(OBJECTREF objRef, ADIndex appDomainIndex);
+void ValidateHandleAssignment(OBJECTHANDLE handle, OBJECTREF objRef);
 
 // Given a handle, returns an OBJECTREF for the object it refers to.
 inline OBJECTREF ObjectFromHandle(OBJECTHANDLE handle)
 {
     _ASSERTE(handle);
 
 #ifdef _DEBUG_IMPL
-    ValidateHandleAndAppDomain(handle);
+    DWORD context = (DWORD)GCHandleUtilities::GetGCHandleManager()->GetHandleContext(handle);
+    OBJECTREF objRef = ObjectToOBJECTREF(*(Object**)handle);
+
+    ValidateObjectAndAppDomain(objRef, ADIndex(context));
 #endif // _DEBUG_IMPL
 
     // Wrap the raw OBJECTREF and return it
@@ -169,6 +173,34 @@ inline OBJECTHANDLE CreateVariableHandle(IGCHandleStore* store, OBJECTREF object
     return store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_VARIABLE, (void*)((uintptr_t)type));
 }
 
+// Handle object manipulation convenience functions
+
+inline void StoreObjectInHandle(OBJECTHANDLE handle, OBJECTREF object)
+{
+    ValidateHandleAssignment(handle, object);
+
+    GCHandleUtilities::GetGCHandleManager()->StoreObjectInHandle(handle, OBJECTREFToObject(object));
+}
+
+inline bool StoreFirstObjectInHandle(OBJECTHANDLE handle, OBJECTREF object)
+{
+    ValidateHandleAssignment(handle, object);
+
+    return GCHandleUtilities::GetGCHandleManager()->StoreObjectInHandleIfNull(handle, OBJECTREFToObject(object));
+}
+
+inline void* InterlockedCompareExchangeObjectInHandle(OBJECTHANDLE handle, OBJECTREF object, OBJECTREF comparandObject)
+{
+    ValidateHandleAssignment(handle, object);
+
+    return GCHandleUtilities::GetGCHandleManager()->InterlockedCompareExchangeObjectInHandle(handle, OBJECTREFToObject(object), OBJECTREFToObject(comparandObject));
+}
+
+inline void ResetOBJECTHANDLE(OBJECTHANDLE handle)
+{
+    GCHandleUtilities::GetGCHandleManager()->StoreObjectInHandle(handle, NULL);
+}
+
 // Handle destruction convenience functions
 
 inline void DestroyHandle(OBJECTHANDLE handle)
@@ -309,6 +341,7 @@ typedef Wrapper<OBJECTHANDLE, DoNothing<OBJECTHANDLE>, DestroyRefcountedHandle>
 typedef Holder<OBJECTHANDLE, DoNothing<OBJECTHANDLE>, DestroyLongWeakHandle>            LongWeakHandleHolder;
 typedef Holder<OBJECTHANDLE, DoNothing<OBJECTHANDLE>, DestroyGlobalStrongHandle>        GlobalStrongHandleHolder;
 typedef Holder<OBJECTHANDLE, DoNothing<OBJECTHANDLE>, DestroyGlobalShortWeakHandle>     GlobalShortWeakHandleHolder;
+typedef Holder<OBJECTHANDLE, DoNothing<OBJECTHANDLE>, ResetOBJECTHANDLE>                ObjectInHandleHolder;
 
 class RCOBJECTHANDLEHolder : public RefCountedOHWrapper
 {

src/vm/gcheaputilities.cpp

@@ -40,17 +40,12 @@ bool g_sw_ww_enabled_for_gc_heap = false;
 gc_alloc_context g_global_alloc_context = {};
 
 // Debug-only validation for handle.
-void ValidateHandleAndAppDomain(OBJECTHANDLE handle)
+
+void ValidateObjectAndAppDomain(OBJECTREF objRef, ADIndex appDomainIndex)
 {
 #ifdef _DEBUG_IMPL
-    OBJECTREF objRef = ObjectToOBJECTREF(*(Object**)handle);
     VALIDATEOBJECTREF(objRef);
 
-    IGCHandleManager *pHandleManager = GCHandleUtilities::GetGCHandleManager();
-
-    DWORD context = (DWORD)pHandleManager->GetHandleContext(handle);
-
-    ADIndex appDomainIndex = ADIndex(context);
     AppDomain *domain = SystemDomain::GetAppDomainAtIndex(appDomainIndex);
 
     // Access to a handle in an unloaded domain is not allowed
@@ -72,3 +67,25 @@ void ValidateHandleAndAppDomain(OBJECTHANDLE handle)
 #endif // CHECK_APP_DOMAIN_LEAKS
 #endif // _DEBUG_IMPL
 }
+
+void ValidateHandleAssignment(OBJECTHANDLE handle, OBJECTREF objRef)
+{
+#ifdef _DEBUG_IMPL
+    _ASSERTE(handle);
+
+#ifdef DEBUG_DestroyedHandleValue
+    // Verify that we are not trying to access a freed handle.
+    _ASSERTE("Attempt to access destroyed handle." && *(_UNCHECKED_OBJECTREF*)handle != DEBUG_DestroyedHandleValue);
+#endif
+
+    ADIndex appDomainIndex = HndGetHandleADIndex(handle);
+
+    AppDomain *unloadingDomain = SystemDomain::AppDomainBeingUnloaded();
+    if (unloadingDomain && unloadingDomain->GetIndex() == appDomainIndex && unloadingDomain->NoAccessToHandleTable())
+    {
+        _ASSERTE (!"Access to a handle in unloaded domain is not allowed");
+    }
+
+    ValidateObjectAndAppDomain(objRef, appDomainIndex);
+#endif // _DEBUG_IMPL
+}