Revert change in AddInternedString (#76496)
authorEgor Bogatov <egorbo@gmail.com>
Sun, 2 Oct 2022 04:07:48 +0000 (06:07 +0200)
committerGitHub <noreply@github.com>
Sun, 2 Oct 2022 04:07:48 +0000 (21:07 -0700)
* Restore all logic for AddInternedString

Fixes #76494

src/coreclr/vm/dynamicmethod.cpp
src/coreclr/vm/stringliteralmap.cpp
src/coreclr/vm/stringliteralmap.h

index 98b7020..5a140d6 100644 (file)
@@ -1275,8 +1275,7 @@ STRINGREF* LCGMethodResolver::GetOrInternString(STRINGREF *pProtectedStringRef)
     // lock the global string literal interning map
     CrstHolder gch(pStringLiteralMap->GetHashTableCrstGlobal());
 
-    StringLiteralEntryHolder pEntry(pStringLiteralMap->GetInternedString(pProtectedStringRef, dwHash,
-        /* bAddIfNotFound */ TRUE, /* bPreferFrozenObjectHeap */ FALSE));
+    StringLiteralEntryHolder pEntry(pStringLiteralMap->GetInternedString(pProtectedStringRef, dwHash, /* bAddIfNotFound */ TRUE));
 
     DynamicStringLiteral* pStringLiteral = (DynamicStringLiteral*)m_jitTempData.New(sizeof(DynamicStringLiteral));
     pStringLiteral->m_pEntry = pEntry.Extract();
index 4cca97a..6425c14 100644 (file)
@@ -251,9 +251,7 @@ STRINGREF *StringLiteralMap::GetInternedString(STRINGREF *pString, BOOL bAddIfNo
 
         // Retrieve the string literal from the global string literal map.
 
-        // Don't use FOH for collectible modules to avoid potential memory leaks
-        const bool preferFrozenObjectHeap = !bIsCollectible;
-        StringLiteralEntryHolder pEntry(SystemDomain::GetGlobalStringLiteralMap()->GetInternedString(pString, dwHash, bAddIfNotFound, preferFrozenObjectHeap));
+        StringLiteralEntryHolder pEntry(SystemDomain::GetGlobalStringLiteralMap()->GetInternedString(pString, dwHash, bAddIfNotFound));
 
         _ASSERTE(pEntry || !bAddIfNotFound);
 
@@ -402,7 +400,7 @@ StringLiteralEntry *GlobalStringLiteralMap::GetStringLiteral(EEStringData *pStri
     return pEntry;
 }
 
-StringLiteralEntry *GlobalStringLiteralMap::GetInternedString(STRINGREF *pString, DWORD dwHash, BOOL bAddIfNotFound, BOOL bPreferFrozenObjectHeap)
+StringLiteralEntry *GlobalStringLiteralMap::GetInternedString(STRINGREF *pString, DWORD dwHash, BOOL bAddIfNotFound)
 {
     CONTRACTL
     {
@@ -430,7 +428,7 @@ StringLiteralEntry *GlobalStringLiteralMap::GetInternedString(STRINGREF *pString
     else
     {
         if (bAddIfNotFound)
-            pEntry = AddInternedString(pString, bPreferFrozenObjectHeap);
+            pEntry = AddInternedString(pString);
     }
 
     return pEntry;
@@ -534,7 +532,7 @@ StringLiteralEntry *GlobalStringLiteralMap::AddStringLiteral(EEStringData *pStri
     return pRet;
 }
 
-StringLiteralEntry *GlobalStringLiteralMap::AddInternedString(STRINGREF *pString, bool preferFrozenObjHeap)
+StringLiteralEntry *GlobalStringLiteralMap::AddInternedString(STRINGREF *pString)
 {
 
     CONTRACTL
@@ -547,8 +545,29 @@ StringLiteralEntry *GlobalStringLiteralMap::AddInternedString(STRINGREF *pString
     }
     CONTRACTL_END;
 
-    EEStringData StringData = EEStringData((*pString)->GetStringLength(), (*pString)->GetBuffer());
-    return AddStringLiteral(&StringData, preferFrozenObjHeap);
+    StringLiteralEntry* pRet;
+
+    {
+        // All frozen strings are expected to be registered in m_StringToEntryHashTable, we might relax this assert
+        // in future if we start allocating frozen strings for non-literals
+        _ASSERT(!GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(STRINGREFToObject(*pString)));
+
+        PinnedHeapHandleBlockHolder pStrObj(&m_PinnedHeapHandleTable, 1);
+        SetObjectReference(pStrObj[0], (OBJECTREF)*pString);
+
+        // Since the allocation might have caused a GC we need to get the string data after it
+        EEStringData StringData = EEStringData((*pString)->GetStringLength(), (*pString)->GetBuffer());
+
+        StringLiteralEntryHolder pEntry(StringLiteralEntry::AllocateEntry(&StringData, (STRINGREF*)pStrObj[0]));
+        pStrObj.SuppressRelease();
+
+        // Insert the handle to the string into the hash table.
+        m_StringToEntryHashTable->InsertValue(&StringData, (LPVOID)pEntry, FALSE);
+        pEntry.SuppressRelease();
+        pRet = pEntry;
+    }
+
+    return pRet;
 }
 
 void GlobalStringLiteralMap::RemoveStringLiteralEntry(StringLiteralEntry *pEntry)
index 510b66b..9a91a0a 100644 (file)
@@ -74,7 +74,7 @@ public:
     StringLiteralEntry *GetStringLiteral(EEStringData *pStringData, DWORD dwHash, BOOL bAddIfNotFound, BOOL bPreferFrozenObjectHeap);
 
     // Method to explicitly intern a string object. Takes a precomputed hash (for perf).
-    StringLiteralEntry *GetInternedString(STRINGREF *pString, DWORD dwHash, BOOL bAddIfNotFound, BOOL bPreferFrozenObjectHeap);
+    StringLiteralEntry *GetInternedString(STRINGREF *pString, DWORD dwHash, BOOL bAddIfNotFound);
 
     // Method to calculate the hash
     DWORD GetHash(EEStringData* pData)
@@ -95,7 +95,7 @@ private:
     StringLiteralEntry *AddStringLiteral(EEStringData *pStringData, bool preferFrozenObjHeap);
 
     // Helper method to add an interned string.
-    StringLiteralEntry *AddInternedString(STRINGREF *pString, bool preferFrozenObjHeap);
+    StringLiteralEntry *AddInternedString(STRINGREF *pString);
 
     // Called by StringLiteralEntry when its RefCount falls to 0.
     void RemoveStringLiteralEntry(StringLiteralEntry *pEntry);