diff options
author | Jan Vorlicek <janvorli@microsoft.com> | 2019-01-11 10:38:37 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-11 10:38:37 +0100 |
commit | dabe2b8f49479fc3c2958c97e6b1472c2d40af0f (patch) | |
tree | cbd58a93ca0a63f56e52233f58ca2e86858c3641 /src/vm/contractimpl.cpp | |
parent | df88b1f6f858a558306e0e012c16d9de9c62ec15 (diff) | |
download | coreclr-dabe2b8f49479fc3c2958c97e6b1472c2d40af0f.tar.gz coreclr-dabe2b8f49479fc3c2958c97e6b1472c2d40af0f.tar.bz2 coreclr-dabe2b8f49479fc3c2958c97e6b1472c2d40af0f.zip |
Add cleanup of the TypeIDMap at unload time (#21943)
The TypeIDMap is stored in the AppDomain and contains two hash maps -
id to MethodTable and MethodTable to id. We were missing removing
entries for MethodTables that belong to a LoaderAllocator that's being
destroyed. Thus we were leaking some memory, but also causing potential
issue. When at some point after the LoaderAllocator destruction a
MethodTable gets the same address as one of the MethodTables that was
destroyed in the past and was also recorded in the TypeIDMap, we would
incorrectly reuse the id. That is problematic in case the old
MethodTable didn't require fat id and the new does or vice versa.
I've hit assert due to that while running System.Numerics.Vectors.Tests
inside an unloadable AssemblyLoadContext.
The implementation of the fix is very primitive. It is expected that we
will be able to get rid of the TypeIDMap in a near future and so it is
not worth optimizing.
Diffstat (limited to 'src/vm/contractimpl.cpp')
-rw-r--r-- | src/vm/contractimpl.cpp | 35 |
1 files changed, 35 insertions, 0 deletions
diff --git a/src/vm/contractimpl.cpp b/src/vm/contractimpl.cpp index c82bd1b45b..1a83712b3e 100644 --- a/src/vm/contractimpl.cpp +++ b/src/vm/contractimpl.cpp @@ -155,6 +155,41 @@ UINT32 TypeIDMap::GetTypeID(PTR_MethodTable pMT) #ifndef DACCESS_COMPILE //------------------------------------------------------------------------ +// Remove all types that belong to the passed in LoaderAllocator +void TypeIDMap::RemoveTypes(LoaderAllocator *pLoaderAllocator) +{ + CONTRACTL { + NOTHROW; + GC_NOTRIGGER; + SO_TOLERANT; + } CONTRACTL_END; + + // Take the lock + CrstHolder lh(&m_lock); + + for (HashMap::Iterator it = m_mtMap.begin(); !it.end(); ++it) + { + if (((MethodTable*)it.GetKey())->GetLoaderAllocator() == pLoaderAllocator) + { + // Note: the entry is just marked for deletion, the removal happens in Compact below + m_mtMap.DeleteValue(it.GetKey(), it.GetValue()); + } + } + + m_mtMap.Compact(); + + for (HashMap::Iterator it = m_idMap.begin(); !it.end(); ++it) + { + if (((MethodTable*)(it.GetValue() << 1))->GetLoaderAllocator() == pLoaderAllocator) + { + // Note: the entry is just marked for deletion, the removal happens in Compact below + m_idMap.DeleteValue(it.GetKey(), it.GetValue()); + } + } + + m_idMap.Compact(); +} +//------------------------------------------------------------------------ // If TRUE, it points to a matching entry. // If FALSE, it is at the insertion point. BOOL |