Fix ILStubCache allocation for collectible assemblies (dotnet/coreclr#21188)

The ILStubCache was being allocated per domain unless the domain was a
compilation AppDomain. This is wrong for collectible assemblies, since
after an assembly is collected, the cache keeps stale entries referring
to already deleted MethodTables.
The fix is to make ILStubChange per LoaderAllocator instead (and keep
the per module instances for compilation AppDomain).

Commit migrated from https://github.com/dotnet/coreclr/commit/8aa0869eb9153429091fdba49469d89ec33092cb

diff --git a/src/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp
index ce57ee0..3b4f55a 100644 (file)
--- a/src/coreclr/src/vm/appdomain.cpp
+++ b/src/coreclr/src/vm/appdomain.cpp
@@ -3840,9 +3840,6 @@ void AppDomain::Init()
 
     BaseDomain::Init();
 
-    // Set up the IL stub cache
-    m_ILStubCache.Init(GetLoaderAllocator()->GetHighFrequencyHeap());
-
 // Set up the binding caches
     m_AssemblyCache.Init(&m_DomainCacheCrst, GetHighFrequencyHeap());
     m_UnmanagedCache.InitializeTable(this, &m_DomainCacheCrst);

diff --git a/src/coreclr/src/vm/appdomain.hpp b/src/coreclr/src/vm/appdomain.hpp
index fd28316..5bc7ba6 100644 (file)
--- a/src/coreclr/src/vm/appdomain.hpp
+++ b/src/coreclr/src/vm/appdomain.hpp
@@ -25,7 +25,6 @@
 #include "domainfile.h"
 #include "objectlist.h"
 #include "fptrstubs.h"
-#include "ilstubcache.h"
 #include "testhookmgr.h"
 #include "gcheaputilities.h"
 #include "gchandleutilities.h"
@@ -3339,10 +3338,6 @@ private:
     DWORD m_TrackSpinLock;
 #endif
 
-
-    // IL stub cache with fabricated MethodTable parented by a random module in this AD.
-    ILStubCache         m_ILStubCache;
-
     // The number of  times we have entered this AD
     ULONG m_dwThreadEnterCount;
     // The number of threads that have entered this AD, for ADU only
@@ -3420,17 +3415,6 @@ public:
     BOOL IsBindingModelLocked();
     BOOL LockBindingModel();
 
-    ILStubCache* GetILStubCache()
-    {
-        LIMITED_METHOD_CONTRACT;
-        return &m_ILStubCache;
-    }
-
-    static AppDomain* GetDomain(ILStubCache* pILStubCache)
-    {
-        return CONTAINING_RECORD(pILStubCache, AppDomain, m_ILStubCache);
-    }
-
     enum {
         CONTEXT_INITIALIZED =               0x0001,
         USER_CREATED_DOMAIN =               0x0002, // created by call to AppDomain.CreateDomain

diff --git a/src/coreclr/src/vm/ceeload.cpp b/src/coreclr/src/vm/ceeload.cpp
index 9124013..9f3c709 100644 (file)
--- a/src/coreclr/src/vm/ceeload.cpp
+++ b/src/coreclr/src/vm/ceeload.cpp
@@ -4076,10 +4076,10 @@ ILStubCache* Module::GetILStubCache()
     }
     CONTRACTL_END;
 
-    // Use per-AD cache for domain specific modules when not NGENing
+    // Use per-LoaderAllocator cache for modules when not NGENing
     BaseDomain *pDomain = GetDomain();
     if (!IsSystem() && !pDomain->IsSharedDomain() && !pDomain->AsAppDomain()->IsCompilationDomain())
-        return pDomain->AsAppDomain()->GetILStubCache();
+        return GetLoaderAllocator()->GetILStubCache();
 
     if (m_pILStubCache == NULL)
     {

diff --git a/src/coreclr/src/vm/ilstubcache.cpp b/src/coreclr/src/vm/ilstubcache.cpp
index 93eb330..a1b591f 100644 (file)
--- a/src/coreclr/src/vm/ilstubcache.cpp
+++ b/src/coreclr/src/vm/ilstubcache.cpp
@@ -344,9 +344,9 @@ MethodTable* ILStubCache::GetOrCreateStubMethodTable(Module* pModule)
     }
     else
     {
-        // otherwise we are associated with the AD
-        AppDomain* pStubCacheDomain = AppDomain::GetDomain(this);
-        CONSISTENCY_CHECK(pStubCacheDomain == pModule->GetDomain()->AsAppDomain());
+        // otherwise we are associated with the LoaderAllocator
+        LoaderAllocator* pStubLoaderAllocator = LoaderAllocator::GetLoaderAllocator(this);
+        CONSISTENCY_CHECK(pStubLoaderAllocator == pModule->GetLoaderAllocator());
     }
 #endif // _DEBUG
 

diff --git a/src/coreclr/src/vm/loaderallocator.cpp b/src/coreclr/src/vm/loaderallocator.cpp
index d9af299..52de8c5 100644 (file)
--- a/src/coreclr/src/vm/loaderallocator.cpp
+++ b/src/coreclr/src/vm/loaderallocator.cpp
@@ -1186,6 +1186,9 @@ void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory)
 #else
     m_pPrecodeHeap = new (&m_PrecodeHeapInstance) CodeFragmentHeap(this, STUB_CODE_BLOCK_PRECODE);
 #endif
+
+    // Set up the IL stub cache
+    m_ILStubCache.Init(m_pHighFrequencyHeap);
 }
 
 

diff --git a/src/coreclr/src/vm/loaderallocator.hpp b/src/coreclr/src/vm/loaderallocator.hpp
index c560c14..0dded91 100644 (file)
--- a/src/coreclr/src/vm/loaderallocator.hpp
+++ b/src/coreclr/src/vm/loaderallocator.hpp
@@ -18,6 +18,7 @@
 
 class FuncPtrStubs;
 #include "qcall.h"
+#include "ilstubcache.h"
 
 #define VPTRU_LoaderAllocator 0x3200
 
@@ -178,6 +179,9 @@ protected:
     // The cache is keyed by MethodDesc pointers.
     UMEntryThunkCache * m_pUMEntryThunkCache;
 
+    // IL stub cache with fabricated MethodTable parented by a random module in this LoaderAllocator.
+    ILStubCache         m_ILStubCache;
+
 public:
     BYTE *GetVSDHeapInitialBlock(DWORD *pSize);
     BYTE *GetCodeHeapInitialBlock(const BYTE * loAddr, const BYTE * hiAddr, DWORD minimumSize, DWORD *pSize);
@@ -519,6 +523,17 @@ public:
     UMEntryThunkCache *GetUMEntryThunkCache();
 
 #endif
+
+    static LoaderAllocator* GetLoaderAllocator(ILStubCache* pILStubCache)
+    {
+         return CONTAINING_RECORD(pILStubCache, LoaderAllocator, m_ILStubCache);
+    }
+
+    ILStubCache* GetILStubCache()
+    {
+        LIMITED_METHOD_CONTRACT;
+        return &m_ILStubCache;
+    }
 };  // class LoaderAllocator
 
 typedef VPTR(LoaderAllocator) PTR_LoaderAllocator;