Skip to content

Commit 336bed1

Browse files
author
Atul Katti
committed
MSFT:18171347 Parser's Temporary Guest Arena leaks in case of modules with Regex patterns.
1 parent fbec449 commit 336bed1

File tree

4 files changed

+39
-7
lines changed

4 files changed

+39
-7
lines changed

lib/Parser/Parse.cpp

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
8282
m_doingFastScan(false),
8383
#endif
8484
m_nextBlockId(0),
85+
m_tempGuestArenaReleased(false),
8586
m_tempGuestArena(scriptContext->GetTemporaryGuestAllocator(_u("ParserRegex")), scriptContext->GetRecycler()),
8687
// use the GuestArena directly for keeping the RegexPattern* alive during byte code generation
8788
m_registeredRegexPatterns(m_tempGuestArena->GetAllocator()),
@@ -155,11 +156,7 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
155156

156157
Parser::~Parser(void)
157158
{
158-
m_registeredRegexPatterns.Reset();
159-
if (m_scriptContext != nullptr)
160-
{
161-
m_scriptContext->ReleaseTemporaryGuestAllocator(m_tempGuestArena);
162-
}
159+
this->ReleaseTemporaryGuestArena();
163160

164161
#if ENABLE_BACKGROUND_PARSING
165162
if (this->m_hasParallelJob)
@@ -13909,6 +13906,23 @@ void Parser::ProcessCapturedNames(ParseNodeFnc* pnodeFnc)
1390913906
}
1391013907
}
1391113908

13909+
void Parser::ReleaseTemporaryGuestArena()
13910+
{
13911+
// In case of modules the Parser lives longer than the temporary Guest Arena. We may have already released the arena explicitly.
13912+
if (!m_tempGuestArenaReleased)
13913+
{
13914+
// The regex patterns list has references to the temporary Guest Arena. Reset it first.
13915+
m_registeredRegexPatterns.Reset();
13916+
13917+
if (this->m_scriptContext != nullptr)
13918+
{
13919+
this->m_scriptContext->ReleaseTemporaryGuestAllocator(m_tempGuestArena);
13920+
}
13921+
13922+
m_tempGuestArenaReleased = true;
13923+
}
13924+
}
13925+
1391213926
bool Parser::IsCreatingStateCache()
1391313927
{
1391413928
return (((this->m_grfscr & fscrCreateParserState) == fscrCreateParserState)

lib/Parser/Parse.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ class Parser
194194
~Parser(void);
195195

196196
Js::ScriptContext* GetScriptContext() const { return m_scriptContext; }
197-
197+
void ReleaseTemporaryGuestArena();
198198
bool IsCreatingStateCache();
199199

200200
#if ENABLE_BACKGROUND_PARSING
@@ -273,6 +273,7 @@ class Parser
273273
bool m_isInBackground;
274274
bool m_doingFastScan;
275275
#endif
276+
bool m_tempGuestArenaReleased;
276277
int m_nextBlockId;
277278

278279
AutoRecyclerRootPtr<Js::TempArenaAllocatorWrapper<true>> m_tempGuestArena;
@@ -1153,5 +1154,4 @@ class Parser
11531154
public:
11541155
charcount_t GetSourceIchLim() { return m_sourceLim; }
11551156
static BOOL NodeEqualsName(ParseNodePtr pnode, LPCOLESTR sz, uint32 cch);
1156-
11571157
};

lib/Runtime/Language/SourceTextModuleRecord.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,11 @@ namespace Js
915915
childModuleRecord->GenerateRootFunction();
916916
});
917917
}
918+
919+
if (this->parser != nullptr)
920+
{
921+
this->parser->ReleaseTemporaryGuestArena();
922+
}
918923
}
919924

920925
Var SourceTextModuleRecord::ModuleEvaluation()

test/es6module/module-functionality.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,19 @@ var tests = [
350350
testRunner.LoadModule(functionBody, 'samethread');
351351
}
352352
},
353+
{
354+
name: "OS18171347 - Module's parser leaks temporary Guest Arena allocator when module has a regex pattern.",
355+
body: function() {
356+
testRunner.LoadModule(` /x/ ;`, 'samethread');
357+
358+
try
359+
{
360+
// syntax error
361+
testRunner.LoadModule(` /x/ ; for(i=0);`, 'samethread', {shouldFail:true});
362+
}
363+
catch(e){}
364+
}
365+
},
353366
];
354367

355368
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

0 commit comments

Comments
 (0)