Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some cleanups in Assembly/Loader area #57023

Merged
23 commits merged into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
be13383
rename GAC -> TPA
VSadov Aug 6, 2021
440bd6e
remove GetAvailableImageTypes
VSadov Aug 7, 2021
081f08c
Removed ICLRPrivAssembly
VSadov Aug 7, 2021
27d414c
Move Assembly:: Add/Release to .inl
VSadov Aug 7, 2021
767ef95
ApplicationContext is not implementing IUnknown
VSadov Aug 13, 2021
a984bd5
ApplicationContext does not need AppDomainId
VSadov Aug 13, 2021
5b2218c
ICLRPrivBinder is not a COM object
VSadov Aug 13, 2021
68a9b4c
Assembly is not a binder and should not have BindAssemblyByName
VSadov Aug 14, 2021
24ca380
ApplicationContext is always an embedded value and does not need refe…
VSadov Aug 14, 2021
6ed1653
Simplified GetBinderID and GetLoaderAllocator, since not COM
VSadov Aug 14, 2021
d435108
Moved AppContext to up to ICLRPrivBinder
VSadov Aug 16, 2021
eb3bc8e
Removed GetBinderHash
VSadov Aug 16, 2021
331eccb
Removed a couple now pointless AddRef/Release
VSadov Aug 16, 2021
a186ca4
virtualized BindUsingAssemblyName
VSadov Aug 16, 2021
a616b18
renamed ICLRPrivBinder --> AssemblyBinder
VSadov Aug 16, 2021
63ef26b
renamed BINDER_SPACE::AssemblyBinder --> BINDER_SPACE::AssemblyBinder…
VSadov Aug 16, 2021
8d61a2d
Merge CCoreCLRBinderHelper into AssemblyBinderCommon
VSadov Aug 16, 2021
61068b0
Rename CLRPrivBinderCoreCLR -->DefaultAssemblyBinder
VSadov Aug 16, 2021
f1ee77f
Renamed CLRPrivBinderAssemblyLoadContext --> CustomAssemblyBinder
VSadov Aug 16, 2021
9920bc9
Renamed PTR_ICLRPrivBinder --> PTR_AssemblyBinder
VSadov Aug 16, 2021
6cc9cef
Remove clrprivbinding_i.cpp
VSadov Aug 16, 2021
babbcd2
A few touch ups
VSadov Aug 16, 2021
b241939
fix Linux build
VSadov Aug 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/coreclr/binder/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ include_directories(BEFORE "inc")
set(BINDER_COMMON_SOURCES
applicationcontext.cpp
assembly.cpp
assemblybinder.cpp
assemblybindercommon.cpp
assemblyidentitycache.cpp
assemblyname.cpp
bindertracing.cpp
clrprivbindercoreclr.cpp
coreclrbindercommon.cpp
defaultassemblybinder.cpp
failurecache.cpp
stringlexer.cpp
textualidentityparser.cpp
Expand All @@ -23,7 +22,7 @@ set(BINDER_COMMON_HEADERS
inc/applicationcontext.inl
inc/assembly.hpp
inc/assembly.inl
inc/assemblybinder.hpp
inc/assemblybindercommon.hpp
inc/assemblyentry.hpp
inc/assemblyhashtraits.hpp
inc/assemblyidentity.hpp
Expand All @@ -36,8 +35,7 @@ set(BINDER_COMMON_HEADERS
inc/bindertracing.h
inc/bindresult.hpp
inc/bindresult.inl
inc/clrprivbindercoreclr.h
inc/coreclrbindercommon.h
inc/defaultassemblybinder.h
inc/failurecache.hpp
inc/failurecachehashtraits.hpp
inc/loadcontext.hpp
Expand All @@ -51,13 +49,13 @@ set(BINDER_COMMON_HEADERS
set(BINDER_SOURCES
${BINDER_COMMON_SOURCES}
activitytracker.cpp
clrprivbinderassemblyloadcontext.cpp
customassemblybinder.cpp
)

set(BINDER_HEADERS
${BINDER_COMMON_HEADERS}
inc/activitytracker.h
inc/clrprivbinderassemblyloadcontext.h
inc/customassemblybinder.h
inc/contextentry.hpp
)

Expand Down
53 changes: 3 additions & 50 deletions src/coreclr/binder/applicationcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,63 +23,17 @@ using namespace clr::fs;

namespace BINDER_SPACE
{
STDMETHODIMP ApplicationContext::QueryInterface(REFIID riid,
void **ppv)
{
HRESULT hr = S_OK;

if (ppv == NULL)
{
hr = E_POINTER;
}
else
{
if (IsEqualIID(riid, IID_IUnknown))
{
AddRef();
*ppv = static_cast<IUnknown *>(this);
}
else
{
*ppv = NULL;
hr = E_NOINTERFACE;
}
}

return hr;
}

STDMETHODIMP_(ULONG) ApplicationContext::AddRef()
{
return InterlockedIncrement(&m_cRef);
}

STDMETHODIMP_(ULONG) ApplicationContext::Release()
{
ULONG ulRef = InterlockedDecrement(&m_cRef);

if (ulRef == 0)
{
delete this;
}

return ulRef;
}

ApplicationContext::ApplicationContext()
{
m_cRef = 1;
m_dwAppDomainId = 0;
m_pExecutionContext = NULL;
m_pFailureCache = NULL;
m_contextCS = NULL;
m_pTrustedPlatformAssemblyMap = nullptr;
m_binderID = 0;
}

ApplicationContext::~ApplicationContext()
{
SAFE_RELEASE(m_pExecutionContext);
SAFE_DELETE(m_pExecutionContext);
SAFE_DELETE(m_pFailureCache);

if (m_contextCS != NULL)
Expand All @@ -93,11 +47,11 @@ namespace BINDER_SPACE
}
}

HRESULT ApplicationContext::Init(UINT_PTR binderID)
HRESULT ApplicationContext::Init()
{
HRESULT hr = S_OK;

ReleaseHolder<ExecutionContext> pExecutionContext;
NewHolder<ExecutionContext> pExecutionContext;

FailureCache *pFailureCache = NULL;

Expand All @@ -121,7 +75,6 @@ namespace BINDER_SPACE
m_pFailureCache = pFailureCache;
}

m_binderID = binderID;
Exit:
return hr;
}
Expand Down
78 changes: 6 additions & 72 deletions src/coreclr/binder/assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,49 +24,6 @@ namespace BINDER_SPACE
}
};

STDMETHODIMP Assembly::QueryInterface(REFIID riid,
void **ppv)
{
HRESULT hr = S_OK;

if (ppv == NULL)
{
hr = E_POINTER;
}
else
{
if (IsEqualIID(riid, IID_IUnknown))
{
AddRef();
*ppv = static_cast<IUnknown *>(this);
}
else
{
*ppv = NULL;
hr = E_NOINTERFACE;
}
}

return hr;
}

STDMETHODIMP_(ULONG) Assembly::AddRef()
{
return InterlockedIncrement(&m_cRef);
}

STDMETHODIMP_(ULONG) Assembly::Release()
{
ULONG ulRef = InterlockedDecrement(&m_cRef);

if (ulRef == 0)
{
delete this;
}

return ulRef;
}

Assembly::Assembly()
{
m_cRef = 1;
Expand Down Expand Up @@ -103,7 +60,7 @@ namespace BINDER_SPACE
PEImage *pPEImage,
PEImage *pNativePEImage,
SString &assemblyPath,
BOOL fIsInGAC)
BOOL fIsInTPA)
{
HRESULT hr = S_OK;

Expand All @@ -113,15 +70,15 @@ namespace BINDER_SPACE
// Get assembly name def from meta data import and store it for later refs access
IF_FAIL_GO(pAssemblyName->Init(pIMetaDataAssemblyImport, PeKind));
SetMDImport(pIMetaDataAssemblyImport);
if (!fIsInGAC)
if (!fIsInTPA)
{
GetPath().Set(assemblyPath);
}

// Safe architecture for validation
PEKIND kAssemblyArchitecture;
kAssemblyArchitecture = pAssemblyName->GetArchitecture();
SetIsInGAC(fIsInGAC);
SetIsInTPA(fIsInTPA);
SetPEImage(pPEImage);
SetNativePEImage(pNativePEImage);
pAssemblyName->SetIsDefinition(TRUE);
Expand Down Expand Up @@ -174,40 +131,17 @@ namespace BINDER_SPACE
}

// --------------------------------------------------------------------
// ICLRPrivAssembly methods
// BINDER_SPACE::Assembly methods
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this comment - I don't think it makes sense anymore.

Copy link
Member Author

@VSadov VSadov Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Some comments moved with the code and the style could be out of place in the new location or may not be very helpful. Some were not very helpful to start with and likely were forced by some kind of "every method/class must have a comment" policy.
'An assembly represents a particular set of bits.' is my favorite :-).

// --------------------------------------------------------------------
LPCWSTR Assembly::GetSimpleName()
{
AssemblyName *pAsmName = GetAssemblyName();
return (pAsmName == nullptr ? nullptr : (LPCWSTR)pAsmName->GetSimpleName());
}

HRESULT Assembly::BindAssemblyByName(AssemblyNameData *pAssemblyNameData, ICLRPrivAssembly ** ppAssembly)
{
return (m_pBinder == NULL) ? E_FAIL : m_pBinder->BindAssemblyByName(pAssemblyNameData, ppAssembly);
}

HRESULT Assembly::GetBinderID(UINT_PTR *pBinderId)
{
return (m_pBinder == NULL) ? E_FAIL : m_pBinder->GetBinderID(pBinderId);
}

HRESULT Assembly::GetLoaderAllocator(LPVOID* pLoaderAllocator)
{
return (m_pBinder == NULL) ? E_FAIL : m_pBinder->GetLoaderAllocator(pLoaderAllocator);
}

HRESULT Assembly::GetAvailableImageTypes(
LPDWORD pdwImageTypes)
AssemblyLoaderAllocator* Assembly::GetLoaderAllocator()
{
HRESULT hr = E_FAIL;

if(pdwImageTypes == nullptr)
return E_INVALIDARG;

*pdwImageTypes = ASSEMBLY_IMAGE_TYPE_ASSEMBLY;

return S_OK;
return m_pBinder ? m_pBinder->GetLoaderAllocator() : NULL;
}
}

Loading