rework JIT process memory errors

lib/Backend/CodeGenNumberAllocator.cpp

@@ -341,7 +341,8 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou
             // initialize number by WriteProcessMemory
             if (!WriteProcessMemory(hProcess, (void*)number, pLocalNumber, sizeCat, NULL))
             {
-                MemoryOperationLastError::RecordLastErrorAndThrow();
+                MemoryOperationLastError::RecordLastError();
+                Js::Throw::OutOfMemory();
             }
 
             return (Js::JavascriptNumber*) number;

lib/Backend/NativeCodeGenerator.cpp

@@ -3164,18 +3164,15 @@ bool NativeCodeGenerator::TryReleaseNonHiPriWorkItem(CodeGenWorkItem* workItem)
 void
 NativeCodeGenerator::FreeNativeCodeGenAllocation(void* address)
 {
-    if(this->backgroundAllocators)
+    if (JITManager::GetJITManager()->IsOOPJITEnabled())
     {
         ThreadContext * context = this->scriptContext->GetThreadContext();
-        if (JITManager::GetJITManager()->IsOOPJITEnabled())
-        {
-            // OOP JIT TODO: need error handling?
-            JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)address);
-        }
-        else
-        {
-            this->backgroundAllocators->emitBufferManager.FreeAllocation(address);
-        }
+        HRESULT hr = JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)address);
+        JITManager::HandleServerCallResult(hr, RemoteCallType::MemFree);
+    }
+    else if(this->backgroundAllocators)
+    {
+        this->backgroundAllocators->emitBufferManager.FreeAllocation(address);
     }
 }
 
@@ -3202,23 +3199,9 @@ NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* address)
     }
 
     // The foreground allocators may have been used
-    ThreadContext * context = this->scriptContext->GetThreadContext();
-    if(this->foregroundAllocators)
+    if(this->foregroundAllocators && this->foregroundAllocators->emitBufferManager.FreeAllocation(address))
     {
-        if (JITManager::GetJITManager()->IsOOPJITEnabled())
-        {
-            // TODO: OOP JIT, should we always just queue this in background?
-            // OOP JIT TODO: need error handling?
-            JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)address);
-            return;
-        }
-        else
-        {
-            if (this->foregroundAllocators->emitBufferManager.FreeAllocation(address))
-            {
-                return;
-            }
-        }
+        return;
     }
 
     // The background allocators were used. Queue a job to free the allocation from the background thread.
@@ -3664,7 +3647,15 @@ JITManager::HandleServerCallResult(HRESULT hr, RemoteCallType callType)
     case E_ABORT:
         throw Js::OperationAbortedException();
     case E_OUTOFMEMORY:
-        Js::Throw::OutOfMemory();
+        if (callType == RemoteCallType::MemFree)
+        {
+            // if freeing memory fails due to OOM, it means we failed to fill with debug breaks -- so failfast
+            RpcFailure_fatal_error(hr);
+        }
+        else
+        {
+            Js::Throw::OutOfMemory();
+        }
     case VBSERR_OutOfStack:
         throw Js::StackOverflowException();
     default:
@@ -3695,6 +3686,7 @@ JITManager::HandleServerCallResult(HRESULT hr, RemoteCallType callType)
     case RemoteCallType::ThunkCreation:
         Js::Throw::OutOfMemory();
     case RemoteCallType::StateUpdate:
+    case RemoteCallType::MemFree:
         // if server process is gone, we can ignore failures updating its state
         return;
     default:

lib/Common/CommonDefines.h

@@ -145,6 +145,8 @@
 #endif
 
 
+#define MAKE_HR(errnum) (MAKE_HRESULT(SEVERITY_ERROR, FACILITY_CONTROL, errnum))
+
 #if ENABLE_CONCURRENT_GC
 // Write-barrier refers to a software write barrier implementation using a card table.
 // Write watch refers to a hardware backed write-watch feature supported by the Windows memory manager.

lib/Common/Memory/CommonMemoryPch.h

@@ -21,6 +21,8 @@ typedef _Return_type_success_(return >= 0) LONG NTSTATUS;
 #include "Exceptions/ExceptionBase.h"
 #include "Exceptions/OutOfMemoryException.h"
 
+#include "../Parser/rterror.h"
+
 // Other Memory headers
 #include "Memory/LeakReport.h"
 #include "Memory/AutoPtr.h"
@@ -48,6 +50,7 @@ typedef _Return_type_success_(return >= 0) LONG NTSTATUS;
 #include "Memory/HeapBlock.inl"
 #include "Memory/HeapBlockMap.inl"
 
+
 // Memory Protections
 #ifdef _CONTROL_FLOW_GUARD
 #define PAGE_EXECUTE_RW_TARGETS_INVALID   (PAGE_EXECUTE_READWRITE | PAGE_TARGETS_INVALID)

lib/Common/Memory/CustomHeap.cpp

@@ -262,12 +262,12 @@ Allocation* Heap<TAlloc, TPreReservedAlloc>::Alloc(size_t bytes, ushort pdataCou
         size_t resultBytes = VirtualQueryEx(this->processHandle, page->address, &memBasicInfo, sizeof(memBasicInfo));
         if (resultBytes == 0)
         {
-            if (this->processHandle != GetCurrentProcess())
-            {
-                MemoryOperationLastError::RecordLastErrorAndThrow();
-            }
+            MemoryOperationLastError::RecordLastError();
+        }
+        else
+        {
+            Assert(memBasicInfo.Protect == PAGE_EXECUTE);
         }
-        Assert(memBasicInfo.Protect == PAGE_EXECUTE);
 #endif
 
         Allocation* allocation = nullptr;
@@ -439,12 +439,12 @@ Allocation* Heap<TAlloc, TPreReservedAlloc>::AllocLargeObject(size_t bytes, usho
     size_t resultBytes = VirtualQueryEx(this->processHandle, address, &memBasicInfo, sizeof(memBasicInfo));
     if (resultBytes == 0)
     {
-        if (this->processHandle != GetCurrentProcess())
-        {
-            MemoryOperationLastError::RecordLastErrorAndThrow();
-        }
+        MemoryOperationLastError::RecordLastError();
+    }
+    else
+    {
+        Assert(memBasicInfo.Protect == PAGE_EXECUTE);
     }
-    Assert(memBasicInfo.Protect == PAGE_EXECUTE);
 #endif
 
     Allocation* allocation = this->largeObjectAllocations.PrependNode(this->auxiliaryAllocator);
@@ -826,7 +826,7 @@ bool Heap<TAlloc, TPreReservedAlloc>::FreeAllocation(Allocation* object)
             char* localAddr = this->codePageAllocators->AllocLocal(object->address, object->size, page->segment);
             if (!localAddr)
             {
-                MemoryOperationLastError::CheckProcessAndThrowFatalError(this->processHandle);
+                MemoryOperationLastError::RecordError(JSERR_FatalMemoryExhaustion);
                 return false;
             }
             FillDebugBreak((BYTE*)localAddr, object->size);
@@ -903,7 +903,8 @@ void Heap<TAlloc, TPreReservedAlloc>::FreeAllocationHelper(Allocation* object, B
     }
     else
     {
-        MemoryOperationLastError::CheckProcessAndThrowFatalError(this->processHandle);
+        MemoryOperationLastError::RecordError(JSERR_FatalMemoryExhaustion);
+        return;
     }
     VerboseHeapTrace(_u("Setting %d bits starting at bit %d, Free bit vector in page was "), length, index);
 #if VERBOSE_HEAP

lib/Common/Memory/PageAllocator.cpp

@@ -7,7 +7,7 @@
 #define UpdateMinimum(dst, src) if (dst > src) { dst = src; }
 
 #if ENABLE_OOP_NATIVE_CODEGEN
-THREAD_LOCAL DWORD MemoryOperationLastError::MemOpLastError = 0;
+THREAD_LOCAL HRESULT MemoryOperationLastError::MemOpLastError = 0;
 #endif
 
 //=============================================================================================================
@@ -977,29 +977,18 @@ PageAllocatorBase<TVirtualAlloc, TSegment, TPageSegment>::FillAllocPages(__in vo
 
 #if DBG
 #ifdef RECYCLER_ZERO_MEM_CHECK
-    const bool isLocalProc = this->processHandle == GetCurrentProcess();
-    byte * readBuffer;
-    if (isLocalProc)
+    byte * localAddr = (byte *)this->GetVirtualAllocator()->AllocLocal(address, bufferSize);
+    if (!localAddr)
     {
-        readBuffer = (byte*)address;
-    }
-    else
-    {
-        readBuffer = HeapNewArray(byte, bufferSize);
-        if (!ReadProcessMemory(this->processHandle, address, readBuffer, bufferSize, NULL))
-        {
-            MemoryOperationLastError::RecordLastErrorAndThrow();
-        }
+        MemoryOperationLastError::RecordError(E_OUTOFMEMORY);
+        return;
     }
     for (size_t i = 0; i < bufferSize; i++)
     {
         // new pages are filled with zeros, old pages are filled with DbgMemFill
-        Assert(readBuffer[i] == 0 || readBuffer[i] == DbgMemFill);
-    }
-    if (!isLocalProc)
-    {
-        HeapDeleteArray(bufferSize, readBuffer);
+        Assert(localAddr[i] == 0 || localAddr[i] == DbgMemFill);
     }
+    this->GetVirtualAllocator()->FreeLocal(localAddr);
 #endif
 #endif
 
@@ -1640,7 +1629,7 @@ PageAllocatorBase<SectionAllocWrapper>::MemSetLocal(_In_ void *dst, int val, siz
     LPVOID localAddr = this->GetVirtualAllocator()->AllocLocal(dst, sizeInBytes);
     if (localAddr == nullptr)
     {
-        MemoryOperationLastError::CheckProcessAndThrowFatalError(this->processHandle);
+        MemoryOperationLastError::RecordError(JSERR_FatalMemoryExhaustion);
     }
     else
     {
@@ -1656,7 +1645,7 @@ PageAllocatorBase<PreReservedSectionAllocWrapper>::MemSetLocal(_In_ void *dst, i
     LPVOID localAddr = this->GetVirtualAllocator()->AllocLocal(dst, sizeInBytes);
     if (localAddr == nullptr)
     {
-        MemoryOperationLastError::CheckProcessAndThrowFatalError(this->processHandle);
+        MemoryOperationLastError::RecordError(JSERR_FatalMemoryExhaustion);
     }
     else
     {

lib/Common/Memory/PageAllocator.h

@@ -393,65 +393,40 @@ public:
     static void RecordLastError()
     {
 #if ENABLE_OOP_NATIVE_CODEGEN
-        if (MemOpLastError == 0)
+        if (MemOpLastError == S_OK)
         {
             MemOpLastError = GetLastError();
         }
 #endif
     }
-    static void RecordLastErrorAndThrow()
+
+    static void RecordError(HRESULT error)
     {
 #if ENABLE_OOP_NATIVE_CODEGEN
-        if (MemOpLastError == 0)
+        if (MemOpLastError == S_OK)
         {
-            MemOpLastError = GetLastError();
-            AssertOrFailFast(false);
+            MemOpLastError = HRESULT_FROM_WIN32(error);
         }
 #endif
     }
-    static void CheckProcessAndThrowFatalError(HANDLE hProcess)
-    {
-        DWORD lastError = GetLastError();
-#if ENABLE_OOP_NATIVE_CODEGEN
-        if (MemOpLastError == 0)
-        {
-            MemOpLastError = lastError;
-        }
-#endif
-        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()
     {
 #if ENABLE_OOP_NATIVE_CODEGEN
-        MemOpLastError = 0;
+        MemOpLastError = S_OK;
 #endif
     }
-    static DWORD GetLastError()
+    static HRESULT GetLastError()
     {
 #if ENABLE_OOP_NATIVE_CODEGEN
         return MemOpLastError;
 #else
-        return 0;
+        return S_OK;
 #endif
     }
 #if ENABLE_OOP_NATIVE_CODEGEN
 private:
-    THREAD_LOCAL static DWORD MemOpLastError;
+    THREAD_LOCAL static HRESULT MemOpLastError;
 #endif
 };
 

lib/Common/Memory/VirtualAllocWrapper.h

@@ -24,6 +24,8 @@ class VirtualAllocWrapper
 public:
     LPVOID  Alloc(LPVOID lpAddress, DECLSPEC_GUARD_OVERFLOW size_t dwSize, DWORD allocationType, DWORD protectFlags, bool isCustomHeapAllocation);
     BOOL    Free(LPVOID lpAddress, size_t dwSize, DWORD dwFreeType);
+    LPVOID  AllocLocal(LPVOID lpAddress, DECLSPEC_GUARD_OVERFLOW size_t dwSize) { return lpAddress; }
+    BOOL    FreeLocal(LPVOID lpAddress) { return true; }
 
     static VirtualAllocWrapper Instance;  // single instance
 private:
@@ -54,6 +56,8 @@ public:
     ~PreReservedVirtualAllocWrapper();
     LPVOID      Alloc(LPVOID lpAddress, DECLSPEC_GUARD_OVERFLOW size_t dwSize, DWORD allocationType, DWORD protectFlags, bool isCustomHeapAllocation);
     BOOL        Free(LPVOID lpAddress,  size_t dwSize, DWORD dwFreeType);
+    LPVOID  AllocLocal(LPVOID lpAddress, DECLSPEC_GUARD_OVERFLOW size_t dwSize) { return lpAddress; }
+    BOOL    FreeLocal(LPVOID lpAddress) { return true; }
 
     bool        IsInRange(void * address);
     static bool IsInRange(void * regionStart, void * address);

lib/JITClient/JITManager.h

@@ -13,7 +13,8 @@ enum class RemoteCallType
     CodeGen,
     ThunkCreation,
     HeapQuery,
-    StateUpdate
+    StateUpdate,
+    MemFree
 };
 
 #if _WIN32 || ENABLE_OOP_NATIVE_CODEGEN

lib/JITServer/JITServer.cpp

@@ -913,15 +913,7 @@ HRESULT ServerCallWrapper(ServerThreadContext* threadContextInfo, Fn fn)
         AssertOrFailFastMsg(false, "Unknown exception caught in JIT server call.");
     }
 
-    if (hr == E_OUTOFMEMORY)
-    {
-        if (HRESULT_FROM_WIN32(MemoryOperationLastError::GetLastError()) != S_OK)
-        {
-            hr = HRESULT_FROM_WIN32(MemoryOperationLastError::GetLastError());
-        }
-    }
-
-    return hr;
+    return MemoryOperationLastError::GetLastError();
 }
 
 template<typename Fn>

lib/Parser/ParserCommon.h

@@ -55,4 +55,3 @@ typedef SList<IdentPtr, ArenaAllocator> IdentPtrList;
 // Below was moved from scrutil.h to share with chakradiag.
 //
 #define HR(sc) ((HRESULT)(sc))
-#define MAKE_HR(vbserr) (MAKE_HRESULT(SEVERITY_ERROR, FACILITY_CONTROL, vbserr))

lib/Parser/rterror.cpp

@@ -3,7 +3,6 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 #include "ParserPch.h"
-#include "rterror.h"
 
 // PUBLIC ERROR codes
 

lib/Parser/rterrors.h

@@ -364,6 +364,7 @@ RT_ERROR_MSG(JSERR_CannotSuspendBuffer, 5665, "", "Current agent cannot be suspe
 RT_ERROR_MSG(JSERR_CantDeleteNonConfigProp, 5666, "Cannot delete non-configurable property '%s'", "Cannot delete non-configurable property", kjstTypeError, 0)
 RT_ERROR_MSG(JSERR_CantRedefineProp, 5667, "Cannot redefine property '%s'", "Cannot redefine property", kjstTypeError, 0)
 RT_ERROR_MSG(JSERR_FunctionArgument_NeedArrayLike, 5668, "%s: argument is not an array or array-like object", "Array or array-like object expected", kjstTypeError, 0)
+RT_ERROR_MSG(JSERR_FatalMemoryExhaustion, 5669, "", "Encountered a non-recoverable OOM", kjstError, 0)
 
 // WebAssembly Errors
 RT_ERROR_MSG(WASMERR_WasmCompileError, 7000, "%s", "Compilation failed.", kjstWebAssemblyCompileError, 0)