Skip to content

Commit

Permalink
Make AppDomain::AssemblyIterator iterate over Assembly instead of Dom…
Browse files Browse the repository at this point in the history
…ainAssembly (dotnet#107768)
  • Loading branch information
elinor-fung authored and jtschuster committed Sep 17, 2024
1 parent ef02cd1 commit c0cafd7
Show file tree
Hide file tree
Showing 21 changed files with 101 additions and 123 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4421,12 +4421,12 @@ void DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(

// Pass the magical flags to the loader enumerator to get all Execution-only assemblies.
iterator = pAppDomain->IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoading | kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

while (iterator.Next(pDomainAssembly.This()))
while (iterator.Next(pAssembly.This()))
{
VMPTR_DomainAssembly vmDomainAssembly = VMPTR_DomainAssembly::NullPtr();
vmDomainAssembly.SetHostPtr(pDomainAssembly);
vmDomainAssembly.SetHostPtr(pAssembly->GetDomainAssembly());

fpCallback(vmDomainAssembly, pUserData);
}
Expand Down
6 changes: 2 additions & 4 deletions src/coreclr/debug/daccess/dacimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -528,14 +528,12 @@ struct ProcessModIter
kIncludeLoaded | kIncludeExecution));
}

CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
if (!m_assemIter.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
if (!m_assemIter.Next(pAssembly.This()))
{
return NULL;
}

// Note: DAC doesn't need to keep the assembly alive - see code:CollectibleAssemblyHolder#CAH_DAC
CollectibleAssemblyHolder<Assembly *> pAssembly = pDomainAssembly->GetAssembly();
return pAssembly;
}

Expand Down
15 changes: 7 additions & 8 deletions src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2855,11 +2855,11 @@ ClrDataAccess::GetAppDomainData(CLRDATA_ADDRESS addr, struct DacpAppDomainData *
// The assembly list is not valid in a closed appdomain.
AppDomain::AssemblyIterator i = pAppDomain->IterateAssembliesEx((AssemblyIterationFlags)(
kIncludeLoading | kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

while (i.Next(pDomainAssembly.This()))
while (i.Next(pAssembly.This()))
{
if (pDomainAssembly->GetAssembly()->IsLoaded())
if (pAssembly->IsLoaded())
{
appdomainData->AssemblyCount++;
}
Expand Down Expand Up @@ -2971,14 +2971,13 @@ ClrDataAccess::GetAssemblyList(CLRDATA_ADDRESS addr, int count, CLRDATA_ADDRESS
PTR_AppDomain pAppDomain = PTR_AppDomain(TO_TADDR(addr));
AppDomain::AssemblyIterator i = pAppDomain->IterateAssembliesEx(
(AssemblyIterationFlags)(kIncludeLoading | kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

int n = 0;
if (values)
{
while (i.Next(pDomainAssembly.This()) && (n < count))
while (i.Next(pAssembly.This()) && (n < count))
{
CollectibleAssemblyHolder<Assembly *> pAssembly = pDomainAssembly->GetAssembly();
if (pAssembly->IsLoaded())
{
// Note: DAC doesn't need to keep the assembly alive - see code:CollectibleAssemblyHolder#CAH_DAC
Expand All @@ -2988,8 +2987,8 @@ ClrDataAccess::GetAssemblyList(CLRDATA_ADDRESS addr, int count, CLRDATA_ADDRESS
}
else
{
while (i.Next(pDomainAssembly.This()))
if (pDomainAssembly->GetAssembly()->IsLoaded())
while (i.Next(pAssembly.This()))
if (pAssembly->IsLoaded())
n++;
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/daccess/task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5196,8 +5196,8 @@ EnumMethodInstances::Next(ClrDataAccess* dac,
NextMethod:
{
// Note: DAC doesn't need to keep the assembly alive - see code:CollectibleAssemblyHolder#CAH_DAC
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
if (!m_methodIter.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
if (!m_methodIter.Next(pAssembly.This()))
{
return S_FALSE;
}
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12322,12 +12322,12 @@ HRESULT Debugger::DeoptimizeMethod(Module* pModule, mdMethodDef methodDef)
}

// Now deoptimize anything that has inlined it in a R2R method
AppDomain::AssemblyIterator domainAssemblyIterator = SystemDomain::System()->DefaultDomain()->IterateAssembliesEx((AssemblyIterationFlags) (kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
AppDomain::AssemblyIterator assemblyIterator = SystemDomain::System()->DefaultDomain()->IterateAssembliesEx((AssemblyIterationFlags) (kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<Assembly *> pAssembly;
NativeImageInliningIterator inlinerIter;
while (domainAssemblyIterator.Next(pDomainAssembly.This()))
while (assemblyIterator.Next(pAssembly.This()))
{
Module *pCandidateModule = pDomainAssembly->GetModule();
Module *pCandidateModule = pAssembly->GetModule();
if (pCandidateModule->HasReadyToRunInlineTrackingMap())
{
inlinerIter.Reset(pCandidateModule, MethodInModule(pModule, methodDef));
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/ee/functioninfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2023,8 +2023,8 @@ void DebuggerMethodInfo::CreateDJIsForNativeBlobs(AppDomain * pAppDomain, Method
// This also handles the possibility of getting the same methoddesc back from the iterator.
// It also lets EnC + generics play nice together (including if an generic method was EnC-ed)
LoadedMethodDescIterator it(pAppDomain, m_module, m_token);
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
while (it.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
while (it.Next(pAssembly.This()))
{
MethodDesc * pDesc = it.Current();
if (!pDesc->HasNativeCode())
Expand Down
57 changes: 28 additions & 29 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1819,11 +1819,10 @@ BOOL AppDomain::ContainsAssembly(Assembly * assem)
WRAPPER_NO_CONTRACT;
AssemblyIterator i = IterateAssembliesEx((AssemblyIterationFlags)(
kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

while (i.Next(pDomainAssembly.This()))
while (i.Next(pAssembly.This()))
{
CollectibleAssemblyHolder<Assembly *> pAssembly = pDomainAssembly->GetAssembly();
if (pAssembly == assem)
return TRUE;
}
Expand Down Expand Up @@ -2776,15 +2775,15 @@ Assembly * AppDomain::FindAssembly(PEAssembly * pPEAssembly, FindAssemblyOptions
kIncludeLoaded |
(includeFailedToLoad ? kIncludeFailedToLoad : 0) |
kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

while (i.Next(pDomainAssembly.This()))
while (i.Next(pAssembly.This()))
{
PEAssembly * pManifestFile = pDomainAssembly->GetPEAssembly();
PEAssembly * pManifestFile = pAssembly->GetPEAssembly();
if (pManifestFile &&
pManifestFile->Equals(pPEAssembly))
{
return pDomainAssembly.GetValue()->GetAssembly();
return pAssembly;
}
}
return NULL;
Expand Down Expand Up @@ -3579,10 +3578,10 @@ BOOL AppDomain::NotifyDebuggerLoad(int flags, BOOL attaching)
// Attach to our assemblies
LOG((LF_CORDB, LL_INFO100, "AD::NDA: Iterating assemblies\n"));
i = IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeLoading | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
while (i.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
while (i.Next(pAssembly.This()))
{
result = (pDomainAssembly->GetAssembly()->NotifyDebuggerLoad(flags, attaching) ||
result = (pAssembly->NotifyDebuggerLoad(flags, attaching) ||
result);
}

Expand All @@ -3599,13 +3598,13 @@ void AppDomain::NotifyDebuggerUnload()

LOG((LF_CORDB, LL_INFO100, "AD::NDD: Interating domain bound assemblies\n"));
AssemblyIterator i = IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeLoading | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

// Detach from our assemblies
while (i.Next(pDomainAssembly.This()))
while (i.Next(pAssembly.This()))
{
LOG((LF_CORDB, LL_INFO100, "AD::NDD: Iterating assemblies\n"));
pDomainAssembly->GetAssembly()->NotifyDebuggerUnload();
pAssembly->NotifyDebuggerUnload();
}
}
#endif // DEBUGGING_SUPPORTED
Expand Down Expand Up @@ -4024,7 +4023,7 @@ void AppDomain::RemoveTypesFromTypeIDMap(LoaderAllocator* pLoaderAllocator)
//
BOOL
AppDomain::AssemblyIterator::Next(
CollectibleAssemblyHolder<DomainAssembly *> * pDomainAssemblyHolder)
CollectibleAssemblyHolder<Assembly *> * pAssemblyHolder)
{
CONTRACTL {
NOTHROW;
Expand All @@ -4033,7 +4032,7 @@ AppDomain::AssemblyIterator::Next(
} CONTRACTL_END;

CrstHolder ch(m_pAppDomain->GetAssemblyListLock());
return Next_Unlocked(pDomainAssemblyHolder);
return Next_Unlocked(pAssemblyHolder);
}

//---------------------------------------------------------------------------------------
Expand All @@ -4042,7 +4041,7 @@ AppDomain::AssemblyIterator::Next(
//
BOOL
AppDomain::AssemblyIterator::Next_Unlocked(
CollectibleAssemblyHolder<DomainAssembly *> * pDomainAssemblyHolder)
CollectibleAssemblyHolder<Assembly *> * pAssemblyHolder)
{
CONTRACTL {
NOTHROW;
Expand All @@ -4068,13 +4067,13 @@ AppDomain::AssemblyIterator::Next_Unlocked(
{
if (m_assemblyIterationFlags & kIncludeFailedToLoad)
{
*pDomainAssemblyHolder = pDomainAssembly;
*pAssemblyHolder = pAssembly;
return TRUE;
}
continue; // reject
}

// First, reject DomainAssemblies whose load status is not to be included in
// First, reject assemblies whose load status is not to be included in
// the enumeration

if (pAssembly->IsAvailableToProfilers() &&
Expand Down Expand Up @@ -4104,7 +4103,7 @@ AppDomain::AssemblyIterator::Next_Unlocked(
}
}

// Next, reject DomainAssemblies whose execution status is
// Next, reject assemblies whose execution status is
// not to be included in the enumeration

// execution assembly
Expand All @@ -4114,7 +4113,7 @@ AppDomain::AssemblyIterator::Next_Unlocked(
}

// Next, reject collectible assemblies
if (pDomainAssembly->IsCollectible())
if (pAssembly->IsCollectible())
{
if (m_assemblyIterationFlags & kExcludeCollectible)
{
Expand All @@ -4130,14 +4129,14 @@ AppDomain::AssemblyIterator::Next_Unlocked(
continue; // reject
}

if (pDomainAssembly->GetLoaderAllocator()->AddReferenceIfAlive())
if (pAssembly->GetLoaderAllocator()->AddReferenceIfAlive())
{ // The assembly is alive

// Set the holder value (incl. increasing ref-count)
*pDomainAssemblyHolder = pDomainAssembly;
*pAssemblyHolder = pAssembly;

// Now release the reference we took in the if-condition
pDomainAssembly->GetLoaderAllocator()->Release();
pAssembly->GetLoaderAllocator()->Release();
return TRUE;
}
// The assembly is not alive anymore (and we didn't increase its ref-count in the
Expand All @@ -4149,15 +4148,15 @@ AppDomain::AssemblyIterator::Next_Unlocked(
}
// Set the holder value to assembly with 0 ref-count without increasing the ref-count (won't
// call Release either)
pDomainAssemblyHolder->Assign(pDomainAssembly, FALSE);
pAssemblyHolder->Assign(pAssembly, FALSE);
return TRUE;
}

*pDomainAssemblyHolder = pDomainAssembly;
*pAssemblyHolder = pAssembly;
return TRUE;
}

*pDomainAssemblyHolder = NULL;
*pAssemblyHolder = NULL;
return FALSE;
} // AppDomain::AssemblyIterator::Next_Unlocked

Expand Down Expand Up @@ -4424,11 +4423,11 @@ AppDomain::EnumMemoryRegions(CLRDataEnumMemoryFlags flags, bool enumThis)

m_Assemblies.EnumMemoryRegions(flags);
AssemblyIterator assem = IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

while (assem.Next(pDomainAssembly.This()))
while (assem.Next(pAssembly.This()))
{
pDomainAssembly->EnumMemoryRegions(flags);
pAssembly->GetDomainAssembly()->EnumMemoryRegions(flags);
}
}

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 @@ -1002,9 +1002,9 @@ class AppDomain final
AssemblyIterationFlags m_assemblyIterationFlags;

public:
BOOL Next(CollectibleAssemblyHolder<DomainAssembly *> * pDomainAssemblyHolder);
// Note: Does not lock the assembly list, but AddRefs collectible assemblies.
BOOL Next_Unlocked(CollectibleAssemblyHolder<DomainAssembly *> * pDomainAssemblyHolder);
BOOL Next(CollectibleAssemblyHolder<Assembly *> * pAssemblyHolder);
// Note: Does not lock the assembly list, but AddRefs collectible assemblies' loader allocator.
BOOL Next_Unlocked(CollectibleAssemblyHolder<Assembly *> * pAssemblyHolder);

private:
inline DWORD GetIndex()
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/appdomainnative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ extern "C" void QCALLTYPE AssemblyNative_GetLoadedAssemblies(QCall::ObjectHandle
// loaded into this appdomain, on another thread.
AppDomain::AssemblyIterator i = pApp->IterateAssembliesEx((AssemblyIterationFlags)(
kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
CollectibleAssemblyHolder<Assembly *> pAssembly;

while (i.Next(pDomainAssembly.This()) && (numAssemblies < nArrayElems))
while (i.Next(pAssembly.This()) && (numAssemblies < nArrayElems))
{
// Do not change this code. This is done this way to
// prevent a GC hole in the SetObjectReference() call. The compiler
// is free to pick the order of evaluation.
OBJECTREF o = (OBJECTREF)pDomainAssembly->GetAssembly()->GetExposedObject();
OBJECTREF o = (OBJECTREF)pAssembly->GetExposedObject();
if (o == NULL)
{ // The assembly was collected and is not reachable from managed code anymore
continue;
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/vm/class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,10 @@ HRESULT EEClass::AddField(MethodTable* pMT, mdFieldDef fieldDef, FieldDesc** ppN
AppDomain::AssemblyIterator appIt = pDomain->IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution));

bool isStaticField = !!pNewFD->IsStatic();
CollectibleAssemblyHolder<DomainAssembly*> pDomainAssembly;
while (appIt.Next(pDomainAssembly.This()) && SUCCEEDED(hr))
CollectibleAssemblyHolder<Assembly*> pAssembly;
while (appIt.Next(pAssembly.This()) && SUCCEEDED(hr))
{
Module* pMod = pDomainAssembly->GetModule();
Module* pMod = pAssembly->GetModule();
LOG((LF_ENC, LL_INFO100, "EEClass::AddField Checking: %s mod:%p\n", pMod->GetDebugName(), pMod));

EETypeHashTable* paramTypes = pMod->GetAvailableParamTypes();
Expand Down Expand Up @@ -655,10 +655,10 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc**
PTR_AppDomain pDomain = AppDomain::GetCurrentDomain();
AppDomain::AssemblyIterator appIt = pDomain->IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution));

CollectibleAssemblyHolder<DomainAssembly*> pDomainAssembly;
while (appIt.Next(pDomainAssembly.This()) && SUCCEEDED(hr))
CollectibleAssemblyHolder<Assembly*> pAssembly;
while (appIt.Next(pAssembly.This()) && SUCCEEDED(hr))
{
Module* pMod = pDomainAssembly->GetModule();
Module* pMod = pAssembly->GetModule();
LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Checking: %s mod:%p\n", pMod->GetDebugName(), pMod));

EETypeHashTable* paramTypes = pMod->GetAvailableParamTypes();
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/codeversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2055,8 +2055,8 @@ HRESULT CodeVersionManager::EnumerateDomainClosedMethodDescs(
pModuleContainingMethodDef,
methodDef,
assemFlags);
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
while (it.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
while (it.Next(pAssembly.This()))
{
MethodDesc * pLoadedMD = it.Current();

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/encee.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod)
module,
tkMethod,
AssemblyIterationFlags(kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
while (it.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
while (it.Next(pAssembly.This()))
{
MethodDesc* pMD = it.Current();
pMD->ResetCodeEntryPointForEnC();
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/vm/eventtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5373,16 +5373,15 @@ VOID ETW::EnumerationLog::IterateAppDomain(DWORD enumerationOptions)

AppDomain::AssemblyIterator assemblyIterator = pDomain->IterateAssembliesEx(
(AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;
while (assemblyIterator.Next(pDomainAssembly.This()))
CollectibleAssemblyHolder<Assembly *> pAssembly;
while (assemblyIterator.Next(pAssembly.This()))
{
CollectibleAssemblyHolder<Assembly *> pAssembly = pDomainAssembly->GetAssembly();
if (enumerationOptions & ETW::EnumerationLog::EnumerationStructs::DomainAssemblyModuleDCStart)
{
ETW::EnumerationLog::IterateAssembly(pAssembly, enumerationOptions);
}

Module * pModule = pDomainAssembly->GetModule();
Module * pModule = pAssembly->GetModule();
ETW::EnumerationLog::IterateModule(pModule, enumerationOptions);

if((enumerationOptions & ETW::EnumerationLog::EnumerationStructs::DomainAssemblyModuleDCEnd) ||
Expand Down
Loading

0 comments on commit c0cafd7

Please sign in to comment.