Skip to content

Remove BaseDomain use in LoaderAllocator and event tracing helpers #107481

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

Merged
merged 5 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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/inc/eventtracebase.h
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,6 @@ class Module;
class Assembly;
class MethodDesc;
class MethodTable;
class BaseDomain;
class AppDomain;
class SString;
class CrawlFrame;
Expand Down Expand Up @@ -750,12 +749,11 @@ namespace ETW
#ifdef FEATURE_EVENT_TRACE
static VOID SendThreadRundownEvent();
static VOID SendGCRundownEvent();
static VOID IterateDomain(BaseDomain *pDomain, DWORD enumerationOptions);
static VOID IterateAppDomain(AppDomain * pAppDomain, DWORD enumerationOptions);
static VOID IterateCollectibleLoaderAllocator(AssemblyLoaderAllocator *pLoaderAllocator, DWORD enumerationOptions);
static VOID IterateAssembly(Assembly *pAssembly, DWORD enumerationOptions);
static VOID IterateModule(Module *pModule, DWORD enumerationOptions);
static VOID EnumerationHelper(Module *moduleFilter, BaseDomain *domainFilter, DWORD enumerationOptions);
static VOID EnumerationHelper(Module *moduleFilter, AppDomain *domainFilter, DWORD enumerationOptions);
static DWORD GetEnumerationOptionsFromRuntimeKeywords();
public:
typedef union _EnumerationStructs
Expand Down Expand Up @@ -839,7 +837,7 @@ namespace ETW
static VOID SendModuleEvent(Module *pModule, DWORD dwEventOptions, BOOL bFireDomainModuleEvents=FALSE);
static ULONG SendModuleRange(_In_ Module *pModule, _In_ DWORD dwEventOptions);
static VOID SendAssemblyEvent(Assembly *pAssembly, DWORD dwEventOptions);
static VOID SendDomainEvent(BaseDomain *pBaseDomain, DWORD dwEventOptions, LPCWSTR wszFriendlyName=NULL);
static VOID SendDomainEvent(AppDomain *pDomain, DWORD dwEventOptions, LPCWSTR wszFriendlyName=NULL);
public:
typedef union _LoaderStructs
{
Expand Down Expand Up @@ -877,9 +875,9 @@ namespace ETW

}LoaderStructs;

static VOID DomainLoadReal(BaseDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName=NULL);
static VOID DomainLoadReal(AppDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName=NULL);

static VOID DomainLoad(BaseDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName = NULL)
static VOID DomainLoad(AppDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName = NULL)
{
if (ETW_PROVIDER_ENABLED(MICROSOFT_WINDOWS_DOTNETRUNTIME_PROVIDER))
{
Expand All @@ -892,7 +890,7 @@ namespace ETW
static VOID ModuleLoad(Module *pModule, LONG liReportedSharedModule);
#else
public:
static VOID DomainLoad(BaseDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName=NULL) {};
static VOID DomainLoad(AppDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName=NULL) {};
static VOID DomainUnload(AppDomain *pDomain) {};
static VOID CollectibleLoaderAllocatorUnload(AssemblyLoaderAllocator *pLoaderAllocator) {};
static VOID ModuleLoad(Module *pModule, LONG liReportedSharedModule) {};
Expand All @@ -904,7 +902,7 @@ namespace ETW
{
friend class ETW::EnumerationLog;
#ifdef FEATURE_EVENT_TRACE
static VOID SendEventsForJitMethods(BaseDomain *pDomainFilter, LoaderAllocator *pLoaderAllocatorFilter, DWORD dwEventOptions);
static VOID SendEventsForJitMethods(AppDomain *pDomainFilter, LoaderAllocator *pLoaderAllocatorFilter, DWORD dwEventOptions);
static VOID SendEventsForJitMethodsHelper(
LoaderAllocator *pLoaderAllocatorFilter,
DWORD dwEventOptions,
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/vm/appdomain.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,7 @@ class SystemDomain : public BaseDomain

m_pDelayedUnloadListOfLoaderAllocators=NULL;

m_GlobalAllocator.Init(this);
m_GlobalAllocator.Init();
}
#endif

Expand All @@ -2023,7 +2023,6 @@ class SystemDomain : public BaseDomain

GlobalLoaderAllocator m_GlobalAllocator;


InlineSString<100> m_BaseLibrary;

InlineSString<100> m_SystemDirectory;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ Assembly *Assembly::CreateDynamic(AssemblyBinder* pBinder, NativeAssemblyNamePar

// Some of the initialization functions are not virtual. Call through the derived class
// to prevent calling the base class version.
pCollectibleLoaderAllocator->Init(pDomain);
pCollectibleLoaderAllocator->Init();

// Setup the managed proxy now, but do not actually transfer ownership to it.
// Once everything is setup and nothing can fail anymore, the ownership will be
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/assemblynative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ extern "C" INT_PTR QCALLTYPE AssemblyNative_InitializeAssemblyLoadContext(INT_PT
GCX_PREEMP();
// Some of the initialization functions are not virtual. Call through the derived class
// to prevent calling the base class version.
loaderAllocator->Init(pCurDomain);
loaderAllocator->Init();
loaderAllocator->InitVirtualCallStubManager();

// Setup the managed proxy now, but do not actually transfer ownership to it.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/corhost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ HRESULT CorHost2::CreateAppDomainWithManager(

BEGIN_EXTERNAL_ENTRYPOINT(&hr);

AppDomain* pDomain = SystemDomain::System()->DefaultDomain();
AppDomain* pDomain = AppDomain::GetCurrentDomain();

pDomain->SetFriendlyName(wszFriendlyName);

Expand Down
76 changes: 20 additions & 56 deletions src/coreclr/vm/eventtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2964,7 +2964,7 @@ VOID ETW::ExceptionLog::ExceptionFilterEnd()
/****************************************************************************/
/* This is called by the runtime when a domain is loaded */
/****************************************************************************/
VOID ETW::LoaderLog::DomainLoadReal(BaseDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName)
VOID ETW::LoaderLog::DomainLoadReal(AppDomain *pDomain, _In_opt_ LPWSTR wszFriendlyName)
{
CONTRACTL {
NOTHROW;
Expand Down Expand Up @@ -3926,20 +3926,20 @@ VOID ETW::EnumerationLog::ProcessShutdown()
/****************************************************************************/
/* This routine is used to send a domain load/unload or rundown event */
/****************************************************************************/
VOID ETW::LoaderLog::SendDomainEvent(BaseDomain *pBaseDomain, DWORD dwEventOptions, LPCWSTR wszFriendlyName)
VOID ETW::LoaderLog::SendDomainEvent(AppDomain *pDomain, DWORD dwEventOptions, LPCWSTR wszFriendlyName)
{
CONTRACTL {
THROWS;
GC_TRIGGERS;
} CONTRACTL_END;

if(!pBaseDomain)
if(!pDomain)
return;

PCWSTR szDtraceOutput1=W("");
BOOL bIsAppDomain = pBaseDomain->IsAppDomain();
BOOL bIsAppDomain = TRUE;

ULONGLONG ullDomainId = (ULONGLONG)pBaseDomain;
ULONGLONG ullDomainId = (ULONGLONG)pDomain;
ULONG ulDomainFlags = ETW::LoaderLog::LoaderStructs::DefaultDomain | ETW::LoaderLog::LoaderStructs::ExecutableDomain;

LPCWSTR wsEmptyString = W("");
Expand All @@ -3949,7 +3949,7 @@ VOID ETW::LoaderLog::SendDomainEvent(BaseDomain *pBaseDomain, DWORD dwEventOptio
if(wszFriendlyName)
lpswzDomainName = (PWCHAR)wszFriendlyName;
else
lpswzDomainName = (PWCHAR)pBaseDomain->AsAppDomain()->GetFriendlyName();
lpswzDomainName = (PWCHAR)pDomain->GetFriendlyName();

/* prepare events args for ETW and ETM */
szDtraceOutput1 = (PCWSTR)lpswzDomainName;
Expand Down Expand Up @@ -5243,7 +5243,7 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(LoaderAllocator *pLoaderAlloc
/****************************************************************************/
// Code review indicates this method is never called with both filters NULL. Ideally we would
// assert this and change the comment above, but given I am making a change late in the release I am being cautious
VOID ETW::MethodLog::SendEventsForJitMethods(BaseDomain *pDomainFilter, LoaderAllocator *pLoaderAllocatorFilter, DWORD dwEventOptions)
VOID ETW::MethodLog::SendEventsForJitMethods(AppDomain *pDomainFilter, LoaderAllocator *pLoaderAllocatorFilter, DWORD dwEventOptions)
{
CONTRACTL {
NOTHROW;
Expand Down Expand Up @@ -5337,61 +5337,32 @@ VOID ETW::MethodLog::SendEventsForJitMethods(BaseDomain *pDomainFilter, LoaderAl

//---------------------------------------------------------------------------------------
//
// Wrapper around IterateDomain, which locks the AppDomain to be <
// STAGE_FINALIZED until the iteration is complete.
// This routine fires ETW events for
// Domain
// Assemblies in them
// Modules in them
// JIT methods in them
// R2R methods in them
// based on enumeration options
//
// Arguments:
// pAppDomain - AppDomain to iterate
// enumerationOptions - Flags indicating what to enumerate. Just passed
// straight through to IterateDomain
// pDomain - AppDomain to iterate
// enumerationOptions - Flags indicating what to enumerate.
//
VOID ETW::EnumerationLog::IterateAppDomain(AppDomain * pAppDomain, DWORD enumerationOptions)
VOID ETW::EnumerationLog::IterateAppDomain(AppDomain * pDomain, DWORD enumerationOptions)
{
CONTRACTL
{
THROWS;
GC_TRIGGERS;
PRECONDITION(pAppDomain != NULL);
PRECONDITION(pDomain != NULL);
}
CONTRACTL_END;

// Hold the system domain lock during the entire iteration, so we can
// ensure the App Domain does not get finalized until we're all done
SystemDomain::LockHolder lh;

// Now it's safe to do the iteration
IterateDomain(pAppDomain, enumerationOptions);
}

/********************************************************************************/
/* This routine fires ETW events for
Domain,
Assemblies in them,
DomainModule's in them,
Modules in them,
JIT methods in them,
and the NGEN methods in them
based on enumerationOptions.*/
/********************************************************************************/
VOID ETW::EnumerationLog::IterateDomain(BaseDomain *pDomain, DWORD enumerationOptions)
{
CONTRACTL {
THROWS;
GC_TRIGGERS;
PRECONDITION(pDomain != NULL);
} CONTRACTL_END;

#if defined(_DEBUG) && !defined(DACCESS_COMPILE)
// Do not call IterateDomain() directly with an AppDomain. Use
// IterateAppDomain(), which wraps this function with a hold on the
// SystemDomain lock, which ensures pDomain's type data doesn't disappear
// on us.
if (pDomain->IsAppDomain())
{
_ASSERTE(SystemDomain::IsUnderDomainLock());
}
#endif // defined(_DEBUG) && !defined(DACCESS_COMPILE)

EX_TRY
{
// DC Start events for Domain
Expand Down Expand Up @@ -5603,7 +5574,7 @@ VOID ETW::EnumerationLog::IterateModule(Module *pModule, DWORD enumerationOption
//

// static
VOID ETW::EnumerationLog::EnumerationHelper(Module *moduleFilter, BaseDomain *domainFilter, DWORD enumerationOptions)
VOID ETW::EnumerationLog::EnumerationHelper(Module *moduleFilter, AppDomain *domainFilter, DWORD enumerationOptions)
{
CONTRACTL {
THROWS;
Expand Down Expand Up @@ -5638,14 +5609,7 @@ VOID ETW::EnumerationLog::EnumerationHelper(Module *moduleFilter, BaseDomain *do
{
if(domainFilter)
{
if(domainFilter->IsAppDomain())
{
ETW::EnumerationLog::IterateAppDomain(domainFilter->AsAppDomain(), enumerationOptions);
}
else
{
ETW::EnumerationLog::IterateDomain(domainFilter, enumerationOptions);
}
ETW::EnumerationLog::IterateAppDomain(domainFilter, enumerationOptions);
}
else
{
Expand Down
22 changes: 11 additions & 11 deletions src/coreclr/vm/loaderallocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ LoaderAllocator::LoaderAllocator(bool collectible) :
m_fUnloaded = false;
m_fMarked = false;
m_pLoaderAllocatorDestroyNext = NULL;
m_pDomain = NULL;
m_pCodeHeapInitialAlloc = NULL;
m_pVSDHeapInitialAlloc = NULL;
m_pLastUsedCodeHeap = NULL;
Expand Down Expand Up @@ -506,13 +505,16 @@ void LoaderAllocator::GCLoaderAllocators(LoaderAllocator* pOriginalLoaderAllocat
THROWS;
GC_TRIGGERS;
MODE_PREEMPTIVE;
PRECONDITION(pOriginalLoaderAllocator != NULL);
PRECONDITION(pOriginalLoaderAllocator->IsCollectible());
PRECONDITION(pOriginalLoaderAllocator->Id()->GetType() == LAT_Assembly);
}
CONTRACTL_END;

// List of LoaderAllocators being deleted
LoaderAllocator * pFirstDestroyedLoaderAllocator = NULL;

AppDomain* pAppDomain = (AppDomain*)pOriginalLoaderAllocator->GetDomain();
AppDomain* pAppDomain = AppDomain::GetCurrentDomain();

// Collect all LoaderAllocators that don't have anymore DomainAssemblies alive
// Note: that it may not collect our pOriginalLoaderAllocator in case this
Expand Down Expand Up @@ -1059,12 +1061,10 @@ void LoaderAllocator::ActivateManagedTracking()
#define COLLECTIBLE_CODEHEAP_SIZE (10 * GetOsPageSize())
#define COLLECTIBLE_VIRTUALSTUBDISPATCH_HEAP_SPACE (2 * GetOsPageSize())

void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory)
void LoaderAllocator::Init(BYTE *pExecutableHeapMemory)
{
STANDARD_VM_CONTRACT;

m_pDomain = pDomain;

m_crstLoaderAllocator.Init(CrstLoaderAllocator, (CrstFlags)CRST_UNSAFE_COOPGC);
m_crstLoaderAllocatorHandleTable.Init(CrstLeafLock, (CrstFlags)CRST_UNSAFE_COOPGC);
m_InteropDataCrst.Init(CrstInteropData, CRST_REENTRANCY);
Expand Down Expand Up @@ -1446,7 +1446,6 @@ void LoaderAllocator::Terminate()
m_pFuncPtrStubs = NULL;
}

// This was the block reserved by BaseDomain::Init for the loaderheaps.
if (m_InitialReservedMemForLoaderHeaps)
{
ExecutableAllocator::Instance()->Release(m_InitialReservedMemForLoaderHeaps);
Expand Down Expand Up @@ -1725,15 +1724,15 @@ void DomainAssemblyIterator::operator++()

#ifndef DACCESS_COMPILE

void AssemblyLoaderAllocator::Init(AppDomain* pAppDomain)
void AssemblyLoaderAllocator::Init()
{
m_Id.Init();

// This is CRST_UNSAFE_ANYMODE to enable registering/unregistering dependent handles to native objects without changing the
// GC mode, in case the caller requires that
m_dependentHandleToNativeObjectSetCrst.Init(CrstLeafLock, CRST_UNSAFE_ANYMODE);

LoaderAllocator::Init((BaseDomain *)pAppDomain);
LoaderAllocator::Init(NULL);
if (IsCollectible())
{
// TODO: the ShuffleThunkCache should really be using the m_pStubHeap, however the unloadability support
Expand Down Expand Up @@ -1933,10 +1932,11 @@ void AssemblyLoaderAllocator::CleanupHandles()
NOTHROW;
MODE_ANY;
CAN_TAKE_LOCK;
PRECONDITION(IsCollectible());
PRECONDITION(Id()->GetType() == LAT_Assembly);
}
CONTRACTL_END;

_ASSERTE(GetDomain()->IsAppDomain());

if (m_hLoaderAllocatorObjectHandle != NULL)
{
Expand Down Expand Up @@ -2058,12 +2058,12 @@ void LoaderAllocator::CleanupFailedTypeInit()
return;
}

_ASSERTE(GetDomain()->IsAppDomain());
_ASSERTE(Id()->GetType() == LAT_Assembly);

// This method doesn't take a lock around loader allocator state access, because
// it's supposed to be called only during cleanup. However, the domain-level state
// might be accessed by multiple threads.
ListLock *pLock = ((AppDomain*)GetDomain())->GetClassInitLock();
ListLock *pLock = AppDomain::GetCurrentDomain()->GetClassInitLock();

while (!m_failedTypeInitCleanupList.IsEmpty())
{
Expand Down
9 changes: 3 additions & 6 deletions src/coreclr/vm/loaderallocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,6 @@ class LoaderAllocator
BYTE *GetVSDHeapInitialBlock(DWORD *pSize);
BYTE *GetCodeHeapInitialBlock(const BYTE * loAddr, const BYTE * hiAddr, DWORD minimumSize, DWORD *pSize);

BaseDomain *m_pDomain;

// ExecutionManager caches
void * m_pLastUsedCodeHeap;
void * m_pLastUsedDynamicCodeHeap;
Expand Down Expand Up @@ -735,9 +733,8 @@ class LoaderAllocator

LoaderAllocator(bool collectible);
virtual ~LoaderAllocator();
BaseDomain *GetDomain() { LIMITED_METHOD_CONTRACT; return m_pDomain; }
virtual BOOL CanUnload() = 0;
void Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory = NULL);
void Init(BYTE *pExecutableHeapMemory);
void Terminate();
virtual void ReleaseManagedAssemblyLoadContext() {}

Expand Down Expand Up @@ -887,7 +884,7 @@ class GlobalLoaderAllocator : public LoaderAllocator
LoaderAllocatorID m_Id;

public:
void Init(BaseDomain *pDomain);
void Init();
GlobalLoaderAllocator() : LoaderAllocator(false), m_Id(LAT_Global, (void*)1) { LIMITED_METHOD_CONTRACT;};
virtual LoaderAllocatorID* Id();
virtual BOOL CanUnload();
Expand All @@ -913,7 +910,7 @@ class AssemblyLoaderAllocator : public LoaderAllocator
, m_binderToRelease(NULL)
#endif
{ LIMITED_METHOD_CONTRACT; }
void Init(AppDomain *pAppDomain);
void Init();
virtual BOOL CanUnload();

void AddDomainAssembly(DomainAssembly *pDomainAssembly)
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/loaderallocator.inl
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ inline bool LoaderAllocator::IsExposedObjectLive()
return !ObjectHandleIsNull(m_hLoaderAllocatorObjectHandle);
}

inline void GlobalLoaderAllocator::Init(BaseDomain *pDomain)
inline void GlobalLoaderAllocator::Init()
{
LoaderAllocator::Init(pDomain, m_ExecutableHeapInstance);
LoaderAllocator::Init(m_ExecutableHeapInstance);
}

inline BOOL LoaderAllocatorID::Equals(LoaderAllocatorID *pId)
Expand Down
Loading