Use reader/writer lock instead of mutex for RCW cache (#91149)

Co-authored-by: Aaron R Robinson <arobins@microsoft.com>

diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def
index 3a1e81f..3bccb73 100644 (file)
--- a/src/coreclr/inc/CrstTypes.def
+++ b/src/coreclr/inc/CrstTypes.def
@@ -394,9 +394,6 @@ End
 Crst RCWCleanupList
 End
 
-Crst ExternalObjectContextCache
-End
-
 Crst Reflection
     AcquiredBefore LoaderHeap UnresolvedClassLock
 End

diff --git a/src/coreclr/inc/crsttypes_generated.h b/src/coreclr/inc/crsttypes_generated.h
index 9710f65..70847a5 100644 (file)
--- a/src/coreclr/inc/crsttypes_generated.h
+++ b/src/coreclr/inc/crsttypes_generated.h
@@ -50,92 +50,91 @@ enum CrstType
     CrstException = 32,
     CrstExecutableAllocatorLock = 33,
     CrstExecuteManRangeLock = 34,
-    CrstExternalObjectContextCache = 35,
-    CrstFCall = 36,
-    CrstFrozenObjectHeap = 37,
-    CrstFuncPtrStubs = 38,
-    CrstFusionAppCtx = 39,
-    CrstGCCover = 40,
-    CrstGlobalStrLiteralMap = 41,
-    CrstHandleTable = 42,
-    CrstIbcProfile = 43,
-    CrstIJWFixupData = 44,
-    CrstIJWHash = 45,
-    CrstILStubGen = 46,
-    CrstInlineTrackingMap = 47,
-    CrstInstMethodHashTable = 48,
-    CrstInterop = 49,
-    CrstInteropData = 50,
-    CrstIsJMCMethod = 51,
-    CrstISymUnmanagedReader = 52,
-    CrstJit = 53,
-    CrstJitGenericHandleCache = 54,
-    CrstJitInlineTrackingMap = 55,
-    CrstJitPatchpoint = 56,
-    CrstJitPerf = 57,
-    CrstJumpStubCache = 58,
-    CrstLeafLock = 59,
-    CrstListLock = 60,
-    CrstLoaderAllocator = 61,
-    CrstLoaderAllocatorReferences = 62,
-    CrstLoaderHeap = 63,
-    CrstManagedObjectWrapperMap = 64,
-    CrstMethodDescBackpatchInfoTracker = 65,
-    CrstMethodTableExposedObject = 66,
-    CrstModule = 67,
-    CrstModuleFixup = 68,
-    CrstModuleLookupTable = 69,
-    CrstMulticoreJitHash = 70,
-    CrstMulticoreJitManager = 71,
-    CrstNativeImageEagerFixups = 72,
-    CrstNativeImageLoad = 73,
-    CrstNls = 74,
-    CrstNotifyGdb = 75,
-    CrstObjectList = 76,
-    CrstPEImage = 77,
-    CrstPendingTypeLoadEntry = 78,
-    CrstPerfMap = 79,
-    CrstPgoData = 80,
-    CrstPinnedByrefValidation = 81,
-    CrstPinnedHeapHandleTable = 82,
-    CrstProfilerGCRefDataFreeList = 83,
-    CrstProfilingAPIStatus = 84,
-    CrstRCWCache = 85,
-    CrstRCWCleanupList = 86,
-    CrstReadyToRunEntryPointToMethodDescMap = 87,
-    CrstReflection = 88,
-    CrstReJITGlobalRequest = 89,
-    CrstRetThunkCache = 90,
-    CrstSavedExceptionInfo = 91,
-    CrstSaveModuleProfileData = 92,
-    CrstSecurityStackwalkCache = 93,
-    CrstSigConvert = 94,
-    CrstSingleUseLock = 95,
-    CrstSpecialStatics = 96,
-    CrstStackSampler = 97,
-    CrstStaticBoxInit = 98,
-    CrstStressLog = 99,
-    CrstStubCache = 100,
-    CrstStubDispatchCache = 101,
-    CrstStubUnwindInfoHeapSegments = 102,
-    CrstSyncBlockCache = 103,
-    CrstSyncHashLock = 104,
-    CrstSystemBaseDomain = 105,
-    CrstSystemDomain = 106,
-    CrstSystemDomainDelayedUnloadList = 107,
-    CrstThreadIdDispenser = 108,
-    CrstThreadStore = 109,
-    CrstTieredCompilation = 110,
-    CrstTypeEquivalenceMap = 111,
-    CrstTypeIDMap = 112,
-    CrstUMEntryThunkCache = 113,
-    CrstUMEntryThunkFreeListLock = 114,
-    CrstUniqueStack = 115,
-    CrstUnresolvedClassLock = 116,
-    CrstUnwindInfoTableLock = 117,
-    CrstVSDIndirectionCellLock = 118,
-    CrstWrapperTemplate = 119,
-    kNumberOfCrstTypes = 120
+    CrstFCall = 35,
+    CrstFrozenObjectHeap = 36,
+    CrstFuncPtrStubs = 37,
+    CrstFusionAppCtx = 38,
+    CrstGCCover = 39,
+    CrstGlobalStrLiteralMap = 40,
+    CrstHandleTable = 41,
+    CrstIbcProfile = 42,
+    CrstIJWFixupData = 43,
+    CrstIJWHash = 44,
+    CrstILStubGen = 45,
+    CrstInlineTrackingMap = 46,
+    CrstInstMethodHashTable = 47,
+    CrstInterop = 48,
+    CrstInteropData = 49,
+    CrstIsJMCMethod = 50,
+    CrstISymUnmanagedReader = 51,
+    CrstJit = 52,
+    CrstJitGenericHandleCache = 53,
+    CrstJitInlineTrackingMap = 54,
+    CrstJitPatchpoint = 55,
+    CrstJitPerf = 56,
+    CrstJumpStubCache = 57,
+    CrstLeafLock = 58,
+    CrstListLock = 59,
+    CrstLoaderAllocator = 60,
+    CrstLoaderAllocatorReferences = 61,
+    CrstLoaderHeap = 62,
+    CrstManagedObjectWrapperMap = 63,
+    CrstMethodDescBackpatchInfoTracker = 64,
+    CrstMethodTableExposedObject = 65,
+    CrstModule = 66,
+    CrstModuleFixup = 67,
+    CrstModuleLookupTable = 68,
+    CrstMulticoreJitHash = 69,
+    CrstMulticoreJitManager = 70,
+    CrstNativeImageEagerFixups = 71,
+    CrstNativeImageLoad = 72,
+    CrstNls = 73,
+    CrstNotifyGdb = 74,
+    CrstObjectList = 75,
+    CrstPEImage = 76,
+    CrstPendingTypeLoadEntry = 77,
+    CrstPerfMap = 78,
+    CrstPgoData = 79,
+    CrstPinnedByrefValidation = 80,
+    CrstPinnedHeapHandleTable = 81,
+    CrstProfilerGCRefDataFreeList = 82,
+    CrstProfilingAPIStatus = 83,
+    CrstRCWCache = 84,
+    CrstRCWCleanupList = 85,
+    CrstReadyToRunEntryPointToMethodDescMap = 86,
+    CrstReflection = 87,
+    CrstReJITGlobalRequest = 88,
+    CrstRetThunkCache = 89,
+    CrstSavedExceptionInfo = 90,
+    CrstSaveModuleProfileData = 91,
+    CrstSecurityStackwalkCache = 92,
+    CrstSigConvert = 93,
+    CrstSingleUseLock = 94,
+    CrstSpecialStatics = 95,
+    CrstStackSampler = 96,
+    CrstStaticBoxInit = 97,
+    CrstStressLog = 98,
+    CrstStubCache = 99,
+    CrstStubDispatchCache = 100,
+    CrstStubUnwindInfoHeapSegments = 101,
+    CrstSyncBlockCache = 102,
+    CrstSyncHashLock = 103,
+    CrstSystemBaseDomain = 104,
+    CrstSystemDomain = 105,
+    CrstSystemDomainDelayedUnloadList = 106,
+    CrstThreadIdDispenser = 107,
+    CrstThreadStore = 108,
+    CrstTieredCompilation = 109,
+    CrstTypeEquivalenceMap = 110,
+    CrstTypeIDMap = 111,
+    CrstUMEntryThunkCache = 112,
+    CrstUMEntryThunkFreeListLock = 113,
+    CrstUniqueStack = 114,
+    CrstUnresolvedClassLock = 115,
+    CrstUnwindInfoTableLock = 116,
+    CrstVSDIndirectionCellLock = 117,
+    CrstWrapperTemplate = 118,
+    kNumberOfCrstTypes = 119
 };
 
 #endif // __CRST_TYPES_INCLUDED
@@ -181,7 +180,6 @@ int g_rgCrstLevelMap[] =
     0,          // CrstException
     0,          // CrstExecutableAllocatorLock
     0,          // CrstExecuteManRangeLock
-    0,          // CrstExternalObjectContextCache
     4,          // CrstFCall
     -1,         // CrstFrozenObjectHeap
     7,          // CrstFuncPtrStubs
@@ -306,7 +304,6 @@ LPCSTR g_rgCrstNameMap[] =
     "CrstException",
     "CrstExecutableAllocatorLock",
     "CrstExecuteManRangeLock",
-    "CrstExternalObjectContextCache",
     "CrstFCall",
     "CrstFrozenObjectHeap",
     "CrstFuncPtrStubs",

diff --git a/src/coreclr/vm/interoplibinterface_comwrappers.cpp b/src/coreclr/vm/interoplibinterface_comwrappers.cpp
index 93fa6a4..4b41b68 100644 (file)
--- a/src/coreclr/vm/interoplibinterface_comwrappers.cpp
+++ b/src/coreclr/vm/interoplibinterface_comwrappers.cpp
@@ -5,6 +5,7 @@
 
 // Runtime headers
 #include "common.h"
+#include "simplerwlock.hpp"
 #include "rcwrefcache.h"
 #ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
 #include "olecontexthelpers.h"
@@ -257,32 +258,36 @@ namespace
         using Element = SHash<Traits>::element_t;
         using Iterator = SHash<Traits>::Iterator;
 
-        class LockHolder : public CrstHolder
+        class ReaderLock final
         {
+            SimpleReadLockHolder _lock;
         public:
-            LockHolder(_In_ ExtObjCxtCache *cache)
-                : CrstHolder(&cache->_lock)
-            {
-                // This cache must be locked in Cooperative mode
-                // since releases of wrappers can occur during a GC.
-                CONTRACTL
-                {
-                    NOTHROW;
-                    GC_NOTRIGGER;
-                    MODE_COOPERATIVE;
-                }
-                CONTRACTL_END;
-            }
+            ReaderLock(_In_ ExtObjCxtCache* cache)
+                : _lock{ &cache->_lock }
+            { }
+
+            ~ReaderLock() = default;
+        };
+
+        class WriterLock final
+        {
+            SimpleWriteLockHolder _lock;
+        public:
+            WriterLock(_In_ ExtObjCxtCache* cache)
+                : _lock{ &cache->_lock }
+            { }
+
+            ~WriterLock() = default;
         };
 
     private:
         friend struct InteropLibImports::RuntimeCallContext;
         SHash<Traits> _hashMap;
-        Crst _lock;
+        SimpleRWLock _lock;
         ExtObjCxtRefCache* _refCache;
 
         ExtObjCxtCache()
-            : _lock(CrstExternalObjectContextCache, CRST_UNSAFE_COOPGC)
+            : _lock(COOPERATIVE, LOCK_TYPE_DEFAULT)
             , _refCache(GetAppDomain()->GetRCWRefCache())
         { }
         ~ExtObjCxtCache() = default;
@@ -292,7 +297,7 @@ namespace
         bool IsLockHeld()
         {
             WRAPPER_NO_CONTRACT;
-            return (_lock.OwnedByCurrentThread() != FALSE);
+            return (_lock.LockTaken() != FALSE);
         }
 #endif // _DEBUG
 
@@ -343,7 +348,7 @@ namespace
             // Determine the count of objects to return.
             SIZE_T objCountMax = 0;
             {
-                LockHolder lock(this);
+                ReaderLock lock(this);
                 Iterator end = _hashMap.End();
                 for (Iterator curr = _hashMap.Begin(); curr != end; ++curr)
                 {
@@ -365,7 +370,7 @@ namespace
             SIZE_T objCount = 0;
             if (0 < objCountMax)
             {
-                LockHolder lock(this);
+                ReaderLock lock(this);
                 Iterator end = _hashMap.End();
                 for (Iterator curr = _hashMap.Begin(); curr != end; ++curr)
                 {
@@ -823,11 +828,22 @@ namespace
         if (!uniqueInstance)
         {
             bool objectFound = false;
+            bool tryRemove = false;
             {
-                // Query the external object cache
-                ExtObjCxtCache::LockHolder lock(cache);
+                // Perform a quick look up to determine if we know of the object and if
+                // we need to perform a more expensive cleanup operation below.
+                ExtObjCxtCache::ReaderLock lock(cache);
                 extObjCxt = cache->Find(cacheKey);
+                objectFound = extObjCxt != NULL;
+                tryRemove = objectFound && extObjCxt->IsSet(ExternalObjectContext::Flags_Detached);
+            }
 
+            if (tryRemove)
+            {
+                // Perform the slower cleanup operation that may be appropriate
+                // if the object still exists and has been detached.
+                ExtObjCxtCache::WriterLock lock(cache);
+                extObjCxt = cache->Find(cacheKey);
                 objectFound = extObjCxt != NULL;
                 if (objectFound && extObjCxt->IsSet(ExternalObjectContext::Flags_Detached))
                 {
@@ -958,7 +974,7 @@ namespace
                 else
                 {
                     // Attempt to insert the new context into the cache.
-                    ExtObjCxtCache::LockHolder lock(cache);
+                    ExtObjCxtCache::WriterLock lock(cache);
                     extObjCxt = cache->FindOrAdd(cacheKey, resultHolder.GetContext());
                 }
 
@@ -980,7 +996,7 @@ namespace
                     {
                         // Failed to set the context; one must already exist.
                         // Remove from the cache above as well.
-                        ExtObjCxtCache::LockHolder lock(cache);
+                        ExtObjCxtCache::WriterLock lock(cache);
                         cache->Remove(resultHolder.GetContext());
 
                         COMPlusThrow(kNotSupportedException);