MSFT:18139538 Remove the use of Guest Arena from parser code to avoid ScriptContext leak.

Author: atulkatti

lib/Common/Memory/RecyclerRootPtr.h

@@ -51,7 +51,7 @@ class AutoRecyclerRootPtr : public RecyclerRootPtr<T>
     }
     void Unroot()
     {
-        if (ptr != nullptr)
+        if (this->ptr != nullptr)
         {
             __super::Unroot(recycler);
         }

lib/Parser/Parse.cpp

@@ -82,8 +82,9 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
     m_doingFastScan(false),
 #endif
     m_nextBlockId(0),
+    m_tempGuestArena(scriptContext->GetTemporaryGuestAllocator(_u("ParserRegex")), scriptContext->GetRecycler()),
     // use the GuestArena directly for keeping the RegexPattern* alive during byte code generation
-    m_registeredRegexPatterns(scriptContext->GetGuestArena()),
+    m_registeredRegexPatterns(m_tempGuestArena->GetAllocator()),
 
     m_scriptContext(scriptContext),
     m_token(), // should initialize to 0/nullptrs
@@ -154,11 +155,10 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
 
 Parser::~Parser(void)
 {
-    if (m_scriptContext == nullptr || m_scriptContext->GetGuestArena() == nullptr)
+    m_registeredRegexPatterns.Reset();
+    if (m_scriptContext != nullptr)
     {
-        // If the scriptContext or guestArena have gone away, there is no point clearing each item of this list.
-        // Just reset it so that destructor of the SList will be no-op
-        m_registeredRegexPatterns.Reset();
+        m_scriptContext->ReleaseTemporaryGuestAllocator(m_tempGuestArena);
     }
 
 #if ENABLE_BACKGROUND_PARSING
@@ -1925,7 +1925,7 @@ void Parser::RegisterRegexPattern(UnifiedRegex::RegexPattern *const regexPattern
     Assert(regexPattern);
 
     // ensure a no-throw add behavior here, to catch out of memory exceptions, using the guest arena allocator
-    if (!m_registeredRegexPatterns.PrependNoThrow(m_scriptContext->GetGuestArena(), regexPattern))
+    if (!m_registeredRegexPatterns.PrependNoThrow(m_tempGuestArena->GetAllocator(), regexPattern))
     {
         Parser::Error(ERRnoMemory);
     }

lib/Parser/Parse.h

@@ -177,6 +177,8 @@ namespace Js
 {
     class ParseableFunctionInfo;
     class FunctionBody;
+    template <bool isGuestArena>
+    class TempArenaAllocatorWrapper;
 };
 
 class Parser
@@ -273,8 +275,9 @@ class Parser
 #endif
     int                 m_nextBlockId;
 
+    AutoRecyclerRootPtr<Js::TempArenaAllocatorWrapper<true>> m_tempGuestArena;
     // RegexPattern objects created for literal regexes are recycler-allocated and need to be kept alive until the function body
-    // is created during byte code generation. The RegexPattern pointer is stored in the script context's guest
+    // is created during byte code generation. The RegexPattern pointer is stored in a temporary guest
     // arena for that purpose. This list is then unregistered from the guest arena at the end of parsing/scanning.
     SList<UnifiedRegex::RegexPattern *, ArenaAllocator> m_registeredRegexPatterns;
 

lib/Runtime/Base/ScriptContext.cpp

@@ -88,7 +88,6 @@ namespace Js
         integerStringMapCacheMissCount(0),
         integerStringMapCacheUseCount(0),
 #endif
-        guestArena(nullptr),
 #ifdef ENABLE_SCRIPT_DEBUGGING
         diagnosticArena(nullptr),
         raiseMessageToDebuggerFunctionType(nullptr),
@@ -798,12 +797,6 @@ namespace Js
             interpreterArena = nullptr;
         }
 
-        if (this->guestArena)
-        {
-            ReleaseGuestArena();
-            guestArena = nullptr;
-        }
-
         builtInLibraryFunctions = nullptr;
 
         pActiveScriptDirect = nullptr;
@@ -1304,8 +1297,6 @@ namespace Js
 
     void ScriptContext::InitializePreGlobal()
     {
-        this->guestArena = this->GetRecycler()->CreateGuestArena(_u("Guest"), Throw::OutOfMemory);
-
 #if ENABLE_BACKGROUND_PARSING
         if (PHASE_ON1(Js::ParallelParsePhase))
         {
@@ -2680,17 +2671,6 @@ namespace Js
         }
     }
 
-
-    void ScriptContext::ReleaseGuestArena()
-    {
-        AssertMsg(this->guestArena, "No guest arena to release");
-        if (this->guestArena)
-        {
-            this->GetRecycler()->DeleteGuestArena(this->guestArena);
-            this->guestArena = nullptr;
-        }
-    }
-
     void ScriptContext::SetScriptStartEventHandler(ScriptContext::EventHandler eventHandler)
     {
         AssertMsg(this->scriptStartEventHandler == nullptr, "Do not support multi-cast yet");
@@ -4895,7 +4875,6 @@ namespace Js
     void ScriptContext::BindReference(void * addr)
     {
         Assert(!this->isClosed);
-        Assert(this->guestArena);
         Assert(recycler->IsValidObject(addr));
 #if DBG
         Assert(!bindRef.ContainsKey(addr));     // Make sure we don't bind the same pointer twice
@@ -5062,7 +5041,6 @@ namespace Js
         {
             return;
         }
-        Assert(this->guestArena);
 
         if (EnableEvalMapCleanup())
         {

lib/Runtime/Base/ScriptContext.h

@@ -578,7 +578,6 @@ namespace Js
         CacheAllocator enumeratorCacheAllocator;
 
         ArenaAllocator* interpreterArena;
-        ArenaAllocator* guestArena;
 
 #ifdef ENABLE_SCRIPT_DEBUGGING
         ArenaAllocator* diagnosticArena;
@@ -1378,13 +1377,6 @@ namespace Js
         bool EnsureInterpreterArena(ArenaAllocator **);
         void ReleaseInterpreterArena();
 
-        ArenaAllocator* GetGuestArena() const
-        {
-            return guestArena;
-        }
-
-        void ReleaseGuestArena();
-
         Recycler* GetRecycler() const { return recycler; }
         RecyclerJavascriptNumberAllocator * GetNumberAllocator() { return &numberAllocator; }
 #if ENABLE_NATIVE_CODEGEN