deps: update ChakraCore to chakra-core/ChakraCore@8cf80e378c

[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. Reviewed-By: chakrabot <[email protected]>
nodejs · Feb 13, 2018 · 42265ee · 42265ee
1 parent 673c123
commit 42265ee
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 14 deletions.
diff --git a/deps/chakrashim/core/lib/Common/Common/Jobs.cpp b/deps/chakrashim/core/lib/Common/Common/Jobs.cpp
@@ -617,7 +617,7 @@ namespace JsUtil
         threadCount(0),
         maxThreadCount(0)
 #if PDATA_ENABLED && defined(_WIN32)
-        ,hasExtraWork(false)
+        ,hasExtraWork(0)
 #endif
     {
         if (!threadService->HasCallback())
@@ -713,7 +713,7 @@ namespace JsUtil
 #if PDATA_ENABLED && defined(_WIN32)
     void BackgroundJobProcessor::DoExtraWork()
     {
-        while (hasExtraWork)
+        while (InterlockedExchangeAdd(&hasExtraWork, 0) > 0)
         {
             DelayDeletingFunctionTable::Clear();
             Sleep(50);
@@ -1431,14 +1431,15 @@ namespace JsUtil
 #if PDATA_ENABLED && defined(_WIN32)
     void BackgroundJobProcessor::StartExtraWork()
     {
-        hasExtraWork = true;
+        InterlockedIncrement(&hasExtraWork);
 
         // Signal the background thread to wake up and process the extra work.
         jobReady.Set();
     }
     void BackgroundJobProcessor::EndExtraWork()
     {
-        hasExtraWork = false;
+        LONG newValue = InterlockedDecrement(&hasExtraWork);
+        Assert(newValue >= 0);
     }
 #endif
 

diff --git a/deps/chakrashim/core/lib/Common/Common/Jobs.h b/deps/chakrashim/core/lib/Common/Common/Jobs.h
@@ -463,7 +463,7 @@ namespace JsUtil
         ParallelThreadData **parallelThreadData;
 
 #if PDATA_ENABLED && defined(_WIN32)
-        bool hasExtraWork;
+        LONG hasExtraWork;
 #endif
 
 #if DBG_DUMP

diff --git a/deps/chakrashim/core/lib/Common/Core/DelayLoadLibrary.cpp b/deps/chakrashim/core/lib/Common/Core/DelayLoadLibrary.cpp
@@ -82,6 +82,8 @@ NtdllLibrary::NTSTATUS NtdllLibrary::AddGrowableFunctionTable( _Out_ PVOID * Dyn
                 return 1;
             }
         }
+
+        *DynamicTable = nullptr;
         NTSTATUS status = addGrowableFunctionTable(DynamicTable,
             FunctionTable,
             EntryCount,
@@ -92,7 +94,7 @@ NtdllLibrary::NTSTATUS NtdllLibrary::AddGrowableFunctionTable( _Out_ PVOID * Dyn
         PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("Register: [%d] Begin: %llx, End: %x, Unwind: %llx, RangeBase: %llx, RangeEnd: %llx, table: %llx, Status: %x\n"),
            GetCurrentThreadId(), FunctionTable->BeginAddress, FunctionTable->EndAddress, FunctionTable->UnwindInfoAddress, RangeBase, RangeEnd, *DynamicTable, status);
 #endif
-        Assert((status >= 0 && FunctionTable != nullptr) || status == 0xC000009A /*STATUS_INSUFFICIENT_RESOURCES*/);
+        Assert((status >= 0 && *DynamicTable != nullptr) || status == 0xC000009A /*STATUS_INSUFFICIENT_RESOURCES*/);
         return status;
     }
     return 1;

diff --git a/deps/chakrashim/core/lib/Common/Memory/DelayDeletingFunctionTable.cpp b/deps/chakrashim/core/lib/Common/Memory/DelayDeletingFunctionTable.cpp
@@ -9,6 +9,7 @@
 #include "Core/DelayLoadLibrary.h"
 #include <malloc.h>
 
+CriticalSection DelayDeletingFunctionTable::cs;
 PSLIST_HEADER DelayDeletingFunctionTable::Head = nullptr;
 DelayDeletingFunctionTable DelayDeletingFunctionTable::Instance;
 
@@ -52,6 +53,10 @@ void DelayDeletingFunctionTable::Clear()
 {
     if (Head)
     {
+        // add lock here to prevent in case one thread popped the entry and before deleting
+        // another thread is registering function table for the same address
+        AutoCriticalSection autoCS(&cs); 
+
         SLIST_ENTRY* entry = InterlockedPopEntrySList(Head);
         while (entry)
         {

diff --git a/deps/chakrashim/core/lib/Common/Memory/XDataAllocator.h b/deps/chakrashim/core/lib/Common/Memory/XDataAllocator.h
@@ -23,6 +23,7 @@ struct DelayDeletingFunctionTable
 {
     static PSLIST_HEADER Head;
     static DelayDeletingFunctionTable Instance;
+    static CriticalSection cs;
 
     DelayDeletingFunctionTable();
     ~DelayDeletingFunctionTable();

diff --git a/deps/chakrashim/core/lib/Runtime/Base/FunctionBody.cpp b/deps/chakrashim/core/lib/Runtime/Base/FunctionBody.cpp
@@ -9179,14 +9179,6 @@ namespace Js
                 {
                     scriptContext->FreeFunctionEntryPoint((Js::JavascriptMethod)this->GetNativeAddress(), this->GetThunkAddress());
                 }
-#if PDATA_ENABLED && defined(_WIN32)
-                else
-                {
-                    // in case of debugger attaching, we have a new code generator and when deleting old code generator,
-                    // the xData is not put in the delay list yet. clear the list now so the code addresses are ready to reuse
-                    DelayDeletingFunctionTable::Clear();
-                }
-#endif
             }
 
 #ifdef PERF_COUNTERS

diff --git a/deps/chakrashim/core/lib/Runtime/Base/ScriptContext.cpp b/deps/chakrashim/core/lib/Runtime/Base/ScriptContext.cpp
@@ -3098,6 +3098,30 @@ namespace Js
     {
         OUTPUT_TRACE(Js::DebuggerPhase, _u("ScriptContext::OnDebuggerAttached: start 0x%p\n"), this);
 
+#if ENABLE_NATIVE_CODEGEN
+#if PDATA_ENABLED && defined(_WIN32)
+        // in case of debugger attaching, we have a new code generator and when deleting old code generator,
+        // the xData is not put in the delay list yet. clear the list now so the code addresses are ready to reuse
+        struct AutoReset
+        {
+            AutoReset(ThreadContext* threadContext)
+                :threadContext(threadContext)
+            {
+                // indicate background thread that we need help to delete the xData
+                threadContext->GetJobProcessor()->StartExtraWork();
+            }
+            ~AutoReset()
+            {
+                threadContext->GetJobProcessor()->EndExtraWork();
+                // in case the background thread didn't clear all the function tables
+                DelayDeletingFunctionTable::Clear();
+            }
+
+            ThreadContext* threadContext;
+        } autoReset(this->GetThreadContext());
+#endif
+#endif
+
         Js::StepController* stepController = &this->GetThreadContext()->GetDebugManager()->stepController;
         if (stepController->IsActive())
         {
@@ -3205,6 +3229,30 @@ namespace Js
     {
         OUTPUT_TRACE(Js::DebuggerPhase, _u("ScriptContext::OnDebuggerDetached: start 0x%p\n"), this);
 
+#if ENABLE_NATIVE_CODEGEN
+#if PDATA_ENABLED && defined(_WIN32)
+        // in case of debugger detaching, we have a new code generator and when deleting old code generator,
+        // the xData is not put in the delay list yet. clear the list now so the code addresses are ready to reuse
+        struct AutoReset
+        {
+            AutoReset(ThreadContext* threadContext)
+                :threadContext(threadContext)
+            {
+                // indicate background thread that we need help to delete the xData
+                threadContext->GetJobProcessor()->StartExtraWork();
+            }
+            ~AutoReset()
+            {
+                threadContext->GetJobProcessor()->EndExtraWork();
+                // in case the background thread didn't clear all the function tables
+                DelayDeletingFunctionTable::Clear();
+            }
+
+            ThreadContext* threadContext;
+        } autoReset(this->GetThreadContext());
+#endif
+#endif
+
         Js::StepController* stepController = &this->GetThreadContext()->GetDebugManager()->stepController;
         if (stepController->IsActive())
         {