Prevent duplicate class addition when a profiler adds a type (#24737)
authorDavid Mason <davmason@microsoft.com>
Thu, 23 May 2019 07:28:09 +0000 (00:28 -0700)
committerGitHub <noreply@github.com>
Thu, 23 May 2019 07:28:09 +0000 (00:28 -0700)
src/vm/ceeload.cpp
src/vm/clsload.cpp
src/vm/clsload.hpp

index 3d26a0a..7f382b1 100644 (file)
@@ -221,22 +221,35 @@ void Module::UpdateNewlyAddedTypes()
     DWORD countExportedTypesAfterProfilerUpdate = GetMDImport()->GetCountWithTokenKind(mdtExportedType);
     DWORD countCustomAttributeCount = GetMDImport()->GetCountWithTokenKind(mdtCustomAttribute);
 
-    // typeDefs rids 0 and 1 aren't included in the count, thus X typeDefs before means rid X+1 was valid and our incremental addition should start at X+2
-    for (DWORD typeDefRid = m_dwTypeCount + 2; typeDefRid < countTypesAfterProfilerUpdate + 2; typeDefRid++)
+    // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can
+    // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the
+    // in memory hashtable as usual.
+    if (!IsResource() && GetAvailableClassHash() == NULL)
     {
-        GetAssembly()->AddType(this, TokenFromRid(typeDefRid, mdtTypeDef));
+        // This call will populate the hash tables with anything that is in metadata already.
+        GetClassLoader()->LazyPopulateCaseSensitiveHashTablesDontHaveLock();
     }
-
-    // exportedType rid 0 isn't included in the count, thus X exportedTypes before means rid X was valid and our incremental addition should start at X+1
-    for (DWORD exportedTypeDef = m_dwExportedTypeCount + 1; exportedTypeDef < countExportedTypesAfterProfilerUpdate + 1; exportedTypeDef++)
+    else
     {
-        GetAssembly()->AddExportedType(TokenFromRid(exportedTypeDef, mdtExportedType));
-    }
+        // If the hash tables already exist (either R2R and we've previously populated the ) we need to manually add the types.
 
-    if ((countCustomAttributeCount != m_dwCustomAttributeCount) && IsReadyToRun())
-    {
-        // Set of custom attributes has changed. Disable the cuckoo filter from ready to run, and do normal custom attribute parsing
-        GetReadyToRunInfo()->DisableCustomAttributeFilter();
+        // typeDefs rids 0 and 1 aren't included in the count, thus X typeDefs before means rid X+1 was valid and our incremental addition should start at X+2
+        for (DWORD typeDefRid = m_dwTypeCount + 2; typeDefRid < countTypesAfterProfilerUpdate + 2; typeDefRid++)
+        {
+            GetAssembly()->AddType(this, TokenFromRid(typeDefRid, mdtTypeDef));
+        }
+
+        // exportedType rid 0 isn't included in the count, thus X exportedTypes before means rid X was valid and our incremental addition should start at X+1
+        for (DWORD exportedTypeDef = m_dwExportedTypeCount + 1; exportedTypeDef < countExportedTypesAfterProfilerUpdate + 1; exportedTypeDef++)
+        {
+            GetAssembly()->AddExportedType(TokenFromRid(exportedTypeDef, mdtExportedType));
+        }
+
+        if ((countCustomAttributeCount != m_dwCustomAttributeCount) && IsReadyToRun())
+        {
+            // Set of custom attributes has changed. Disable the cuckoo filter from ready to run, and do normal custom attribute parsing
+            GetReadyToRunInfo()->DisableCustomAttributeFilter();
+        }
     }
 
     m_dwTypeCount = countTypesAfterProfilerUpdate;
index 479e124..ff4d0a0 100644 (file)
@@ -1016,6 +1016,23 @@ VOID ClassLoader::PopulateAvailableClassHashTable(Module* pModule,
 }
 
 
+void ClassLoader::LazyPopulateCaseSensitiveHashTablesDontHaveLock()
+{
+    CONTRACTL
+    {
+        INSTANCE_CHECK;
+        THROWS;
+        GC_TRIGGERS;
+        MODE_ANY;
+        INJECT_FAULT(COMPlusThrowOM());
+    }
+    CONTRACTL_END;
+
+
+    CrstHolder ch(&m_AvailableClassLock);
+    LazyPopulateCaseSensitiveHashTables();
+}
+
 void ClassLoader::LazyPopulateCaseSensitiveHashTables()
 {
     CONTRACTL
@@ -1036,7 +1053,7 @@ void ClassLoader::LazyPopulateCaseSensitiveHashTables()
     {
         Module *pModule = i.GetModule();
         PREFIX_ASSUME(pModule != NULL);
-        if (pModule->IsResource())
+        if (pModule->IsResource() || pModule->GetAvailableClassHash() != NULL)
             continue;
 
         // Lazy construction of the case-sensitive hashtable of types is *only* a scenario for ReadyToRun images
@@ -4089,14 +4106,6 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule,
 
     CrstHolder ch(&m_AvailableClassLock);
 
-    // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can
-    // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the
-    // in memory hashtable as usual.
-    if (!pModule->IsResource() && pModule->GetAvailableClassHash() == NULL)
-    {
-        LazyPopulateCaseSensitiveHashTables();
-    }
-
     AddAvailableClassHaveLock(
         pModule, 
         classdef, 
@@ -4293,14 +4302,6 @@ VOID ClassLoader::AddExportedTypeDontHaveLock(Module *pManifestModule,
     CONTRACTL_END
 
     CrstHolder ch(&m_AvailableClassLock);
-        
-    // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can
-    // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the
-    // in memory hashtable as usual.
-    if (!pManifestModule->IsResource() && pManifestModule->GetAvailableClassHash() == NULL)
-    {
-        LazyPopulateCaseSensitiveHashTables();
-    }
 
     AddExportedTypeHaveLock(
         pManifestModule,
index 85ee72c..920628b 100644 (file)
@@ -581,6 +581,7 @@ private:
     VOID PopulateAvailableClassHashTable(Module *pModule,
                                          AllocMemTracker *pamTracker);
 
+    void LazyPopulateCaseSensitiveHashTablesDontHaveLock();
     void LazyPopulateCaseSensitiveHashTables();
     void LazyPopulateCaseInsensitiveHashTables();