Reflect feedback

Author: janvorli

src/System.Private.CoreLib/Resources/Strings.resx

@@ -2884,9 +2884,6 @@
   <data name="NotSupported_CollectibleBoundNonCollectible" xml:space="preserve">
     <value>A non-collectible assembly may not reference a collectible assembly.</value>
   </data>
-  <data name="NotSupported_CollectibleCOM" xml:space="preserve">
-    <value>COM Interop is not supported for collectible types.</value>
-  </data>
   <data name="NotSupported_CreateInstanceWithTypeBuilder" xml:space="preserve">
     <value>CreateInstance cannot be used with an object of type TypeBuilder.</value>
   </data>

src/inc/crsttypes.h

@@ -189,7 +189,8 @@ enum CrstType
     CrstVSDIndirectionCellLock = 170,
     CrstWinRTFactoryCache = 171,
     CrstWrapperTemplate = 172,
-    kNumberOfCrstTypes = 173
+    CrstComCallWrapper = 173,
+    kNumberOfCrstTypes = 174
 };
 
 #endif // __CRST_TYPES_INCLUDED
@@ -373,6 +374,7 @@ int g_rgCrstLevelMap[] =
     3,          // CrstVSDIndirectionCellLock
     3,          // CrstWinRTFactoryCache
     3,          // CrstWrapperTemplate
+    0,          // CrstComCallWrapper
 };
 
 // An array mapping CrstType to a stringized name.
@@ -551,6 +553,7 @@ LPCSTR g_rgCrstNameMap[] =
     "CrstVSDIndirectionCellLock",
     "CrstWinRTFactoryCache",
     "CrstWrapperTemplate",
+    "CrstComCallWrapper",
 };
 
 // Define a special level constant for unordered locks.

src/vm/comcallablewrapper.cpp

@@ -2273,11 +2273,11 @@ ComCallWrapper* ComCallWrapper::CopyFromTemplate(ComCallWrapperTemplate* pTempla
     // num interfaces on the object    
     size_t numInterfaces = pTemplate->GetNumInterfaces();
 
-    // we have a template, create a wrapper and initialize from the template
-    // alloc wrapper, aligned 32 bytes
     if (pWrapperCache->IsLoaderAllocatorUnloading())
         COMPlusThrow(kAppDomainUnloadedException);
 
+    // we have a template, create a wrapper and initialize from the template
+    // alloc wrapper, aligned to cache line
     NewCCWHolder pStartWrapper(pWrapperCache);
     pStartWrapper = (ComCallWrapper*)pWrapperCache->GetCacheLineAllocator()->
 #ifdef _WIN64
@@ -3953,6 +3953,7 @@ ComCallWrapperCache *ComCallWrapperCache::Create(LoaderAllocator *pLoaderAllocat
         GC_TRIGGERS;
         MODE_ANY;
         INJECT_FAULT(COMPlusThrowOM());
+        PRECONDITION(CheckPointer(pLoaderAllocator));
         POSTCONDITION(CheckPointer(RETVAL));
     }
     CONTRACT_END;

src/vm/loaderallocator.cpp

@@ -1070,6 +1070,7 @@ void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory)
     m_crstLoaderAllocator.Init(CrstLoaderAllocator, (CrstFlags)CRST_UNSAFE_COOPGC);
 #ifdef FEATURE_COMINTEROP
     m_InteropDataCrst.Init(CrstInteropData, CRST_REENTRANCY);
+    m_ComCallWrapperCrst.Init(CrstComCallWrapper);
 #endif
 
     //
@@ -1346,6 +1347,10 @@ void LoaderAllocator::Terminate()
     m_pUMEntryThunkCache = NULL;
 
     m_crstLoaderAllocator.Destroy();
+#ifdef FEATURE_COMINTEROP
+    m_ComCallWrapperCrst.Destroy();
+    m_InteropDataCrst.Destroy();
+#endif
     m_LoaderAllocatorReferences.RemoveAll();
 
     // In collectible types we merge the low frequency and high frequency heaps
@@ -1934,7 +1939,6 @@ void AssemblyLoaderAllocator::ReleaseManagedAssemblyLoadContext()
 }
 
 #ifdef FEATURE_COMINTEROP
-// TODO: should it rather go to AssemblyLoaderAllocator?
 ComCallWrapperCache * LoaderAllocator::GetComCallWrapperCache()
 {
     CONTRACTL
@@ -1948,8 +1952,7 @@ ComCallWrapperCache * LoaderAllocator::GetComCallWrapperCache()
 
     if (!m_pComCallWrapperCache)
     {
-        //        BaseDomain::LockHolder lh(this);
-                // TODO: add a lock
+        CrstHolder lh(&m_ComCallWrapperCrst);
 
         if (!m_pComCallWrapperCache)
             m_pComCallWrapperCache = ComCallWrapperCache::Create(this);

src/vm/loaderallocator.hpp

@@ -256,8 +256,9 @@ class LoaderAllocator
     // The wrapper cache for this loader allocator - it has its own CCacheLineAllocator on a per loader allocator basis
     // to allow the loader allocator to go away and eventually kill the memory when all refs are gone
 
-    // TODO: is the volatile needed? The AppDomain version didn't have one
     VolatilePtr<ComCallWrapperCache> m_pComCallWrapperCache;
+    // Used for synchronizing creation of the m_pComCallWrapperCache
+    CrstExplicitInit m_ComCallWrapperCrst;
     // Hash table that maps a MethodTable to COM Interop compatibility data.
     PtrHashMap m_interopDataHash;
     // Used for synchronizing access to the m_interopDataHash