record ServerScriptContext closing stack

fail fast for failure in ChakraMemSet/ChakraMemCpy call
remove unnecessary ChakraMemSet and ChakraMemCpy call which should be in recycler code path

lib/Backend/ServerScriptContext.cpp

@@ -29,18 +29,6 @@ ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData, Ser
 
 ServerScriptContext::~ServerScriptContext()
 {
-    HeapDelete(m_domFastPathHelperMap);
-    m_moduleRecords.Map([](uint, Js::ServerSourceTextModuleRecord* record)
-    {
-        HeapDelete(record);
-    });
-
-#ifdef PROFILE_EXEC
-    if (m_codeGenProfiler)
-    {
-        HeapDelete(m_codeGenProfiler);
-    }
-#endif
 }
 
 intptr_t
@@ -286,6 +274,9 @@ ServerScriptContext::Close()
 {
     Assert(!IsClosed());
     m_isClosed = true;
+#ifdef STACK_BACK_TRACE
+    closingStack = StackBackTrace::Capture(&NoThrowHeapAllocator::Instance);
+#endif
 }
 
 void
@@ -300,7 +291,23 @@ ServerScriptContext::Release()
     InterlockedExchangeSubtract(&m_refCount, 1u);
     if (m_isClosed && m_refCount == 0)
     {
+        HeapDelete(m_domFastPathHelperMap);
+        m_moduleRecords.Map([](uint, Js::ServerSourceTextModuleRecord* record)
+        {
+            HeapDelete(record);
+        });
+
+#ifdef PROFILE_EXEC
+        if (m_codeGenProfiler)
+        {
+            HeapDelete(m_codeGenProfiler);
+        }
+#endif
+
+        // OOP JIT TODO: fix leak in chk build after the issue that script context closed prematurely is identified
+#ifndef STACK_BACK_TRACE
         HeapDelete(this);
+#endif
     }
 }
 

lib/Backend/ServerScriptContext.h

@@ -83,4 +83,7 @@ private:
 
     bool m_isPRNGSeeded;
     bool m_isClosed;
+#ifdef STACK_BACK_TRACE
+    StackBackTrace* closingStack;
+#endif
 };

lib/Common/Exceptions/Throw.cpp

@@ -71,6 +71,10 @@ namespace Js {
         int scenario = 2;
         ReportFatalException(NULL, E_FAIL, Fatal_Internal_Error, scenario);
     }
+    void Throw::FatalInternalErrorEx(int scenario)
+    {
+        ReportFatalException(NULL, E_FAIL, Fatal_Internal_Error, scenario);
+    }
 
     void Throw::FatalProjectionError()
     {

lib/Common/Exceptions/Throw.h

@@ -20,6 +20,7 @@ namespace Js {
         static void __declspec(noreturn) NotImplemented();
         static void __declspec(noreturn) InternalError();
         static void __declspec(noreturn) FatalInternalError();
+        static void __declspec(noreturn) FatalInternalErrorEx(int scenario);
         static void __declspec(noreturn) FatalProjectionError();
 
         static void CheckAndThrowOutOfMemory(BOOLEAN status);

lib/Common/Memory/CustomHeap.h

@@ -478,7 +478,7 @@ private:
     {
         if (allocation->IsLargeAllocation())
         {
-            BOOL result = this->ProtectAllocation(allocation, readWriteFlags, PAGE_EXECUTE);
+            this->ProtectAllocation(allocation, readWriteFlags, PAGE_EXECUTE);
             return PAGE_EXECUTE;
         }
         else

lib/Common/Memory/MemUtils.cpp

@@ -24,7 +24,7 @@ Memory::ChakraMemSet(_In_ void *dst, int val, size_t sizeInBytes, HANDLE process
     {
         if (!WriteProcessMemory(processHandle, dst, writeBuffer, sizeInBytes, NULL))
         {
-            MemoryOperationLastError::RecordLastErrorAndThrow();
+            MemoryOperationLastError::CheckProcessAndThrowFatalError(processHandle);
         }
         HeapDeleteArray(sizeInBytes, writeBuffer);
     }
@@ -45,7 +45,7 @@ Memory::ChakraMemCopy(_In_ void *dst, size_t sizeInBytes, _In_reads_bytes_(count
     }
     else if (!WriteProcessMemory(processHandle, dst, src, count, NULL))
     {
-        MemoryOperationLastError::RecordLastErrorAndThrow();
+        MemoryOperationLastError::CheckProcessAndThrowFatalError(processHandle);
     }
 
 }

lib/Common/Memory/PageAllocator.cpp

@@ -970,7 +970,8 @@ PageAllocatorBase<T>::FillAllocPages(__in void * address, uint pageCount)
 #ifdef RECYCLER_MEMORY_VERIFY
     if (verifyEnabled)
     {
-        ChakraMemSet(address, Recycler::VerifyMemFill, bufferSize, this->processHandle);
+        Assert(this->processHandle == GetCurrentProcess());
+        memset(address, Recycler::VerifyMemFill, bufferSize);
         return;
     }
 #endif
@@ -979,7 +980,8 @@ PageAllocatorBase<T>::FillAllocPages(__in void * address, uint pageCount)
     if (ZeroPages())
     {
         // for release build, the page is zeroed in ReleasePages
-        ChakraMemSet(address, 0, bufferSize, this->processHandle);
+        Assert(this->processHandle == GetCurrentProcess());
+        memset(address, 0, bufferSize);
     }
 #endif
 }
@@ -1122,7 +1124,8 @@ PageAllocatorBase<T>::AllocSegment(size_t pageCount)
 #ifdef RECYCLER_MEMORY_VERIFY
     if (verifyEnabled)
     {
-        ChakraMemSet(segment->GetAddress(), Recycler::VerifyMemFill, AutoSystemInfo::PageSize * segment->GetPageCount(), this->processHandle);
+        Assert(this->processHandle == GetCurrentProcess());
+        memset(segment->GetAddress(), Recycler::VerifyMemFill, AutoSystemInfo::PageSize * segment->GetPageCount());
     }
 #endif
 
@@ -1678,7 +1681,8 @@ PageAllocatorBase<T>::ZeroQueuedPages()
         else
 #endif
         {
-        ChakraMemSet(freePageEntry, 0, pageCount * AutoSystemInfo::PageSize, this->processHandle);
+            Assert(this->processHandle == GetCurrentProcess());
+            memset(freePageEntry, 0, pageCount * AutoSystemInfo::PageSize);
         }
 
         QueuePages(freePageEntry, pageCount, segment);
@@ -1734,7 +1738,8 @@ PageAllocatorBase<T>::FlushBackgroundPages()
         DListBase<PageSegmentBase<T>> * fromSegmentList = GetSegmentList(segment);
         Assert(fromSegmentList != nullptr);
 
-        ChakraMemSet(freePageEntry, 0, sizeof(FreePageEntry), this->processHandle);
+        Assert(this->processHandle == GetCurrentProcess());
+        memset(freePageEntry, 0, sizeof(FreePageEntry));
 
         segment->ReleasePages(freePageEntry, pageCount);
         newFreePages += pageCount;
@@ -1835,7 +1840,8 @@ PageAllocatorBase<T>::DecommitNow(bool all)
             else
             {
                 // Zero them and release them in case we don't decommit them.
-                ChakraMemSet(freePageEntry, 0, pageCount * AutoSystemInfo::PageSize, this->processHandle);
+                Assert(this->processHandle == GetCurrentProcess());
+                memset(freePageEntry, 0, pageCount * AutoSystemInfo::PageSize);
                 segment->ReleasePages(freePageEntry, pageCount);
                 LogFreePages(pageCount);
             }

lib/Common/Memory/PageAllocator.h

@@ -399,6 +399,30 @@ public:
             throw Js::InternalErrorException();
         }
     }
+    static void CheckProcessAndThrowFatalError(HANDLE hProcess)
+    {
+        DWORD lastError = GetLastError();
+        if (MemOpLastError == 0)
+        {
+            MemOpLastError = lastError;
+        }
+        if (lastError != 0)
+        {
+            DWORD exitCode = STILL_ACTIVE;
+            if (!GetExitCodeProcess(hProcess, &exitCode) || exitCode == STILL_ACTIVE)
+            {
+                // REVIEW: In OOP JIT, target process is still alive but the memory operation failed 
+                // we should fail fast(terminate) the runtime process, fail fast here in server process
+                // is to capture bug might exist in CustomHeap implementation.
+                // if target process is already gone, we don't care the failure here
+
+                // REVIEW: the VM operation might fail if target process is in middle of terminating
+                // will GetExitCodeProcess return STILL_ACTIVE for such case?
+                Js::Throw::FatalInternalErrorEx(lastError);
+            }
+        }
+
+    }
     static void ClearLastError()
     {
         MemOpLastError = 0;

lib/JITServer/JITServer.cpp

@@ -633,9 +633,10 @@ void ServerContextManager::UnRegisterThreadContext(ServerThreadContext* threadCo
     auto iter = scriptContexts.GetIteratorWithRemovalSupport();
     while (iter.IsValid())
     {
-        if (iter.Current().Key()->GetThreadContext() == threadContext)
-        {
-            iter.Current().Key()->Close();
+        ServerScriptContext* scriptContext = iter.Current().Key();
+        if (scriptContext->GetThreadContext() == threadContext)
+        {   
+            scriptContext->Close();
             iter.RemoveCurrent();
         }
         iter.MoveNext();