Skip to content

Commit

Permalink
Store Assembly instead of DomainAssembly in `AssemblySpecBindingC…
Browse files Browse the repository at this point in the history
…ache` and `BINDER_SPACE::Assembly` (dotnet#107799)
  • Loading branch information
elinor-fung authored and jtschuster committed Sep 17, 2024
1 parent 3ec917f commit ecf6115
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 47 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/binder/assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace BINDER_SPACE
m_pAssemblyName = NULL;
m_isInTPA = false;
m_pBinder = NULL;
m_domainAssembly = NULL;
m_runtimeAssembly = NULL;
}

Assembly::~Assembly()
Expand Down
14 changes: 6 additions & 8 deletions src/coreclr/binder/inc/assembly.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
#include "bundle.h"
#include <assemblybinderutil.h>

class DomainAssembly;

namespace BINDER_SPACE
{
// BINDER_SPACE::Assembly represents a result of binding to an actual assembly (PEImage)
Expand All @@ -56,15 +54,15 @@ namespace BINDER_SPACE
return m_pBinder;
}

DomainAssembly* GetDomainAssembly()
::Assembly* GetRuntimeAssembly()
{
return m_domainAssembly;
return m_runtimeAssembly;
}

void SetDomainAssembly(DomainAssembly* value)
void SetRuntimeAssembly(::Assembly* value)
{
_ASSERTE(value == NULL || m_domainAssembly == NULL);
m_domainAssembly = value;
_ASSERTE(value == NULL || m_runtimeAssembly == NULL);
m_runtimeAssembly = value;
}

private:
Expand All @@ -73,7 +71,7 @@ namespace BINDER_SPACE
AssemblyName *m_pAssemblyName;
PTR_AssemblyBinder m_pBinder;
bool m_isInTPA;
DomainAssembly *m_domainAssembly;
::Assembly *m_runtimeAssembly;

#if !defined(DACCESS_COMPILE)
inline void SetBinder(AssemblyBinder *pBinder)
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2516,11 +2516,11 @@ Assembly *AppDomain::LoadAssemblyInternal(AssemblySpec* pIdentity,
{
AssemblySpec spec;
spec.InitializeSpec(result->GetPEAssembly());
GetAppDomain()->AddAssemblyToCache(&spec, result->GetDomainAssembly());
GetAppDomain()->AddAssemblyToCache(&spec, result);
}
else
{
GetAppDomain()->AddAssemblyToCache(pIdentity, result->GetDomainAssembly());
GetAppDomain()->AddAssemblyToCache(pIdentity, result);
}

RETURN result;
Expand Down Expand Up @@ -2763,10 +2763,10 @@ Assembly * AppDomain::FindAssembly(PEAssembly * pPEAssembly, FindAssemblyOptions

if (pPEAssembly->HasHostAssembly())
{
DomainAssembly * pDA = pPEAssembly->GetHostAssembly()->GetDomainAssembly();
if (pDA != nullptr && (pDA->GetAssembly()->IsLoaded() || (includeFailedToLoad && pDA->GetAssembly()->IsError())))
Assembly * pAssembly = pPEAssembly->GetHostAssembly()->GetRuntimeAssembly();
if (pAssembly != nullptr && (pAssembly->IsLoaded() || (includeFailedToLoad && pAssembly->IsError())))
{
return pDA->GetAssembly();
return pAssembly;
}
return nullptr;
}
Expand Down Expand Up @@ -2910,7 +2910,7 @@ BOOL AppDomain::AddFileToCache(AssemblySpec* pSpec, PEAssembly * pPEAssembly)
return m_AssemblyCache.StorePEAssembly(pSpec, pPEAssembly);
}

BOOL AppDomain::AddAssemblyToCache(AssemblySpec* pSpec, DomainAssembly *pAssembly)
BOOL AppDomain::AddAssemblyToCache(AssemblySpec* pSpec, Assembly *pAssembly)
{
CONTRACTL
{
Expand Down Expand Up @@ -3027,7 +3027,7 @@ BOOL AppDomain::RemoveFileFromCache(PEAssembly * pPEAssembly)
return TRUE;
}

BOOL AppDomain::RemoveAssemblyFromCache(DomainAssembly* pAssembly)
BOOL AppDomain::RemoveAssemblyFromCache(Assembly* pAssembly)
{
CONTRACTL
{
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/appdomain.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ class AppDomain final
//****************************************************************************************
// Returns and Inserts assemblies into a lookup cache based on the binding information
// in the AssemblySpec. There can be many AssemblySpecs to a single assembly.
DomainAssembly* FindCachedAssembly(AssemblySpec* pSpec, BOOL fThrow=TRUE)
Assembly* FindCachedAssembly(AssemblySpec* pSpec, BOOL fThrow=TRUE)
{
WRAPPER_NO_CONTRACT;
return m_AssemblyCache.LookupAssembly(pSpec, fThrow);
Expand All @@ -1126,8 +1126,8 @@ class AppDomain final
BOOL AddFileToCache(AssemblySpec* pSpec, PEAssembly *pPEAssembly);
BOOL RemoveFileFromCache(PEAssembly *pPEAssembly);

BOOL AddAssemblyToCache(AssemblySpec* pSpec, DomainAssembly *pAssembly);
BOOL RemoveAssemblyFromCache(DomainAssembly* pAssembly);
BOOL AddAssemblyToCache(AssemblySpec* pSpec, Assembly *pAssembly);
BOOL RemoveAssemblyFromCache(Assembly* pAssembly);

BOOL AddExceptionToCache(AssemblySpec* pSpec, Exception *ex);
void AddUnmanagedImageToCache(LPCWSTR libraryName, NATIVE_LIBRARY_HANDLE hMod);
Expand Down
22 changes: 11 additions & 11 deletions src/coreclr/vm/assemblyspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// assertions. The problem is that the real LookupAssembly can throw an OOM
// simply because it can't allocate scratch space. For the sake of asserting,
// we can treat those as successful lookups.
BOOL UnsafeVerifyLookupAssembly(AssemblySpecBindingCache *pCache, AssemblySpec *pSpec, DomainAssembly *pComparator)
BOOL UnsafeVerifyLookupAssembly(AssemblySpecBindingCache *pCache, AssemblySpec *pSpec, Assembly *pComparator)
{
STATIC_CONTRACT_NOTHROW;
STATIC_CONTRACT_GC_TRIGGERS;
Expand Down Expand Up @@ -372,14 +372,14 @@ Assembly *AssemblySpec::LoadAssembly(FileLoadLevel targetLevel,
ETWOnStartup (LoaderCatchCall_V1, LoaderCatchCallEnd_V1);
AppDomain* pDomain = GetAppDomain();

DomainAssembly* domainAssembly = pDomain->FindCachedAssembly(this);
if (domainAssembly)
Assembly* assembly = pDomain->FindCachedAssembly(this);
if (assembly)
{
BinderTracing::AssemblyBindOperation bindOperation(this);
bindOperation.SetResult(domainAssembly->GetPEAssembly(), true /*cached*/);
bindOperation.SetResult(assembly->GetPEAssembly(), true /*cached*/);

pDomain->LoadAssembly(domainAssembly->GetAssembly(), targetLevel);
RETURN domainAssembly->GetAssembly();
pDomain->LoadAssembly(assembly, targetLevel);
RETURN assembly;
}

PEAssemblyHolder pFile(pDomain->BindAssemblySpec(this, fThrowOnFileNotFound));
Expand Down Expand Up @@ -661,10 +661,10 @@ BOOL AssemblySpecBindingCache::Contains(AssemblySpec *pSpec)
return (LookupInternal(pSpec, TRUE) != (AssemblyBinding *) INVALIDENTRY);
}

DomainAssembly *AssemblySpecBindingCache::LookupAssembly(AssemblySpec *pSpec,
Assembly *AssemblySpecBindingCache::LookupAssembly(AssemblySpec *pSpec,
BOOL fThrow /*=TRUE*/)
{
CONTRACT(DomainAssembly *)
CONTRACT(Assembly *)
{
INSTANCE_CHECK;
if (fThrow) {
Expand Down Expand Up @@ -830,7 +830,7 @@ class AssemblyBindingHolder
// 2 -> 4


BOOL AssemblySpecBindingCache::StoreAssembly(AssemblySpec *pSpec, DomainAssembly *pAssembly)
BOOL AssemblySpecBindingCache::StoreAssembly(AssemblySpec *pSpec, Assembly *pAssembly)
{
CONTRACT(BOOL)
{
Expand Down Expand Up @@ -866,7 +866,7 @@ BOOL AssemblySpecBindingCache::StoreAssembly(AssemblySpec *pSpec, DomainAssembly
}

entry = abHolder.CreateAssemblyBinding(pHeap);
entry->Init(pSpec,pAssembly->GetPEAssembly(),pAssembly,NULL,pHeap, abHolder.GetPamTracker());
entry->Init(pSpec, pAssembly->GetPEAssembly(), pAssembly, NULL, pHeap, abHolder.GetPamTracker());

m_map.InsertValue(key, entry);

Expand Down Expand Up @@ -1048,7 +1048,7 @@ BOOL AssemblySpecBindingCache::StoreException(AssemblySpec *pSpec, Exception* pE
}
}

BOOL AssemblySpecBindingCache::RemoveAssembly(DomainAssembly* pAssembly)
BOOL AssemblySpecBindingCache::RemoveAssembly(Assembly* pAssembly)
{
CONTRACT(BOOL)
{
Expand Down
21 changes: 10 additions & 11 deletions src/coreclr/vm/assemblyspec.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ class AssemblySpecBindingCache
delete m_pException;
};

inline DomainAssembly* GetAssembly(){ LIMITED_METHOD_CONTRACT; return m_pAssembly;};
inline void SetAssembly(DomainAssembly* pAssembly){ LIMITED_METHOD_CONTRACT; m_pAssembly=pAssembly;};
inline Assembly* GetAssembly(){ LIMITED_METHOD_CONTRACT; return m_pAssembly; };
inline void SetAssembly(Assembly* pAssembly){ LIMITED_METHOD_CONTRACT; m_pAssembly = pAssembly; };
inline PEAssembly* GetFile(){ LIMITED_METHOD_CONTRACT; return m_pPEAssembly;};
inline BOOL IsError(){ LIMITED_METHOD_CONTRACT; return (m_exceptionType!=EXTYPE_NONE);};

Expand All @@ -344,7 +344,7 @@ class AssemblySpecBindingCache
default: _ASSERTE(!"Unexpected exception type");
}
};
inline void Init(AssemblySpec* pSpec, PEAssembly* pPEAssembly, DomainAssembly* pAssembly, Exception* pEx, LoaderHeap *pHeap, AllocMemTracker *pamTracker)
inline void Init(AssemblySpec* pSpec, PEAssembly* pPEAssembly, Assembly* pAssembly, Exception* pEx, LoaderHeap *pHeap, AllocMemTracker *pamTracker)
{
CONTRACTL
{
Expand All @@ -354,7 +354,7 @@ class AssemblySpecBindingCache
}
CONTRACTL_END;

InitInternal(pSpec,pPEAssembly,pAssembly);
InitInternal(pSpec, pPEAssembly, pAssembly);
if (pHeap != NULL)
{
m_spec.CloneFieldsToLoaderHeap(pHeap, pamTracker);
Expand All @@ -364,7 +364,6 @@ class AssemblySpecBindingCache
m_spec.CloneFields();
}
InitException(pEx);

}

inline HRESULT GetHR()
Expand Down Expand Up @@ -423,7 +422,7 @@ class AssemblySpecBindingCache
};
protected:

inline void InitInternal(AssemblySpec* pSpec, PEAssembly* pPEAssembly, DomainAssembly* pAssembly )
inline void InitInternal(AssemblySpec* pSpec, PEAssembly* pPEAssembly, Assembly* pAssembly )
{
WRAPPER_NO_CONTRACT;
m_spec.CopyFrom(pSpec);
Expand All @@ -436,7 +435,7 @@ class AssemblySpecBindingCache

AssemblySpec m_spec;
PEAssembly *m_pPEAssembly;
DomainAssembly *m_pAssembly;
Assembly *m_pAssembly;
enum{
EXTYPE_NONE = 0x00000000,
EXTYPE_HR = 0x00000001,
Expand Down Expand Up @@ -465,15 +464,15 @@ class AssemblySpecBindingCache

BOOL Contains(AssemblySpec *pSpec);

DomainAssembly *LookupAssembly(AssemblySpec *pSpec, BOOL fThrow=TRUE);
Assembly *LookupAssembly(AssemblySpec *pSpec, BOOL fThrow=TRUE);
PEAssembly *LookupFile(AssemblySpec *pSpec, BOOL fThrow = TRUE);

BOOL StoreAssembly(AssemblySpec *pSpec, DomainAssembly *pAssembly);
BOOL StoreAssembly(AssemblySpec *pSpec, Assembly *pAssembly);
BOOL StorePEAssembly(AssemblySpec *pSpec, PEAssembly *pPEAssembly);

BOOL StoreException(AssemblySpec *pSpec, Exception* pEx);

BOOL RemoveAssembly(DomainAssembly* pAssembly);
BOOL RemoveAssembly(Assembly* pAssembly);

DWORD Hash(AssemblySpec *pSpec)
{
Expand All @@ -482,7 +481,7 @@ class AssemblySpecBindingCache
}

#if !defined(DACCESS_COMPILE)
void GetAllAssemblies(SetSHash<PTR_DomainAssembly>& assemblyList)
void GetAllAssemblies(SetSHash<PTR_Assembly>& assemblyList)
{
PtrHashMap::PtrIterator i = m_map.begin();
while (!i.end())
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/ceeload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2327,10 +2327,10 @@ Module::GetAssemblyIfLoaded(
spec.SetBinder(pBinderForLoadedAssembly);
}

DomainAssembly * pDomainAssembly = AppDomain::GetCurrentDomain()->FindCachedAssembly(&spec, FALSE /*fThrow*/);
Assembly * pCachedAssembly = AppDomain::GetCurrentDomain()->FindCachedAssembly(&spec, FALSE /*fThrow*/);

if (pDomainAssembly && pDomainAssembly->GetAssembly()->IsLoaded())
pAssembly = pDomainAssembly->GetAssembly();
if (pCachedAssembly && pCachedAssembly->IsLoaded())
pAssembly = pCachedAssembly;

// Only store in the rid map if working with the current AppDomain.
if (fCanUseRidMap && pAssembly)
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/domainassembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ void Assembly::RegisterWithHostAssembly()

if (GetPEAssembly()->HasHostAssembly())
{
GetPEAssembly()->GetHostAssembly()->SetDomainAssembly(GetDomainAssembly());
GetPEAssembly()->GetHostAssembly()->SetRuntimeAssembly(this);
}
}

Expand All @@ -468,7 +468,7 @@ void Assembly::UnregisterFromHostAssembly()

if (GetPEAssembly()->HasHostAssembly())
{
GetPEAssembly()->GetHostAssembly()->SetDomainAssembly(nullptr);
GetPEAssembly()->GetHostAssembly()->SetRuntimeAssembly(nullptr);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/loaderallocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ LoaderAllocator * LoaderAllocator::GCLoaderAllocators_RemoveAssemblies(AppDomain
pAppDomain->RemoveFileFromCache(domainAssemblyToRemove->GetPEAssembly());
AssemblySpec spec;
spec.InitializeSpec(domainAssemblyToRemove->GetPEAssembly());
VERIFY(pAppDomain->RemoveAssemblyFromCache(domainAssemblyToRemove));
VERIFY(pAppDomain->RemoveAssemblyFromCache(domainAssemblyToRemove->GetAssembly()));
}

domainAssemblyIt++;
Expand Down

0 comments on commit ecf6115

Please sign in to comment.