From 1128b96b1148a644a5071b7c786c401091e5452d Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Tue, 10 Sep 2024 15:21:56 -0700 Subject: [PATCH] Always pass LoaderAllocator when creating Assembly (#107643) We are currently conditionally passing the `LoaderAllocator` to initialize the `Assembly` based on whether it is collectible and then having the initialization explicitly get the global one if it was null. When we create the `Assembly`, we already have the appropriate loader allocator (global loader allocator for non-collectible, corresponding assembly loader allocator for collectible), so just create the `Assembly` with it. --- src/coreclr/vm/assembly.cpp | 75 ++++++++++++++----------------- src/coreclr/vm/assembly.hpp | 6 +-- src/coreclr/vm/domainassembly.cpp | 2 +- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 0d344df01297bd..0fe4bf8896384b 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -117,28 +117,35 @@ void Assembly::Initialize() // It cannot do any allocations or operations that might fail. Those operations should be done // in Assembly::Init() //---------------------------------------------------------------------------------------------- -Assembly::Assembly(PEAssembly* pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, BOOL fIsCollectible) : - m_pClassLoader(NULL), - m_pEntryPoint(NULL), - m_pModule(NULL), - m_pPEAssembly(clr::SafeAddRef(pPEAssembly)), - m_pFriendAssemblyDescriptor(NULL), - m_isDynamic(false), +Assembly::Assembly(PEAssembly* pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, LoaderAllocator *pLoaderAllocator) + : m_pClassLoader(NULL) + , m_pEntryPoint(NULL) + , m_pModule(NULL) + , m_pPEAssembly(clr::SafeAddRef(pPEAssembly)) + , m_pFriendAssemblyDescriptor(NULL) + , m_isDynamic(false) #ifdef FEATURE_COLLECTIBLE_TYPES - m_isCollectible(fIsCollectible), + , m_isCollectible{pLoaderAllocator->IsCollectible()} #endif - m_pLoaderAllocator(NULL), + , m_pLoaderAllocator{pLoaderAllocator} #ifdef FEATURE_COMINTEROP - m_pITypeLib(NULL), + , m_pITypeLib(NULL) #endif // FEATURE_COMINTEROP #ifdef FEATURE_COMINTEROP - m_InteropAttributeStatus(INTEROP_ATTRIBUTE_UNSET), + , m_InteropAttributeStatus(INTEROP_ATTRIBUTE_UNSET) #endif - m_debuggerFlags(debuggerFlags), - m_fTerminated(FALSE), - m_hExposedObject{} + , m_debuggerFlags(debuggerFlags) + , m_fTerminated(FALSE) + , m_hExposedObject{} { - STANDARD_VM_CONTRACT; + CONTRACTL + { + STANDARD_VM_CHECK; + PRECONDITION(pPEAssembly != NULL); + PRECONDITION(pLoaderAllocator != NULL); + PRECONDITION(pLoaderAllocator->IsCollectible() || pLoaderAllocator == SystemDomain::GetGlobalLoaderAllocator()); + } + CONTRACTL_END } // This name needs to stay in sync with AssemblyBuilder.ManifestModuleName @@ -151,32 +158,10 @@ Assembly::Assembly(PEAssembly* pPEAssembly, DebuggerAssemblyControlFlags debugge // and the assembly is safely destructable. Whether this function throws or succeeds, // it must leave the Assembly in a safely destructable state. //---------------------------------------------------------------------------------------------- -void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator) +void Assembly::Init(AllocMemTracker *pamTracker) { STANDARD_VM_CONTRACT; - if (IsSystem()) - { - _ASSERTE(pLoaderAllocator == NULL); // pLoaderAllocator may only be non-null for collectible types - m_pLoaderAllocator = SystemDomain::GetGlobalLoaderAllocator(); - } - else - { - if (!IsCollectible()) - { - // pLoaderAllocator will only be non-null for reflection emit assemblies - _ASSERTE((pLoaderAllocator == NULL) || (pLoaderAllocator == AppDomain::GetCurrentDomain()->GetLoaderAllocator())); - m_pLoaderAllocator = AppDomain::GetCurrentDomain()->GetLoaderAllocator(); - } - else - { - _ASSERTE(pLoaderAllocator != NULL); // ppLoaderAllocator must be non-null for collectible assemblies - - m_pLoaderAllocator = pLoaderAllocator; - } - } - _ASSERTE(m_pLoaderAllocator != NULL); - m_pClassLoader = new ClassLoader(this); m_pClassLoader->Init(pamTracker); @@ -321,13 +306,19 @@ void Assembly::Terminate( BOOL signalProfiler ) Assembly * Assembly::Create( PEAssembly * pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, - BOOL fIsCollectible, AllocMemTracker * pamTracker, LoaderAllocator * pLoaderAllocator) { - STANDARD_VM_CONTRACT; + CONTRACTL + { + STANDARD_VM_CHECK; + PRECONDITION(pPEAssembly != NULL); + PRECONDITION(pLoaderAllocator != NULL); + PRECONDITION(pLoaderAllocator->IsCollectible() || pLoaderAllocator == SystemDomain::GetGlobalLoaderAllocator()); + } + CONTRACTL_END - NewHolder pAssembly (new Assembly(pPEAssembly, debuggerFlags, fIsCollectible)); + NewHolder pAssembly (new Assembly(pPEAssembly, debuggerFlags, pLoaderAllocator)); #ifdef PROFILING_SUPPORTED { @@ -341,7 +332,7 @@ Assembly * Assembly::Create( EX_TRY #endif { - pAssembly->Init(pamTracker, pLoaderAllocator); + pAssembly->Init(pamTracker); } #ifdef PROFILING_SUPPORTED EX_HOOK diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index 701b0acfe54398..76b834c2683f6c 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -55,14 +55,14 @@ class Assembly friend class ClrDataAccess; private: - Assembly(PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, BOOL fIsCollectible); - void Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator); + Assembly(PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, LoaderAllocator* pLoaderAllocator); + void Init(AllocMemTracker *pamTracker); public: void StartUnload(); void Terminate( BOOL signalProfiler = TRUE ); - static Assembly *Create(PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, BOOL fIsCollectible, AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator); + static Assembly *Create(PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator); static void Initialize(); BOOL IsSystem() { WRAPPER_NO_CONTRACT; return m_pPEAssembly->IsSystem(); } diff --git a/src/coreclr/vm/domainassembly.cpp b/src/coreclr/vm/domainassembly.cpp index 64fb98569f10f8..f41c21e2c7b8f6 100644 --- a/src/coreclr/vm/domainassembly.cpp +++ b/src/coreclr/vm/domainassembly.cpp @@ -55,7 +55,7 @@ DomainAssembly::DomainAssembly(PEAssembly* pPEAssembly, LoaderAllocator* pLoader SetupDebuggingConfig(); // Create the Assembly - NewHolder assembly = Assembly::Create(GetPEAssembly(), GetDebuggerInfoBits(), IsCollectible(), memTracker, IsCollectible() ? GetLoaderAllocator() : NULL); + NewHolder assembly = Assembly::Create(pPEAssembly, GetDebuggerInfoBits(), memTracker, pLoaderAllocator); m_pAssembly = assembly.Extract(); m_pModule = m_pAssembly->GetModule();