Skip to content

Commit 8cf80e3

Browse files
leirocksleirocks
committed
[1.8>1.9] [MERGE #4666 @leirocks] fix missing function table clearing for loop body cleanup
Merge pull request #4666 from leirocks:xdata1 clear the function table for loop body entrypoint case. when debugger attaching the code address is not freed through normal path hence clearing the xdata in main thread. this is done for function entrypoint in last change but missed loop body case. also make sure of the integrity of clearing function table in the slist.
2 parents e7945af + 250a66a commit 8cf80e3

File tree

7 files changed

+63
-14
lines changed

7 files changed

+63
-14
lines changed

‎lib/Common/Common/Jobs.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ namespace JsUtil
617617
threadCount(0),
618618
maxThreadCount(0)
619619
#if PDATA_ENABLED && defined(_WIN32)
620-
,hasExtraWork(false)
620+
,hasExtraWork(0)
621621
#endif
622622
{
623623
if (!threadService->HasCallback())
@@ -713,7 +713,7 @@ namespace JsUtil
713713
#if PDATA_ENABLED && defined(_WIN32)
714714
void BackgroundJobProcessor::DoExtraWork()
715715
{
716-
while (hasExtraWork)
716+
while (InterlockedExchangeAdd(&hasExtraWork, 0) > 0)
717717
{
718718
DelayDeletingFunctionTable::Clear();
719719
Sleep(50);
@@ -1431,14 +1431,15 @@ namespace JsUtil
14311431
#if PDATA_ENABLED && defined(_WIN32)
14321432
void BackgroundJobProcessor::StartExtraWork()
14331433
{
1434-
hasExtraWork = true;
1434+
InterlockedIncrement(&hasExtraWork);
14351435

14361436
// Signal the background thread to wake up and process the extra work.
14371437
jobReady.Set();
14381438
}
14391439
void BackgroundJobProcessor::EndExtraWork()
14401440
{
1441-
hasExtraWork = false;
1441+
LONG newValue = InterlockedDecrement(&hasExtraWork);
1442+
Assert(newValue >= 0);
14421443
}
14431444
#endif
14441445

‎lib/Common/Common/Jobs.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ namespace JsUtil
463463
ParallelThreadData **parallelThreadData;
464464

465465
#if PDATA_ENABLED && defined(_WIN32)
466-
bool hasExtraWork;
466+
LONG hasExtraWork;
467467
#endif
468468

469469
#if DBG_DUMP

‎lib/Common/Core/DelayLoadLibrary.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ NtdllLibrary::NTSTATUS NtdllLibrary::AddGrowableFunctionTable( _Out_ PVOID * Dyn
8282
return 1;
8383
}
8484
}
85+
86+
*DynamicTable = nullptr;
8587
NTSTATUS status = addGrowableFunctionTable(DynamicTable,
8688
FunctionTable,
8789
EntryCount,
@@ -92,7 +94,7 @@ NtdllLibrary::NTSTATUS NtdllLibrary::AddGrowableFunctionTable( _Out_ PVOID * Dyn
9294
PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("Register: [%d] Begin: %llx, End: %x, Unwind: %llx, RangeBase: %llx, RangeEnd: %llx, table: %llx, Status: %x\n"),
9395
GetCurrentThreadId(), FunctionTable->BeginAddress, FunctionTable->EndAddress, FunctionTable->UnwindInfoAddress, RangeBase, RangeEnd, *DynamicTable, status);
9496
#endif
95-
Assert((status >= 0 && FunctionTable != nullptr) || status == 0xC000009A /*STATUS_INSUFFICIENT_RESOURCES*/);
97+
Assert((status >= 0 && *DynamicTable != nullptr) || status == 0xC000009A /*STATUS_INSUFFICIENT_RESOURCES*/);
9698
return status;
9799
}
98100
return 1;

‎lib/Common/Memory/DelayDeletingFunctionTable.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "Core/DelayLoadLibrary.h"
1010
#include <malloc.h>
1111

12+
CriticalSection DelayDeletingFunctionTable::cs;
1213
PSLIST_HEADER DelayDeletingFunctionTable::Head = nullptr;
1314
DelayDeletingFunctionTable DelayDeletingFunctionTable::Instance;
1415

@@ -52,6 +53,10 @@ void DelayDeletingFunctionTable::Clear()
5253
{
5354
if (Head)
5455
{
56+
// add lock here to prevent in case one thread popped the entry and before deleting
57+
// another thread is registering function table for the same address
58+
AutoCriticalSection autoCS(&cs);
59+
5560
SLIST_ENTRY* entry = InterlockedPopEntrySList(Head);
5661
while (entry)
5762
{

‎lib/Common/Memory/XDataAllocator.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ struct DelayDeletingFunctionTable
2323
{
2424
static PSLIST_HEADER Head;
2525
static DelayDeletingFunctionTable Instance;
26+
static CriticalSection cs;
2627

2728
DelayDeletingFunctionTable();
2829
~DelayDeletingFunctionTable();

‎lib/Runtime/Base/FunctionBody.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9179,14 +9179,6 @@ namespace Js
91799179
{
91809180
scriptContext->FreeFunctionEntryPoint((Js::JavascriptMethod)this->GetNativeAddress(), this->GetThunkAddress());
91819181
}
9182-
#if PDATA_ENABLED && defined(_WIN32)
9183-
else
9184-
{
9185-
// in case of debugger attaching, we have a new code generator and when deleting old code generator,
9186-
// the xData is not put in the delay list yet. clear the list now so the code addresses are ready to reuse
9187-
DelayDeletingFunctionTable::Clear();
9188-
}
9189-
#endif
91909182
}
91919183

91929184
#ifdef PERF_COUNTERS

‎lib/Runtime/Base/ScriptContext.cpp‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3098,6 +3098,30 @@ namespace Js
30983098
{
30993099
OUTPUT_TRACE(Js::DebuggerPhase, _u("ScriptContext::OnDebuggerAttached: start 0x%p\n"), this);
31003100

3101+
#if ENABLE_NATIVE_CODEGEN
3102+
#if PDATA_ENABLED && defined(_WIN32)
3103+
// in case of debugger attaching, we have a new code generator and when deleting old code generator,
3104+
// the xData is not put in the delay list yet. clear the list now so the code addresses are ready to reuse
3105+
struct AutoReset
3106+
{
3107+
AutoReset(ThreadContext* threadContext)
3108+
:threadContext(threadContext)
3109+
{
3110+
// indicate background thread that we need help to delete the xData
3111+
threadContext->GetJobProcessor()->StartExtraWork();
3112+
}
3113+
~AutoReset()
3114+
{
3115+
threadContext->GetJobProcessor()->EndExtraWork();
3116+
// in case the background thread didn't clear all the function tables
3117+
DelayDeletingFunctionTable::Clear();
3118+
}
3119+
3120+
ThreadContext* threadContext;
3121+
} autoReset(this->GetThreadContext());
3122+
#endif
3123+
#endif
3124+
31013125
Js::StepController* stepController = &this->GetThreadContext()->GetDebugManager()->stepController;
31023126
if (stepController->IsActive())
31033127
{
@@ -3205,6 +3229,30 @@ namespace Js
32053229
{
32063230
OUTPUT_TRACE(Js::DebuggerPhase, _u("ScriptContext::OnDebuggerDetached: start 0x%p\n"), this);
32073231

3232+
#if ENABLE_NATIVE_CODEGEN
3233+
#if PDATA_ENABLED && defined(_WIN32)
3234+
// in case of debugger detaching, we have a new code generator and when deleting old code generator,
3235+
// the xData is not put in the delay list yet. clear the list now so the code addresses are ready to reuse
3236+
struct AutoReset
3237+
{
3238+
AutoReset(ThreadContext* threadContext)
3239+
:threadContext(threadContext)
3240+
{
3241+
// indicate background thread that we need help to delete the xData
3242+
threadContext->GetJobProcessor()->StartExtraWork();
3243+
}
3244+
~AutoReset()
3245+
{
3246+
threadContext->GetJobProcessor()->EndExtraWork();
3247+
// in case the background thread didn't clear all the function tables
3248+
DelayDeletingFunctionTable::Clear();
3249+
}
3250+
3251+
ThreadContext* threadContext;
3252+
} autoReset(this->GetThreadContext());
3253+
#endif
3254+
#endif
3255+
32083256
Js::StepController* stepController = &this->GetThreadContext()->GetDebugManager()->stepController;
32093257
if (stepController->IsActive())
32103258
{

0 commit comments

Comments
 (0)