From 53114fe2b38b9474550b2455b519da9024a10e1a Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 10:07:47 -0700 Subject: [PATCH 01/13] OOP JIT error handling adding a new type of exception to indicate system call failed(mostly because content process is terminated/crash and VM is deleted, the JIT server is still trying to do the VM operations, which fails) wrap all server call in exception handling make relationship between scriptcontext and threadcontext in server side --- lib/Backend/CodeGenNumberAllocator.cpp | 12 +- lib/Backend/EmitBuffer.cpp | 7 + lib/Backend/NativeCodeGenerator.cpp | 26 +- lib/Backend/ServerScriptContext.cpp | 25 +- lib/Backend/ServerScriptContext.h | 13 +- lib/Backend/ServerThreadContext.cpp | 17 + lib/Backend/ServerThreadContext.h | 10 +- lib/Common/Common.h | 1 + lib/Common/Common/Jobs.cpp | 9 + .../Chakra.Common.Exceptions.vcxproj | 3 +- .../Exceptions/JITOperationFailedException.h | 18 + lib/Common/Exceptions/Throw.cpp | 25 + lib/Common/Exceptions/Throw.h | 3 + lib/Common/Memory/CommonMemoryPch.h | 1 + lib/Common/Memory/CustomHeap.cpp | 12 +- lib/Common/Memory/PageAllocator.cpp | 23 +- lib/Common/Memory/VirtualAllocWrapper.cpp | 58 ++- lib/JITClient/JITManager.cpp | 3 +- lib/JITClient/JITManager.h | 4 + lib/JITIDL/ChakraJIT.idl | 1 + lib/JITServer/Chakra.JITServer.vcxproj | 1 + .../Chakra.JITServer.vcxproj.filters | 7 +- lib/JITServer/JITServer.cpp | 444 +++++++++++++----- lib/JITServer/JITServer.h | 43 ++ lib/JITServer/JITServerPch.h | 1 + lib/Runtime/Base/ScriptContext.cpp | 15 +- lib/Runtime/Base/ThreadContext.cpp | 1 + lib/Runtime/Base/ThreadContextInfo.cpp | 22 +- lib/Runtime/Base/ThreadContextInfo.h | 8 +- 29 files changed, 627 insertions(+), 186 deletions(-) create mode 100644 lib/Common/Exceptions/JITOperationFailedException.h create mode 100644 lib/JITServer/JITServer.h diff --git a/lib/Backend/CodeGenNumberAllocator.cpp b/lib/Backend/CodeGenNumberAllocator.cpp index 97de96e6934..9fbc8fba4d3 100644 --- a/lib/Backend/CodeGenNumberAllocator.cpp +++ b/lib/Backend/CodeGenNumberAllocator.cpp @@ -345,9 +345,13 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou { Assert((unsigned int)((char*)tail->GetEndAddress() - (char*)tail->GetCommitEndAddress()) >= BlockSize); // TODO: implement guard pages (still necessary for OOP JIT?) - auto ret = ::VirtualAllocEx(hProcess, tail->GetCommitEndAddress(), BlockSize, MEM_COMMIT, PAGE_READWRITE); - if (!ret) + LPVOID addr = ::VirtualAllocEx(hProcess, tail->GetCommitEndAddress(), BlockSize, MEM_COMMIT, PAGE_READWRITE); + if (addr == nullptr) { + if (hProcess != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } Js::Throw::OutOfMemory(); } tail->committedEnd += BlockSize; @@ -359,6 +363,10 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou void* pages = ::VirtualAllocEx(hProcess, nullptr, PageCount * AutoSystemInfo::PageSize, MEM_RESERVE, PAGE_READWRITE); if (pages == nullptr) { + if (hProcess != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } Js::Throw::OutOfMemory(); } diff --git a/lib/Backend/EmitBuffer.cpp b/lib/Backend/EmitBuffer.cpp index 6d065771027..55f2b674301 100644 --- a/lib/Backend/EmitBuffer.cpp +++ b/lib/Backend/EmitBuffer.cpp @@ -290,6 +290,13 @@ EmitBufferAllocation* EmitBufferManager::AllocateBuffer(__in size_t #if DBG MEMORY_BASIC_INFORMATION memBasicInfo; size_t resultBytes = VirtualQueryEx(this->processHandle, allocation->allocation->address, &memBasicInfo, sizeof(memBasicInfo)); + if (resultBytes == 0) + { + if (this->processHandle != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + } Assert(resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE); #endif diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index bc61a90a41a..f09f85f8f90 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -909,6 +909,9 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor Js::Throw::OutOfMemory(); case VBSERR_OutOfStack: throw Js::StackOverflowException(); + case E_ACCESSDENIED: + // OOP JIT TODO: if server side can't handle request any more, use better error code and turn off JIT + throw Js::JITOperationFailedException(workItem->codeGenResult); default: Js::Throw::FatalInternalError(); } @@ -2026,18 +2029,6 @@ NativeCodeGenerator::UpdateJITState() scriptContext->InitializeRemoteScriptContext(); } - bool allowPrereserveAlloc = true; -#if !_M_X64_OR_ARM64 - if (this->scriptContext->webWorkerId != Js::Constants::NonWebWorkerContextId) - { - allowPrereserveAlloc = false; - } -#endif -#ifndef _CONTROL_FLOW_GUARD - allowPrereserveAlloc = false; -#endif - scriptContext->GetThreadContext()->EnsureJITThreadContext(allowPrereserveAlloc); - // update all property records on server that have been changed since last jit ThreadContext::PropertyMap * pendingProps = scriptContext->GetThreadContext()->GetPendingJITProperties(); PropertyRecordIDL ** newPropArray = nullptr; @@ -2080,10 +2071,7 @@ NativeCodeGenerator::UpdateJITState() props.newRecordCount = newCount; props.newRecordArray = newPropArray; HRESULT hr = JITManager::GetJITManager()->UpdatePropertyRecordMap(scriptContext->GetThreadContext()->GetRemoteThreadContextAddr(), &props); - if (hr != S_OK) - { - Js::Throw::FatalInternalError(); - } + if (newPropArray) { HeapDeleteArray(newCount, newPropArray); @@ -2092,6 +2080,12 @@ NativeCodeGenerator::UpdateJITState() { HeapDeleteArray(reclaimedCount, reclaimedPropArray); } + + if (hr != S_OK) + { + // OOP JIT TODO: use better exception when failed to update JIT server state + Js::Throw::OutOfMemory(); + } } } } diff --git a/lib/Backend/ServerScriptContext.cpp b/lib/Backend/ServerScriptContext.cpp index 76bfb975e79..f99a01097bc 100644 --- a/lib/Backend/ServerScriptContext.cpp +++ b/lib/Backend/ServerScriptContext.cpp @@ -5,16 +5,16 @@ #include "Backend.h" -ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData) : +ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData, ServerThreadContext* threadContextInfo) : m_contextData(*contextData), + threadContextInfo(threadContextInfo), m_isPRNGSeeded(false), - m_isClosed(false), m_domFastPathHelperMap(nullptr), m_moduleRecords(&HeapAllocator::Instance), #ifdef PROFILE_EXEC m_codeGenProfiler(nullptr), #endif - m_activeJITCount(0) + m_refCount(0) { #ifdef PROFILE_EXEC if (Js::Configuration::Global.flags.IsEnabled(Js::ProfileFlag)) @@ -279,21 +279,19 @@ ServerScriptContext::Close() } void -ServerScriptContext::BeginJIT() +ServerScriptContext::AddRef() { - InterlockedExchangeAdd(&m_activeJITCount, 1u); + InterlockedExchangeAdd(&m_refCount, 1u); } void -ServerScriptContext::EndJIT() +ServerScriptContext::Release() { - InterlockedExchangeSubtract(&m_activeJITCount, 1u); -} - -bool -ServerScriptContext::IsJITActive() -{ - return m_activeJITCount != 0; + InterlockedExchangeSubtract(&m_refCount, 1u); + if (m_isClosed && m_refCount == 0) + { + HeapDelete(this); + } } Js::Var* @@ -328,3 +326,4 @@ ServerScriptContext::GetCodeGenProfiler() const return nullptr; #endif } + diff --git a/lib/Backend/ServerScriptContext.h b/lib/Backend/ServerScriptContext.h index 17dd5da02a5..71895bc5f50 100644 --- a/lib/Backend/ServerScriptContext.h +++ b/lib/Backend/ServerScriptContext.h @@ -8,7 +8,7 @@ class ServerScriptContext : public ScriptContextInfo { public: - ServerScriptContext(ScriptContextDataIDL * contextData); + ServerScriptContext(ScriptContextDataIDL * contextData, ServerThreadContext* threadContextInfo); ~ServerScriptContext(); virtual intptr_t GetNullAddr() const override; virtual intptr_t GetUndefinedAddr() const override; @@ -62,11 +62,12 @@ class ServerScriptContext : public ScriptContextInfo void AddModuleRecordInfo(unsigned int moduleId, __int64 localExportSlotsAddr); Js::ScriptContextProfiler * GetCodeGenProfiler() const; + ServerThreadContext* GetThreadContext() { return threadContextInfo; } void Close(); - void BeginJIT(); - void EndJIT(); - bool IsJITActive(); + void AddRef(); + void Release(); + private: JITDOMFastPathHelperMap * m_domFastPathHelperMap; #ifdef PROFILE_EXEC @@ -74,7 +75,9 @@ class ServerScriptContext : public ScriptContextInfo #endif ScriptContextDataIDL m_contextData; - uint m_activeJITCount; + + ServerThreadContext* threadContextInfo; + uint m_refCount; bool m_isPRNGSeeded; bool m_isClosed; diff --git a/lib/Backend/ServerThreadContext.cpp b/lib/Backend/ServerThreadContext.cpp index 867d1db5295..18dc4ee125b 100644 --- a/lib/Backend/ServerThreadContext.cpp +++ b/lib/Backend/ServerThreadContext.cpp @@ -260,3 +260,20 @@ ServerThreadContext::AddToPropertyMap(const Js::PropertyRecord * origRecord) PropertyRecordTrace(_u("Added JIT property '%s' at 0x%08x, pid = %d\n"), record->GetBuffer(), record, record->pid); } + +void ServerThreadContext::AddRef() +{ + InterlockedExchangeAdd(&m_refCount, (uint)1); +} +void ServerThreadContext::Release() +{ + InterlockedExchangeSubtract(&m_refCount, (uint)1); + if (m_isClosed && m_refCount == 0) + { + HeapDelete(this); + } +} +void ServerThreadContext::Close() +{ + this->m_isClosed = true; +} diff --git a/lib/Backend/ServerThreadContext.h b/lib/Backend/ServerThreadContext.h index 7fb69d60855..5829e2b295e 100644 --- a/lib/Backend/ServerThreadContext.h +++ b/lib/Backend/ServerThreadContext.h @@ -45,6 +45,11 @@ class ServerThreadContext : public ThreadContextInfo void RemoveFromPropertyMap(Js::PropertyId reclaimedId); void AddToPropertyMap(const Js::PropertyRecord * propertyRecord); void SetWellKnownHostTypeId(Js::TypeId typeId) { this->wellKnownHostTypeHTMLAllCollectionTypeId = typeId; } + + void AddRef(); + void Release(); + void Close(); + private: intptr_t GetRuntimeChakraBaseAddress() const; intptr_t GetRuntimeCRTBaseAddress() const; @@ -60,8 +65,9 @@ class ServerThreadContext : public ThreadContextInfo CodeGenAllocators m_codeGenAlloc; ThreadContextDataIDL m_threadContextData; - - ThreadContext * m_threadContext; + intptr_t m_jitChakraBaseAddress; intptr_t m_jitCRTBaseAddress; + uint m_refCount; + }; diff --git a/lib/Common/Common.h b/lib/Common/Common.h index ac783999dc0..db462fc4077 100644 --- a/lib/Common/Common.h +++ b/lib/Common/Common.h @@ -83,6 +83,7 @@ template<> struct IntMath { using Type = Int64Math; }; #include "Exceptions/StackOverflowException.h" #include "Exceptions/NotImplementedException.h" #include "Exceptions/AsmJsParseException.h" +#include "Exceptions/JITOperationFailedException.h" #include "Memory/AutoPtr.h" #include "Memory/AutoAllocatorObjectPtr.h" diff --git a/lib/Common/Common/Jobs.cpp b/lib/Common/Common/Jobs.cpp index e3000cf9d0e..fd78b18d720 100644 --- a/lib/Common/Common/Jobs.cpp +++ b/lib/Common/Common/Jobs.cpp @@ -14,6 +14,7 @@ #include "Exceptions/OperationAbortedException.h" #include "Exceptions/OutOfMemoryException.h" #include "Exceptions/StackOverflowException.h" +#include "Exceptions/JITOperationFailedException.h" #include "TemplateParameter.h" #include "DataStructures/DoublyLinkedListElement.h" @@ -1004,6 +1005,14 @@ namespace JsUtil job->failureReason = Job::FailureReason::Aborted; #endif } + catch (Js::JITOperationFailedException) + { + // This can happen for any reason a job needs to be aborted while executing, like for instance, if the script + // context is closed while the job is being processed in the background +#if ENABLE_DEBUG_CONFIG_OPTIONS + job->failureReason = Job::FailureReason::Unknown; +#endif + } // Since the background job processor processes jobs on a background thread, out-of-memory and stack overflow need to be // caught here. Script would not be active in the background thread, so (at the time of this writing) a diff --git a/lib/Common/Exceptions/Chakra.Common.Exceptions.vcxproj b/lib/Common/Exceptions/Chakra.Common.Exceptions.vcxproj index e34397ffcce..e439bf9ab30 100644 --- a/lib/Common/Exceptions/Chakra.Common.Exceptions.vcxproj +++ b/lib/Common/Exceptions/Chakra.Common.Exceptions.vcxproj @@ -47,6 +47,7 @@ + @@ -58,4 +59,4 @@ - + \ No newline at end of file diff --git a/lib/Common/Exceptions/JITOperationFailedException.h b/lib/Common/Exceptions/JITOperationFailedException.h new file mode 100644 index 00000000000..dfb872d57af --- /dev/null +++ b/lib/Common/Exceptions/JITOperationFailedException.h @@ -0,0 +1,18 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- +#pragma once + +namespace Js { + + class JITOperationFailedException : public ExceptionBase + { + public: + JITOperationFailedException(DWORD lastError) + :LastError(lastError) + {} + DWORD LastError; + }; + +} // namespace Js diff --git a/lib/Common/Exceptions/Throw.cpp b/lib/Common/Exceptions/Throw.cpp index 6558f8f0450..96f28c703e5 100644 --- a/lib/Common/Exceptions/Throw.cpp +++ b/lib/Common/Exceptions/Throw.cpp @@ -18,6 +18,7 @@ #include "InternalErrorException.h" #include "OutOfMemoryException.h" #include "NotImplementedException.h" +#include "JITOperationFailedException.h" // Header files required before including ConfigFlagsTable.h @@ -121,6 +122,30 @@ namespace Js { throw StackOverflowException(); } + void Throw::JITOperationFailed(DWORD lastError) + { +#ifdef ENABLE_DEBUG_CONFIG_OPTIONS + if (CONFIG_FLAG(PrintSystemException)) + { + Output::Print(_u("SystemException: JITOperationFailed\n")); + Output::Flush(); + } +#endif + throw JITOperationFailedException(lastError); + } + + void Throw::CheckAndThrowJITOperationFailed() + { + DWORD lastError = GetLastError(); + // currently this is used for virtual memory(Virtual*Ex) operations only + // which 0 indicate succeed + if (lastError != 0) + { + Throw::JITOperationFailed(lastError); + } + + } + void Throw::NotImplemented() { AssertMsg(false, "This functionality is not yet implemented"); diff --git a/lib/Common/Exceptions/Throw.h b/lib/Common/Exceptions/Throw.h index 2deb577f648..26629c68d63 100644 --- a/lib/Common/Exceptions/Throw.h +++ b/lib/Common/Exceptions/Throw.h @@ -21,6 +21,9 @@ namespace Js { static void __declspec(noreturn) InternalError(); static void __declspec(noreturn) FatalInternalError(); static void __declspec(noreturn) FatalProjectionError(); + static void __declspec(noreturn) JITOperationFailed(DWORD lastError); + + static void CheckAndThrowJITOperationFailed(); static void CheckAndThrowOutOfMemory(BOOLEAN status); diff --git a/lib/Common/Memory/CommonMemoryPch.h b/lib/Common/Memory/CommonMemoryPch.h index 8a8549676cf..1daf3bd53a5 100644 --- a/lib/Common/Memory/CommonMemoryPch.h +++ b/lib/Common/Memory/CommonMemoryPch.h @@ -20,6 +20,7 @@ typedef _Return_type_success_(return >= 0) LONG NTSTATUS; // Exceptions #include "Exceptions/ExceptionBase.h" #include "Exceptions/OutOfMemoryException.h" +#include "Exceptions/JITOperationFailedException.h" // Other Memory headers #include "Memory/LeakReport.h" diff --git a/lib/Common/Memory/CustomHeap.cpp b/lib/Common/Memory/CustomHeap.cpp index f23c8a75596..296f7b03f38 100644 --- a/lib/Common/Memory/CustomHeap.cpp +++ b/lib/Common/Memory/CustomHeap.cpp @@ -223,7 +223,11 @@ Allocation* Heap::Alloc(size_t bytes, ushort pdataCount, ushort xdataSize, bool { MEMORY_BASIC_INFORMATION memBasicInfo; size_t resultBytes = VirtualQueryEx(this->processHandle, allocation->address, &memBasicInfo, sizeof(memBasicInfo)); - Assert(resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE); + if (resultBytes == 0) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + Assert(memBasicInfo.Protect == PAGE_EXECUTE); } #endif return allocation; @@ -254,7 +258,11 @@ Allocation* Heap::Alloc(size_t bytes, ushort pdataCount, ushort xdataSize, bool #if defined(DBG) MEMORY_BASIC_INFORMATION memBasicInfo; size_t resultBytes = VirtualQueryEx(this->processHandle, page->address, &memBasicInfo, sizeof(memBasicInfo)); - Assert(resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE); + if (resultBytes == 0) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + Assert(memBasicInfo.Protect == PAGE_EXECUTE); #endif Allocation* allocation = nullptr; diff --git a/lib/Common/Memory/PageAllocator.cpp b/lib/Common/Memory/PageAllocator.cpp index d2117ed35d6..113556e6cbc 100644 --- a/lib/Common/Memory/PageAllocator.cpp +++ b/lib/Common/Memory/PageAllocator.cpp @@ -210,7 +210,11 @@ PageSegmentBase::Initialize(DWORD allocFlags, bool excludeGuardPages) { DWORD oldProtect; BOOL vpresult = VirtualProtectEx(this->allocator->processHandle, this->address, this->GetAvailablePageCount() * AutoSystemInfo::PageSize, PAGE_NOACCESS, &oldProtect); - Assert(vpresult && oldProtect == PAGE_READWRITE); + if (vpresult == FALSE) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + Assert(oldProtect == PAGE_READWRITE); } return true; } @@ -294,6 +298,10 @@ PageSegmentBase::AllocPages(uint pageCount) #ifdef PAGEALLOCATOR_PROTECT_FREEPAGE DWORD oldProtect; BOOL vpresult = VirtualProtectEx(this->allocator->processHandle, allocAddress, pageCount * AutoSystemInfo::PageSize, PAGE_READWRITE, &oldProtect); + if (vpresult == FALSE) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } Assert(vpresult && oldProtect == PAGE_NOACCESS); #endif return allocAddress; @@ -394,6 +402,10 @@ PageSegmentBase::ReleasePages(__in void * address, uint pageCount) #ifdef PAGEALLOCATOR_PROTECT_FREEPAGE DWORD oldProtect; BOOL vpresult = VirtualProtectEx(this->allocator->processHandle, address, pageCount * AutoSystemInfo::PageSize, PAGE_NOACCESS, &oldProtect); + if (vpresult == FALSE) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } Assert(vpresult && oldProtect == PAGE_READWRITE); #endif @@ -2370,6 +2382,7 @@ HeapPageAllocator::ProtectPages(__in char* address, size_t pageCount, __in vo || address < segment->GetAddress() || ((uint)(((char *)address) - segment->GetAddress()) > (segment->GetPageCount() - pageCount) * AutoSystemInfo::PageSize)) { + // OOPJIT TODO: don't bring down the whole JIT process CustomHeap_BadPageState_fatal_error((ULONG_PTR)this); return FALSE; } @@ -2378,6 +2391,13 @@ HeapPageAllocator::ProtectPages(__in char* address, size_t pageCount, __in vo // check old protection on all pages about to change, ensure the fidelity size_t bytes = VirtualQueryEx(this->processHandle, address, &memBasicInfo, sizeof(memBasicInfo)); + if (bytes == 0) + { + if (this->processHandle != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + } if (bytes == 0 || memBasicInfo.RegionSize < pageCount * AutoSystemInfo::PageSize || desiredOldProtectFlag != memBasicInfo.Protect) @@ -2392,6 +2412,7 @@ HeapPageAllocator::ProtectPages(__in char* address, size_t pageCount, __in vo (dwVirtualProtectFlags & (PAGE_EXECUTE | PAGE_EXECUTE_READ | PAGE_EXECUTE_READWRITE)) && ((dwVirtualProtectFlags & PAGE_TARGETS_NO_UPDATE) == 0)) { + // OOPJIT TODO: don't bring down the whole JIT process CustomHeap_BadPageState_fatal_error((ULONG_PTR)this); return FALSE; } diff --git a/lib/Common/Memory/VirtualAllocWrapper.cpp b/lib/Common/Memory/VirtualAllocWrapper.cpp index 1dbaeb47ca8..2a6f9981553 100644 --- a/lib/Common/Memory/VirtualAllocWrapper.cpp +++ b/lib/Common/Memory/VirtualAllocWrapper.cpp @@ -44,13 +44,27 @@ LPVOID VirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DWORD allocat { allocProtectFlags = PAGE_EXECUTE_READWRITE; } + address = VirtualAllocEx(process, lpAddress, dwSize, allocationType, allocProtectFlags); - VirtualProtectEx(process, address, dwSize, protectFlags, &oldProtectFlags); + if (address == nullptr && process != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + + BOOL result = VirtualProtectEx(process, address, dwSize, protectFlags, &oldProtectFlags); + if (result == FALSE && process != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } } else #endif { address = VirtualAllocEx(process, lpAddress, dwSize, allocationType, protectFlags); + if (address == nullptr && process != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } } return address; @@ -62,7 +76,12 @@ BOOL VirtualAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFreeType AnalysisAssert(dwFreeType == MEM_RELEASE || dwFreeType == MEM_DECOMMIT); size_t bytes = (dwFreeType == MEM_RELEASE)? 0 : dwSize; #pragma warning(suppress: 28160) // Calling VirtualFreeEx without the MEM_RELEASE flag frees memory but not address descriptors (VADs) - return VirtualFreeEx(process, lpAddress, bytes, dwFreeType); + BOOL ret = VirtualFreeEx(process, lpAddress, bytes, dwFreeType); + if (ret == FALSE && process != GetCurrentProcess()) + { + // OOP JIT TODO: check if we need to cleanup the context related to this content process + } + return ret; } /* @@ -88,9 +107,9 @@ PreReservedVirtualAllocWrapper::~PreReservedVirtualAllocWrapper() BOOL success = VirtualFreeEx(processHandle, preReservedStartAddress, 0, MEM_RELEASE); PreReservedHeapTrace(_u("MEM_RELEASE the PreReservedSegment. Start Address: 0x%p, Size: 0x%x * 0x%x bytes"), preReservedStartAddress, PreReservedAllocationSegmentCount, AutoSystemInfo::Data.GetAllocationGranularityPageSize()); - if (!success) + if (!success && this->processHandle != GetCurrentProcess()) { - Assert(false); + // OOP JIT TODO: check if we need to cleanup the context related to this content process } #if !_M_X64_OR_ARM64 && _CONTROL_FLOW_GUARD @@ -119,10 +138,14 @@ PreReservedVirtualAllocWrapper::IsInRange(void * address) //Check if the region is in MEM_COMMIT state. MEMORY_BASIC_INFORMATION memBasicInfo; size_t bytes = VirtualQueryEx(processHandle, address, &memBasicInfo, sizeof(memBasicInfo)); - if (bytes == 0 || memBasicInfo.State != MEM_COMMIT) + if (bytes == 0) { - AssertMsg(false, "Memory not committed? Checking for uncommitted address region?"); + if (this->processHandle != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } } + AssertMsg(memBasicInfo.State == MEM_COMMIT, "Memory not committed? Checking for uncommitted address region?"); #endif return result; } @@ -291,6 +314,13 @@ LPVOID PreReservedVirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW //Check if the region is not already in MEM_COMMIT state. MEMORY_BASIC_INFORMATION memBasicInfo; size_t bytes = VirtualQueryEx(processHandle, addressToReserve, &memBasicInfo, sizeof(memBasicInfo)); + if (bytes == 0) + { + if (this->processHandle != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + } if (bytes == 0 || memBasicInfo.RegionSize < requestedNumOfSegments * AutoSystemInfo::Data.GetAllocationGranularityPageSize() || memBasicInfo.State == MEM_COMMIT @@ -345,7 +375,11 @@ LPVOID PreReservedVirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW if (allocatedAddress != nullptr) { - VirtualProtectEx(processHandle, allocatedAddress, dwSize, protectFlags, &oldProtect); + BOOL result = VirtualProtectEx(processHandle, allocatedAddress, dwSize, protectFlags, &oldProtect); + if (result == FALSE && this->processHandle != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } AssertMsg(oldProtect == (PAGE_EXECUTE_READWRITE), "CFG Bitmap gets allocated and bits will be set to invalid only upon passing these flags."); } } @@ -353,6 +387,10 @@ LPVOID PreReservedVirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW #endif { allocatedAddress = (char *)VirtualAllocEx(processHandle, addressToReserve, dwSize, MEM_COMMIT, protectFlags); + if (allocatedAddress == nullptr && this->processHandle != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } } } else @@ -425,6 +463,12 @@ PreReservedVirtualAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFr freeSegments.SetRange(freeSegmentsBVIndex, static_cast(requestedNumOfSegments)); PreReservedHeapTrace(_u("MEM_RELEASE: Address: 0x%p of size: 0x%x * 0x%x bytes\n"), lpAddress, requestedNumOfSegments, AutoSystemInfo::Data.GetAllocationGranularityPageSize()); } + + if (success == FALSE && process != GetCurrentProcess()) + { + // OOP JIT TODO: check if we need to cleanup the context related to this content process + } + return success; } } diff --git a/lib/JITClient/JITManager.cpp b/lib/JITClient/JITManager.cpp index aadbccd43f8..a58d940a008 100644 --- a/lib/JITClient/JITManager.cpp +++ b/lib/JITClient/JITManager.cpp @@ -443,6 +443,7 @@ JITManager::UpdatePropertyRecordMap( HRESULT JITManager::InitializeScriptContext( __in ScriptContextDataIDL * data, + __in intptr_t threadContextInfoAddress, __out intptr_t * scriptContextInfoAddress) { Assert(IsOOPJITEnabled()); @@ -450,7 +451,7 @@ JITManager::InitializeScriptContext( HRESULT hr = E_FAIL; RpcTryExcept { - hr = ClientInitializeScriptContext(m_rpcBindingHandle, data, scriptContextInfoAddress); + hr = ClientInitializeScriptContext(m_rpcBindingHandle, data, threadContextInfoAddress, scriptContextInfoAddress); } RpcExcept(RpcExceptionFilter(RpcExceptionCode())) { diff --git a/lib/JITClient/JITManager.h b/lib/JITClient/JITManager.h index 1f924393a84..b34b2702868 100644 --- a/lib/JITClient/JITManager.h +++ b/lib/JITClient/JITManager.h @@ -47,6 +47,7 @@ class JITManager HRESULT InitializeScriptContext( __in ScriptContextDataIDL * data, + __in intptr_t threadContextInfoAddress, __out intptr_t *scriptContextInfoAddress); HRESULT CleanupProcess(); @@ -78,6 +79,7 @@ class JITManager HRESULT Shutdown(); + static JITManager * GetJITManager(); private: JITManager(); @@ -96,6 +98,7 @@ class JITManager bool m_isJITServer; static JITManager s_jitManager; + }; #else // !ENABLE_OOP_NATIVE_CODEGEN @@ -148,6 +151,7 @@ class JITManager HRESULT InitializeScriptContext( __in ScriptContextDataIDL * data, + __in intptr_t threadContextInfoAddress, __out intptr_t *scriptContextInfoAddress) { Assert(false); return E_FAIL; } diff --git a/lib/JITIDL/ChakraJIT.idl b/lib/JITIDL/ChakraJIT.idl index 603cd06094f..40c0fbd6ad6 100644 --- a/lib/JITIDL/ChakraJIT.idl +++ b/lib/JITIDL/ChakraJIT.idl @@ -52,6 +52,7 @@ interface IChakraJIT HRESULT InitializeScriptContext( [in] handle_t binding, [in] ScriptContextDataIDL * scriptContextData, + [in] CHAKRA_PTR threadContextInfoAddress, [out] CHAKRA_PTR * scriptContextInfoAddress); HRESULT CloseScriptContext( diff --git a/lib/JITServer/Chakra.JITServer.vcxproj b/lib/JITServer/Chakra.JITServer.vcxproj index 17529415049..762b647e8a0 100644 --- a/lib/JITServer/Chakra.JITServer.vcxproj +++ b/lib/JITServer/Chakra.JITServer.vcxproj @@ -28,6 +28,7 @@ + diff --git a/lib/JITServer/Chakra.JITServer.vcxproj.filters b/lib/JITServer/Chakra.JITServer.vcxproj.filters index f7b1c72f2e5..d854214adbb 100644 --- a/lib/JITServer/Chakra.JITServer.vcxproj.filters +++ b/lib/JITServer/Chakra.JITServer.vcxproj.filters @@ -1,11 +1,12 @@  - - - + + + + \ No newline at end of file diff --git a/lib/JITServer/JITServer.cpp b/lib/JITServer/JITServer.cpp index 2c5980cd865..af231c286c0 100644 --- a/lib/JITServer/JITServer.cpp +++ b/lib/JITServer/JITServer.cpp @@ -131,7 +131,13 @@ ServerInitializeThreadContext( { AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); - ServerThreadContext * contextInfo = HeapNew(ServerThreadContext, threadContextData); + ServerThreadContext * contextInfo = HeapNewNoThrow(ServerThreadContext, threadContextData); + if (contextInfo == nullptr) + { + return E_OUTOFMEMORY; + } + + ServerContextManager::RegisterThreadContext(contextInfo); *threadContextRoot = (intptr_t)EncodePointer(contextInfo); *prereservedRegionAddr = (intptr_t)contextInfo->GetPreReservedVirtualAllocator()->EnsurePreReservedRegion(); @@ -152,8 +158,15 @@ ServerCleanupThreadContext( return RPC_S_INVALID_ARG; } - while (threadContextInfo->IsJITActive()) { Sleep(1); } - HeapDelete(threadContextInfo); + if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) + { + return E_ACCESSDENIED; + } + + AutoReleaseContext autoThreadContext(threadContextInfo); + + threadContextInfo->Close(); + ServerContextManager::UnRegisterThreadContext(threadContextInfo); return S_OK; } @@ -173,17 +186,26 @@ ServerUpdatePropertyRecordMap( return RPC_S_INVALID_ARG; } - for (uint i = 0; i < updatedProps->reclaimedPropertyCount; ++i) + if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { - threadContextInfo->RemoveFromPropertyMap((Js::PropertyId)updatedProps->reclaimedPropertyIdArray[i]); + return E_ACCESSDENIED; } - for (uint i = 0; i < updatedProps->newRecordCount; ++i) + AutoReleaseContext autoThreadContext(threadContextInfo); + return ServerCallWrapper(threadContextInfo, [&]()->HRESULT { - threadContextInfo->AddToPropertyMap((Js::PropertyRecord *)updatedProps->newRecordArray[i]); - } + for (uint i = 0; i < updatedProps->reclaimedPropertyCount; ++i) + { + threadContextInfo->RemoveFromPropertyMap((Js::PropertyId)updatedProps->reclaimedPropertyIdArray[i]); + } - return S_OK; + for (uint i = 0; i < updatedProps->newRecordCount; ++i) + { + threadContextInfo->AddToPropertyMap((Js::PropertyRecord *)updatedProps->newRecordArray[i]); + } + + return S_OK; + }); } HRESULT @@ -202,9 +224,17 @@ ServerAddDOMFastPathHelper( return RPC_S_INVALID_ARG; } - scriptContextInfo->AddToDOMFastPathHelperMap(funcInfoAddr, (IR::JnHelperMethod)helper); + if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + { + return RPC_S_INVALID_ARG; + } - return S_OK; + AutoReleaseContext autoScriptContext(scriptContextInfo); + return ServerCallWrapper(scriptContextInfo, [&]()->HRESULT + { + scriptContextInfo->AddToDOMFastPathHelperMap(funcInfoAddr, (IR::JnHelperMethod)helper); + return S_OK; + }); } HRESULT @@ -221,10 +251,19 @@ ServerAddModuleRecordInfo( { return RPC_S_INVALID_ARG; } - serverScriptContext->AddModuleRecordInfo(moduleId, localExportSlotsAddr); - HRESULT hr = E_FAIL; - return hr; + if (!ServerContextManager::IsScriptContextAlive(serverScriptContext)) + { + return RPC_S_INVALID_ARG; + } + + AutoReleaseContext autoScriptContext(serverScriptContext); + return ServerCallWrapper(serverScriptContext, [&]()->HRESULT + { + serverScriptContext->AddModuleRecordInfo(moduleId, localExportSlotsAddr); + return S_OK; + }); + } HRESULT @@ -239,27 +278,56 @@ ServerSetWellKnownHostTypeId( { return RPC_S_INVALID_ARG; } - threadContextInfo->SetWellKnownHostTypeId((Js::TypeId)typeId); - return S_OK; + if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) + { + return E_ACCESSDENIED; + } + + AutoReleaseContext autoThreadContext(threadContextInfo); + return ServerCallWrapper(threadContextInfo, [&]()->HRESULT + { + threadContextInfo->SetWellKnownHostTypeId((Js::TypeId)typeId); + return S_OK; + }); + } HRESULT ServerInitializeScriptContext( /* [in] */ handle_t binding, /* [in] */ __RPC__in ScriptContextDataIDL * scriptContextData, + /* [in] */ __RPC__in intptr_t threadContextInfoAddress, /* [out] */ __RPC__out intptr_t * scriptContextInfoAddress) { AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); - ServerScriptContext * contextInfo = HeapNew(ServerScriptContext, scriptContextData); - *scriptContextInfoAddress = (intptr_t)EncodePointer(contextInfo); + ServerThreadContext * threadContextInfo = (ServerThreadContext*)DecodePointer((void*)threadContextInfoAddress); + + if (threadContextInfo == nullptr) + { + return E_ACCESSDENIED; + } + + if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) + { + return E_ACCESSDENIED; + } + + return ServerCallWrapper(threadContextInfo, [&]()->HRESULT + { + ServerScriptContext * contextInfo = HeapNew(ServerScriptContext, scriptContextData, threadContextInfo); + + ServerContextManager::RegisterScriptContext(contextInfo); + + *scriptContextInfoAddress = (intptr_t)EncodePointer(contextInfo); #if !FLOATVAR - // TODO: should move this to ServerInitializeThreadContext, also for the fields in IDL - XProcNumberPageSegmentImpl::Initialize(contextInfo->IsRecyclerVerifyEnabled(), contextInfo->GetRecyclerVerifyPad()); + // TODO: should move this to ServerInitializeThreadContext, also for the fields in IDL + XProcNumberPageSegmentImpl::Initialize(contextInfo->IsRecyclerVerifyEnabled(), contextInfo->GetRecyclerVerifyPad()); #endif - return S_OK; + return S_OK; + }); } HRESULT @@ -274,6 +342,13 @@ ServerCloseScriptContext( return RPC_S_INVALID_ARG; } + if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + { + return E_ACCESSDENIED; + } + + AutoReleaseContext autoScriptContext(scriptContextInfo); + #ifdef PROFILE_EXEC auto profiler = scriptContextInfo->GetCodeGenProfiler(); if (profiler && profiler->IsInitialized()) @@ -283,6 +358,7 @@ ServerCloseScriptContext( #endif scriptContextInfo->Close(); + ServerContextManager::UnRegisterScriptContext(scriptContextInfo); return S_OK; } @@ -298,8 +374,15 @@ ServerCleanupScriptContext( return RPC_S_INVALID_ARG; } - while (scriptContextInfo->IsJITActive()) { Sleep(1); } - HeapDelete(scriptContextInfo); + if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + { + return E_ACCESSDENIED; + } + + AutoReleaseContext autoScriptContext(scriptContextInfo); + + scriptContextInfo->Close(); + ServerContextManager::UnRegisterScriptContext(scriptContextInfo); return S_OK; } @@ -317,11 +400,17 @@ ServerFreeAllocation( return RPC_S_INVALID_ARG; } - //DeRegister Entry Point for CFG - context->SetValidCallTargetForCFG((PVOID)address, false); - - bool succeeded = context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)address); - return succeeded ? S_OK : E_FAIL; + if (!ServerContextManager::IsThreadContextAlive(context)) + { + return E_ACCESSDENIED; + } + AutoReleaseContext autoThreadContext(context); + return ServerCallWrapper(context, [&]()->HRESULT + { + context->SetValidCallTargetForCFG((PVOID)address, false); + bool succeeded = context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)address); + return succeeded ? S_OK : E_FAIL; + }); } HRESULT @@ -339,22 +428,32 @@ ServerIsNativeAddr( return RPC_S_INVALID_ARG; } - PreReservedVirtualAllocWrapper *preReservedVirtualAllocWrapper = context->GetPreReservedVirtualAllocator(); - if (preReservedVirtualAllocWrapper->IsInRange((void*)address)) - { - *result = true; - } - else if (!context->IsAllJITCodeInPreReservedRegion()) - { - CustomHeap::CodePageAllocators::AutoLock autoLock(context->GetCodePageAllocators()); - *result = context->GetCodePageAllocators()->IsInNonPreReservedPageAllocator((void*)address); - } - else + if (!ServerContextManager::IsThreadContextAlive(context)) { *result = false; + return E_ACCESSDENIED; } - return S_OK; + AutoReleaseContext autoThreadContext(context); + return ServerCallWrapper(context, [&]()->HRESULT + { + PreReservedVirtualAllocWrapper *preReservedVirtualAllocWrapper = context->GetPreReservedVirtualAllocator(); + if (preReservedVirtualAllocWrapper->IsInRange((void*)address)) + { + *result = true; + } + else if (!context->IsAllJITCodeInPreReservedRegion()) + { + CustomHeap::CodePageAllocators::AutoLock autoLock(context->GetCodePageAllocators()); + *result = context->GetCodePageAllocators()->IsInNonPreReservedPageAllocator((void*)address); + } + else + { + *result = false; + } + + return S_OK; + }); } HRESULT @@ -364,9 +463,24 @@ ServerSetIsPRNGSeeded( /* [in] */ boolean value) { ServerScriptContext * scriptContextInfo = (ServerScriptContext*)DecodePointer((void*)scriptContextInfoAddress); - scriptContextInfo->SetIsPRNGSeeded(value != FALSE); - return S_OK; + if (scriptContextInfo == nullptr) + { + return RPC_S_INVALID_ARG; + } + + if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + { + return RPC_S_INVALID_ARG; + } + + AutoReleaseContext autoScriptContext(scriptContextInfo); + + return ServerCallWrapper(scriptContextInfo, [&]()->HRESULT + { + scriptContextInfo->SetIsPRNGSeeded(value != FALSE); + return S_OK; + }); } HRESULT @@ -380,13 +494,6 @@ ServerRemoteCodeGen( UNREFERENCED_PARAMETER(binding); AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); - LARGE_INTEGER start_time = { 0 }; - if (PHASE_TRACE1(Js::BackEndPhase)) - { - QueryPerformanceCounter(&start_time); - } - memset(jitData, 0, sizeof(JITOutputIDL)); - ServerThreadContext * threadContextInfo = (ServerThreadContext*)DecodePointer((void*)threadContextInfoAddress); ServerScriptContext * scriptContextInfo = (ServerScriptContext*)DecodePointer((void*)scriptContextInfoAddress); @@ -395,51 +502,64 @@ ServerRemoteCodeGen( return RPC_S_INVALID_ARG; } - NoRecoverMemoryJitArenaAllocator jitArena(L"JITArena", threadContextInfo->GetPageAllocator(), Js::Throw::OutOfMemory); + if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) + { + return E_ACCESSDENIED; + } - scriptContextInfo->BeginJIT(); // TODO: OOP JIT, improve how we do this - threadContextInfo->BeginJIT(); + if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + { + return E_ACCESSDENIED; + } - JITTimeWorkItem * jitWorkItem = Anew(&jitArena, JITTimeWorkItem, workItemData); + AutoReleaseContext autoThreadContext(threadContextInfo); + AutoReleaseContext autoScriptContext(scriptContextInfo); - if (PHASE_VERBOSE_TRACE_RAW(Js::BackEndPhase, jitWorkItem->GetJITTimeInfo()->GetSourceContextId(), jitWorkItem->GetJITTimeInfo()->GetLocalFunctionId())) + return ServerCallWrapper(threadContextInfo, [&]() ->HRESULT { - LARGE_INTEGER freq; - LARGE_INTEGER end_time; - QueryPerformanceCounter(&end_time); - QueryPerformanceFrequency(&freq); + LARGE_INTEGER start_time = { 0 }; + if (PHASE_TRACE1(Js::BackEndPhase)) + { + QueryPerformanceCounter(&start_time); + } + memset(jitData, 0, sizeof(JITOutputIDL)); - Output::Print( - L"BackendMarshalIn - function: %s time:%8.6f mSec\r\n", - jitWorkItem->GetJITFunctionBody()->GetDisplayName(), - (((double)((end_time.QuadPart - workItemData->startTime)* (double)1000.0 / (double)freq.QuadPart))) / (1)); - Output::Flush(); - } + NoRecoverMemoryJitArenaAllocator jitArena(L"JITArena", threadContextInfo->GetPageAllocator(), Js::Throw::OutOfMemory); + JITTimeWorkItem * jitWorkItem = Anew(&jitArena, JITTimeWorkItem, workItemData); - auto profiler = scriptContextInfo->GetCodeGenProfiler(); + if (PHASE_VERBOSE_TRACE_RAW(Js::BackEndPhase, jitWorkItem->GetJITTimeInfo()->GetSourceContextId(), jitWorkItem->GetJITTimeInfo()->GetLocalFunctionId())) + { + LARGE_INTEGER freq; + LARGE_INTEGER end_time; + QueryPerformanceCounter(&end_time); + QueryPerformanceFrequency(&freq); + + Output::Print( + L"BackendMarshalIn - function: %s time:%8.6f mSec\r\n", + jitWorkItem->GetJITFunctionBody()->GetDisplayName(), + (((double)((end_time.QuadPart - workItemData->startTime)* (double)1000.0 / (double)freq.QuadPart))) / (1)); + Output::Flush(); + } + + auto profiler = scriptContextInfo->GetCodeGenProfiler(); #ifdef PROFILE_EXEC - if (profiler && !profiler->IsInitialized()) - { - profiler->Initialize(threadContextInfo->GetPageAllocator(), nullptr); - } + if (profiler && !profiler->IsInitialized()) + { + profiler->Initialize(threadContextInfo->GetPageAllocator(), nullptr); + } #endif - if (jitWorkItem->GetWorkItemData()->xProcNumberPageSegment) - { - jitData->numberPageSegments = (XProcNumberPageSegment*)midl_user_allocate(sizeof(XProcNumberPageSegment)); - if (!jitData->numberPageSegments) + if (jitWorkItem->GetWorkItemData()->xProcNumberPageSegment) { - scriptContextInfo->EndJIT(); - threadContextInfo->EndJIT(); - - return E_OUTOFMEMORY; + jitData->numberPageSegments = (XProcNumberPageSegment*)midl_user_allocate(sizeof(XProcNumberPageSegment)); + if (!jitData->numberPageSegments) + { + return E_OUTOFMEMORY; + } + __analysis_assume(jitData->numberPageSegments); + + memcpy_s(jitData->numberPageSegments, sizeof(XProcNumberPageSegment), jitWorkItem->GetWorkItemData()->xProcNumberPageSegment, sizeof(XProcNumberPageSegment)); } - __analysis_assume(jitData->numberPageSegments); - memcpy_s(jitData->numberPageSegments, sizeof(XProcNumberPageSegment), jitWorkItem->GetWorkItemData()->xProcNumberPageSegment, sizeof(XProcNumberPageSegment)); - } - HRESULT hr = S_OK; - try - { Func::Codegen( &jitArena, jitWorkItem, @@ -455,6 +575,105 @@ ServerRemoteCodeGen( #endif profiler, true); + + +#ifdef PROFILE_EXEC + if (profiler && profiler->IsInitialized()) + { + profiler->ProfilePrint(Js::Configuration::Global.flags.Profile.GetFirstPhase()); + } +#endif + + if (PHASE_VERBOSE_TRACE_RAW(Js::BackEndPhase, jitWorkItem->GetJITTimeInfo()->GetSourceContextId(), jitWorkItem->GetJITTimeInfo()->GetLocalFunctionId())) + { + LARGE_INTEGER freq; + LARGE_INTEGER end_time; + QueryPerformanceCounter(&end_time); + QueryPerformanceFrequency(&freq); + + Output::Print( + L"EndBackEndInner - function: %s time:%8.6f mSec\r\n", + jitWorkItem->GetJITFunctionBody()->GetDisplayName(), + (((double)((end_time.QuadPart - start_time.QuadPart)* (double)1000.0 / (double)freq.QuadPart))) / (1)); + Output::Flush(); + + } + LARGE_INTEGER out_time = { 0 }; + if (PHASE_TRACE1(Js::BackEndPhase)) + { + QueryPerformanceCounter(&out_time); + jitData->startTime = out_time.QuadPart; + } + + return S_OK; + }); +} + +JsUtil::BaseHashSet ServerContextManager::threadContexts(&HeapAllocator::Instance); +JsUtil::BaseHashSet ServerContextManager::scriptContexts(&HeapAllocator::Instance); +CriticalSection ServerContextManager::cs; + +void ServerContextManager::RegisterThreadContext(ServerThreadContext* threadContext) +{ + AutoCriticalSection autoCS(&cs); + threadContexts.Add(threadContext); +} + +void ServerContextManager::UnRegisterThreadContext(ServerThreadContext* threadContext) +{ + AutoCriticalSection autoCS(&cs); + threadContexts.Remove(threadContext); + auto iter = scriptContexts.GetIteratorWithRemovalSupport(); + while (iter.IsValid()) + { + if (iter.Current().Key()->GetThreadContext() == threadContext) + { + iter.Current().Key()->Close(); + iter.RemoveCurrent(); + } + iter.MoveNext(); + } +} + +bool ServerContextManager::IsThreadContextAlive(ServerThreadContext* threadContext) +{ + AutoCriticalSection autoCS(&cs); + if (threadContexts.LookupWithKey(threadContext)) + { + return !threadContext->IsClosed(); + } + return false; +} + +void ServerContextManager::RegisterScriptContext(ServerScriptContext* scriptContext) +{ + AutoCriticalSection autoCS(&cs); + scriptContexts.Add(scriptContext); +} + +void ServerContextManager::UnRegisterScriptContext(ServerScriptContext* scriptContext) +{ + AutoCriticalSection autoCS(&cs); + scriptContexts.Remove(scriptContext); +} + +bool ServerContextManager::IsScriptContextAlive(ServerScriptContext* scriptContext) +{ + AutoCriticalSection autoCS(&cs); + if (scriptContexts.LookupWithKey(scriptContext)) + { + return !scriptContext->IsClosed(); + } + return false; +} + +template +HRESULT ServerCallWrapper(ServerThreadContext* threadContextInfo, Fn fn) +{ + HRESULT hr = S_OK; + try + { + hr = fn(); } catch (Js::OutOfMemoryException) { @@ -468,35 +687,42 @@ ServerRemoteCodeGen( { hr = E_ABORT; } - scriptContextInfo->EndJIT(); - threadContextInfo->EndJIT(); - -#ifdef PROFILE_EXEC - if (profiler && profiler->IsInitialized()) + catch (Js::JITOperationFailedException& ex) { - profiler->ProfilePrint(Js::Configuration::Global.flags.Profile.GetFirstPhase()); - } -#endif + hr = HRESULT_FROM_WIN32(ex.LastError); - if (PHASE_VERBOSE_TRACE_RAW(Js::BackEndPhase, jitWorkItem->GetJITTimeInfo()->GetSourceContextId(), jitWorkItem->GetJITTimeInfo()->GetLocalFunctionId())) - { - LARGE_INTEGER freq; - LARGE_INTEGER end_time; - QueryPerformanceCounter(&end_time); - QueryPerformanceFrequency(&freq); + if (hr == E_ACCESSDENIED) + { + // target process might be terminated + DWORD exitCode; + if (GetExitCodeProcess(threadContextInfo->GetProcessHandle(), &exitCode)) + { + if (exitCode != STILL_ACTIVE) + { + threadContextInfo->Close(); + ServerContextManager::UnRegisterThreadContext(threadContextInfo); + + } + } + } + else + { + // OOPJIT TODO: other error code might need to be handled + Assert(false); + } + } - Output::Print( - L"EndBackEndInner - function: %s time:%8.6f mSec\r\n", - jitWorkItem->GetJITFunctionBody()->GetDisplayName(), - (((double)((end_time.QuadPart - start_time.QuadPart)* (double)1000.0 / (double)freq.QuadPart))) / (1)); - Output::Flush(); + return hr; +} - } - LARGE_INTEGER out_time = { 0 }; - if (PHASE_TRACE1(Js::BackEndPhase)) +template +HRESULT ServerCallWrapper(ServerScriptContext* scriptContextInfo, Fn fn) +{ + ServerThreadContext* threadContextInfo = scriptContextInfo->GetThreadContext(); + if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { - QueryPerformanceCounter(&out_time); - jitData->startTime = out_time.QuadPart; + return E_ACCESSDENIED; } - return hr; -} + AutoReleaseContext autoThreadContext(threadContextInfo); + return ServerCallWrapper(threadContextInfo, fn); +} \ No newline at end of file diff --git a/lib/JITServer/JITServer.h b/lib/JITServer/JITServer.h new file mode 100644 index 00000000000..7641e237e8b --- /dev/null +++ b/lib/JITServer/JITServer.h @@ -0,0 +1,43 @@ + +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +class ServerContextManager +{ +public: + static void RegisterThreadContext(ServerThreadContext* threadContext); + static void UnRegisterThreadContext(ServerThreadContext* threadContext); + static bool IsThreadContextAlive(ServerThreadContext* threadContext); + + static void RegisterScriptContext(ServerScriptContext* scriptContext); + static void UnRegisterScriptContext(ServerScriptContext* scriptContext); + static bool IsScriptContextAlive(ServerScriptContext* scriptContext); +private: + static JsUtil::BaseHashSet threadContexts; + static JsUtil::BaseHashSet scriptContexts; + static CriticalSection cs; +}; + +template +struct AutoReleaseContext +{ + AutoReleaseContext(T* context) + :context(context) + { + context->AddRef(); + } + + ~AutoReleaseContext() + { + context->Release(); + } + + T* context; +}; + +template +HRESULT ServerCallWrapper(ServerThreadContext* threadContextInfo, Fn fn); +template +HRESULT ServerCallWrapper(ServerScriptContext* scriptContextInfo, Fn fn); diff --git a/lib/JITServer/JITServerPch.h b/lib/JITServer/JITServerPch.h index 9c6a5785d1e..95d193976ae 100644 --- a/lib/JITServer/JITServerPch.h +++ b/lib/JITServer/JITServerPch.h @@ -11,3 +11,4 @@ #include "Runtime.h" #include "Backend.h" +#include "JITServer.h" diff --git a/lib/Runtime/Base/ScriptContext.cpp b/lib/Runtime/Base/ScriptContext.cpp index 2432a71dfe1..913998ccff5 100644 --- a/lib/Runtime/Base/ScriptContext.cpp +++ b/lib/Runtime/Base/ScriptContext.cpp @@ -4495,7 +4495,20 @@ void ScriptContext::RegisterPrototypeChainEnsuredToHaveOnlyWritableDataPropertie { contextData.vtableAddresses[i] = vtblAddresses[i]; } - JITManager::GetJITManager()->InitializeScriptContext(&contextData, &m_remoteScriptContextAddr); + + bool allowPrereserveAlloc = true; +#if !_M_X64_OR_ARM64 + if (this->webWorkerId != Js::Constants::NonWebWorkerContextId) + { + allowPrereserveAlloc = false; + } +#endif +#ifndef _CONTROL_FLOW_GUARD + allowPrereserveAlloc = false; +#endif + this->GetThreadContext()->EnsureJITThreadContext(allowPrereserveAlloc); + + JITManager::GetJITManager()->InitializeScriptContext(&contextData, this->GetThreadContext()->GetRemoteThreadContextAddr(), &m_remoteScriptContextAddr); } #endif diff --git a/lib/Runtime/Base/ThreadContext.cpp b/lib/Runtime/Base/ThreadContext.cpp index 405cc00b753..f7c6a5a36df 100644 --- a/lib/Runtime/Base/ThreadContext.cpp +++ b/lib/Runtime/Base/ThreadContext.cpp @@ -1979,6 +1979,7 @@ ThreadContext::EnsureJITThreadContext(bool allowPrereserveAlloc) { return; } + ThreadContextDataIDL contextData; contextData.processHandle = (intptr_t)JITManager::GetJITManager()->GetJITTargetHandle(); diff --git a/lib/Runtime/Base/ThreadContextInfo.cpp b/lib/Runtime/Base/ThreadContextInfo.cpp index 87dee000e0d..ec46331afd9 100644 --- a/lib/Runtime/Base/ThreadContextInfo.cpp +++ b/lib/Runtime/Base/ThreadContextInfo.cpp @@ -12,8 +12,7 @@ ThreadContextInfo::ThreadContextInfo() : m_isAllJITCodeInPreReservedRegion(true), - wellKnownHostTypeHTMLAllCollectionTypeId(Js::TypeIds_Undefined), - m_activeJITCount(0) + wellKnownHostTypeHTMLAllCollectionTypeId(Js::TypeIds_Undefined) { } @@ -411,25 +410,12 @@ ThreadContextInfo::SetValidCallTargetForCFG(PVOID callTargetAddress, bool isSetV #endif // _CONTROL_FLOW_GUARD } -void -ThreadContextInfo::BeginJIT() -{ - InterlockedExchangeAdd(&m_activeJITCount, (uint)1); -} - -void -ThreadContextInfo::EndJIT() +bool +ThreadContextInfo::IsClosed() { - InterlockedExchangeSubtract(&m_activeJITCount, (uint)1); + return m_isClosed; } -bool -ThreadContextInfo::IsJITActive() -{ - return m_activeJITCount != 0; -} - - intptr_t SHIFT_ADDR(const ThreadContextInfo*const context, intptr_t address) { #if ENABLE_OOP_NATIVE_CODEGEN diff --git a/lib/Runtime/Base/ThreadContextInfo.h b/lib/Runtime/Base/ThreadContextInfo.h index 7fff5251b71..aa1bdd476a0 100644 --- a/lib/Runtime/Base/ThreadContextInfo.h +++ b/lib/Runtime/Base/ThreadContextInfo.h @@ -100,9 +100,8 @@ class ThreadContextInfo bool CanBeFalsy(Js::TypeId typeId) { return typeId == this->wellKnownHostTypeHTMLAllCollectionTypeId; } bool IsCFGEnabled(); - void BeginJIT(); - void EndJIT(); - bool IsJITActive(); + bool IsClosed(); + #if defined(ENABLE_GLOBALIZATION) && defined(_CONTROL_FLOW_GUARD) Js::DelayLoadWinCoreMemory * GetWinCoreMemoryLibrary(); @@ -113,10 +112,9 @@ class ThreadContextInfo #endif protected: Js::TypeId wellKnownHostTypeHTMLAllCollectionTypeId; -private: - uint m_activeJITCount; bool m_isAllJITCodeInPreReservedRegion; + bool m_isClosed; }; From 4613d284f21ced22ec3ff3adc8582ac7741e0e7d Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 10:13:30 -0700 Subject: [PATCH 02/13] handle exception for ReadProcessMemory/WriteProcessMemory as well --- lib/Backend/CodeGenNumberAllocator.cpp | 1 + lib/Common/Memory/CustomHeap.cpp | 1 + lib/Common/Memory/MemUtils.cpp | 3 +++ lib/Common/Memory/PageAllocator.cpp | 1 + 4 files changed, 6 insertions(+) diff --git a/lib/Backend/CodeGenNumberAllocator.cpp b/lib/Backend/CodeGenNumberAllocator.cpp index 9fbc8fba4d3..d648480267b 100644 --- a/lib/Backend/CodeGenNumberAllocator.cpp +++ b/lib/Backend/CodeGenNumberAllocator.cpp @@ -333,6 +333,7 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou if (!WriteProcessMemory(hProcess, (void*)number, pLocalNumber, sizeCat, &bytesWritten) || bytesWritten != sizeCat) { + Js::Throw::CheckAndThrowJITOperationFailed(); Output::Print(_u("FATAL ERROR: WriteProcessMemory failed, GLE: %d\n"), GetLastError()); Js::Throw::FatalInternalError(); // TODO: don't bring down whole server process, but pass the last error to main process } diff --git a/lib/Common/Memory/CustomHeap.cpp b/lib/Common/Memory/CustomHeap.cpp index 296f7b03f38..30503ca82e2 100644 --- a/lib/Common/Memory/CustomHeap.cpp +++ b/lib/Common/Memory/CustomHeap.cpp @@ -1069,6 +1069,7 @@ void FillDebugBreak(_In_ BYTE* buffer, __in size_t byteCount, HANDLE processHand { if (!WriteProcessMemory(processHandle, buffer, writeBuffer, byteCount, NULL)) { + Js::Throw::CheckAndThrowJITOperationFailed(); Js::Throw::FatalInternalError(); } HeapDeleteArray(byteCount, writeBuffer); diff --git a/lib/Common/Memory/MemUtils.cpp b/lib/Common/Memory/MemUtils.cpp index 086abd6daef..0a7866f9fd8 100644 --- a/lib/Common/Memory/MemUtils.cpp +++ b/lib/Common/Memory/MemUtils.cpp @@ -24,6 +24,8 @@ Memory::ChakraMemSet(_In_ void *dst, int val, size_t sizeInBytes, HANDLE process { if (!WriteProcessMemory(processHandle, dst, writeBuffer, sizeInBytes, NULL)) { + Js::Throw::CheckAndThrowJITOperationFailed(); + // if it's not E_ACCESSDENIED Js::Throw::FatalInternalError(); } HeapDeleteArray(sizeInBytes, writeBuffer); @@ -48,6 +50,7 @@ Memory::ChakraMemCopy(_In_ void *dst, size_t sizeInBytes, _In_reads_bytes_(count if (!WriteProcessMemory(processHandle, dst, src, count, NULL)) { Output::Print(_u("FATAL ERROR: WriteProcessMemory failed, GLE: %d\n"), GetLastError()); + Js::Throw::CheckAndThrowJITOperationFailed(); Js::Throw::FatalInternalError(); } } diff --git a/lib/Common/Memory/PageAllocator.cpp b/lib/Common/Memory/PageAllocator.cpp index 113556e6cbc..aa265cae2c0 100644 --- a/lib/Common/Memory/PageAllocator.cpp +++ b/lib/Common/Memory/PageAllocator.cpp @@ -933,6 +933,7 @@ PageAllocatorBase::FillAllocPages(__in void * address, uint pageCount) readBuffer = HeapNewArray(byte, bufferSize); if (!ReadProcessMemory(this->processHandle, address, readBuffer, bufferSize, NULL)) { + Js::Throw::CheckAndThrowJITOperationFailed(); Js::Throw::InternalError(); } } From 603ab7e575f43350b2de80bbb4cee562c9828a64 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 10:58:13 -0700 Subject: [PATCH 03/13] handle more last error than just E_ACCESSDENIED --- lib/JITServer/JITServer.cpp | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/JITServer/JITServer.cpp b/lib/JITServer/JITServer.cpp index af231c286c0..f476e32e0bb 100644 --- a/lib/JITServer/JITServer.cpp +++ b/lib/JITServer/JITServer.cpp @@ -689,26 +689,22 @@ HRESULT ServerCallWrapper(ServerThreadContext* threadContextInfo, Fn fn) } catch (Js::JITOperationFailedException& ex) { + // pass the HRESULT back to content process + // TODO: looks mostly the error is E_ACCESSDENIED because the content process is gone + // however, it might be some other error if the content process is in middle of terminating + // like VM is in a state of removing but not completed yet hr = HRESULT_FROM_WIN32(ex.LastError); - if (hr == E_ACCESSDENIED) + DWORD exitCode; + if (!GetExitCodeProcess(threadContextInfo->GetProcessHandle(), &exitCode)) { - // target process might be terminated - DWORD exitCode; - if (GetExitCodeProcess(threadContextInfo->GetProcessHandle(), &exitCode)) - { - if (exitCode != STILL_ACTIVE) - { - threadContextInfo->Close(); - ServerContextManager::UnRegisterThreadContext(threadContextInfo); - - } - } + Assert(false); // fail to check target process } - else + + if (exitCode != STILL_ACTIVE) { - // OOPJIT TODO: other error code might need to be handled - Assert(false); + threadContextInfo->Close(); + ServerContextManager::UnRegisterThreadContext(threadContextInfo); } } From 5ee1b86646fa5c2fa82a4a6c27e3c332ae8c03c3 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 11:05:35 -0700 Subject: [PATCH 04/13] fix prefast warning --- lib/JITServer/JITServer.cpp | 16 ++++++++-------- lib/JITServer/JITServer.h | 1 - 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/JITServer/JITServer.cpp b/lib/JITServer/JITServer.cpp index f476e32e0bb..1e9a96b5274 100644 --- a/lib/JITServer/JITServer.cpp +++ b/lib/JITServer/JITServer.cpp @@ -297,7 +297,7 @@ HRESULT ServerInitializeScriptContext( /* [in] */ handle_t binding, /* [in] */ __RPC__in ScriptContextDataIDL * scriptContextData, - /* [in] */ __RPC__in intptr_t threadContextInfoAddress, + /* [in] */ intptr_t threadContextInfoAddress, /* [out] */ __RPC__out intptr_t * scriptContextInfoAddress) { AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); @@ -494,6 +494,13 @@ ServerRemoteCodeGen( UNREFERENCED_PARAMETER(binding); AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); + LARGE_INTEGER start_time = { 0 }; + if (PHASE_TRACE1(Js::BackEndPhase)) + { + QueryPerformanceCounter(&start_time); + } + memset(jitData, 0, sizeof(JITOutputIDL)); + ServerThreadContext * threadContextInfo = (ServerThreadContext*)DecodePointer((void*)threadContextInfoAddress); ServerScriptContext * scriptContextInfo = (ServerScriptContext*)DecodePointer((void*)scriptContextInfoAddress); @@ -517,13 +524,6 @@ ServerRemoteCodeGen( return ServerCallWrapper(threadContextInfo, [&]() ->HRESULT { - LARGE_INTEGER start_time = { 0 }; - if (PHASE_TRACE1(Js::BackEndPhase)) - { - QueryPerformanceCounter(&start_time); - } - memset(jitData, 0, sizeof(JITOutputIDL)); - NoRecoverMemoryJitArenaAllocator jitArena(L"JITArena", threadContextInfo->GetPageAllocator(), Js::Throw::OutOfMemory); JITTimeWorkItem * jitWorkItem = Anew(&jitArena, JITTimeWorkItem, workItemData); diff --git a/lib/JITServer/JITServer.h b/lib/JITServer/JITServer.h index 7641e237e8b..7b9fdcf7e3b 100644 --- a/lib/JITServer/JITServer.h +++ b/lib/JITServer/JITServer.h @@ -1,4 +1,3 @@ - //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. From 0d1a983e4953ea205235bca062a886f24483d5e3 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 11:45:35 -0700 Subject: [PATCH 05/13] handle the failure in client side --- lib/Backend/CodeGenWorkItem.cpp | 1 - lib/Backend/CodeGenWorkItem.h | 3 -- lib/Backend/NativeCodeGenerator.cpp | 46 ++++++++++--------- lib/JITClient/JITManager.h | 1 + lib/Runtime/Base/ThreadContext.cpp | 11 ++--- .../Language/SourceTextModuleRecord.cpp | 3 +- 6 files changed, 32 insertions(+), 33 deletions(-) diff --git a/lib/Backend/CodeGenWorkItem.cpp b/lib/Backend/CodeGenWorkItem.cpp index 6da5bd22743..6a7f2aca26f 100644 --- a/lib/Backend/CodeGenWorkItem.cpp +++ b/lib/Backend/CodeGenWorkItem.cpp @@ -20,7 +20,6 @@ CodeGenWorkItem::CodeGenWorkItem( , isAllocationCommitted(false) , queuedFullJitWorkItem(nullptr) , allocation(nullptr) - , codeGenResult(0) #ifdef IR_VIEWER , isRejitIRViewerFunction(false) , irViewerOutput(nullptr) diff --git a/lib/Backend/CodeGenWorkItem.h b/lib/Backend/CodeGenWorkItem.h index 3788d3c5df2..81a8387fff0 100644 --- a/lib/Backend/CodeGenWorkItem.h +++ b/lib/Backend/CodeGenWorkItem.h @@ -23,9 +23,6 @@ struct CodeGenWorkItem : public JsUtil::Job size_t codeAddress; ptrdiff_t codeSize; -public: - HRESULT codeGenResult; - public: virtual uint GetByteCodeCount() const = 0; virtual size_t GetDisplayName(_Out_writes_opt_z_(sizeInChars) WCHAR* displayName, _In_ size_t sizeInChars) = 0; diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index f09f85f8f90..25242ce0eb0 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -894,27 +894,12 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor workItem->GetJITData()->startTime = (int64)start_time.QuadPart; if (JITManager::GetJITManager()->IsOOPJITEnabled()) { - workItem->codeGenResult = JITManager::GetJITManager()->RemoteCodeGenCall( + HRESULT hr = JITManager::GetJITManager()->RemoteCodeGenCall( workItem->GetJITData(), scriptContext->GetThreadContext()->GetRemoteThreadContextAddr(), scriptContext->GetRemoteScriptAddr(), &jitWriteData); - switch (workItem->codeGenResult) - { - case S_OK: - break; - case E_ABORT: - throw Js::OperationAbortedException(); - case E_OUTOFMEMORY: - Js::Throw::OutOfMemory(); - case VBSERR_OutOfStack: - throw Js::StackOverflowException(); - case E_ACCESSDENIED: - // OOP JIT TODO: if server side can't handle request any more, use better error code and turn off JIT - throw Js::JITOperationFailedException(workItem->codeGenResult); - default: - Js::Throw::FatalInternalError(); - } + JITManager::HandleServerCallResult(hr); } else { @@ -2070,6 +2055,7 @@ NativeCodeGenerator::UpdateJITState() props.reclaimedPropertyIdArray = reclaimedPropArray; props.newRecordCount = newCount; props.newRecordArray = newPropArray; + HRESULT hr = JITManager::GetJITManager()->UpdatePropertyRecordMap(scriptContext->GetThreadContext()->GetRemoteThreadContextAddr(), &props); if (newPropArray) @@ -2081,11 +2067,7 @@ NativeCodeGenerator::UpdateJITState() HeapDeleteArray(reclaimedCount, reclaimedPropArray); } - if (hr != S_OK) - { - // OOP JIT TODO: use better exception when failed to update JIT server state - Js::Throw::OutOfMemory(); - } + JITManager::HandleServerCallResult(hr); } } } @@ -3189,6 +3171,7 @@ NativeCodeGenerator::FreeNativeCodeGenAllocation(void* address) ThreadContext * context = this->scriptContext->GetThreadContext(); if (JITManager::GetJITManager()->IsOOPJITEnabled()) { + // OOP JIT TODO: need error handling? JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)address); } else @@ -3221,6 +3204,7 @@ NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* 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; } @@ -3663,3 +3647,21 @@ bool NativeCodeGenerator::TryAggressiveInlining(Js::FunctionBody *const topFunct #endif return true; } + +void +JITManager::HandleServerCallResult(HRESULT hr) +{ + switch (hr) + { + case S_OK: + break; + case E_ABORT: + throw Js::OperationAbortedException(); + case E_OUTOFMEMORY: + Js::Throw::OutOfMemory(); + case VBSERR_OutOfStack: + throw Js::StackOverflowException(); + default: + Js::Throw::FatalInternalError(); + } +} \ No newline at end of file diff --git a/lib/JITClient/JITManager.h b/lib/JITClient/JITManager.h index b34b2702868..62d2df11833 100644 --- a/lib/JITClient/JITManager.h +++ b/lib/JITClient/JITManager.h @@ -81,6 +81,7 @@ class JITManager static JITManager * GetJITManager(); + static void HandleServerCallResult(HRESULT hr); private: JITManager(); ~JITManager(); diff --git a/lib/Runtime/Base/ThreadContext.cpp b/lib/Runtime/Base/ThreadContext.cpp index f7c6a5a36df..ba0663c74cb 100644 --- a/lib/Runtime/Base/ThreadContext.cpp +++ b/lib/Runtime/Base/ThreadContext.cpp @@ -2249,7 +2249,8 @@ void ThreadContext::SetWellKnownHostTypeId(WellKnownHostType wellKnownType, Js:: #if ENABLE_NATIVE_CODEGEN if (this->m_remoteThreadContextInfo != 0) { - JITManager::GetJITManager()->SetWellKnownHostTypeId(this->m_remoteThreadContextInfo, (int)typeId); + HRESULT hr = JITManager::GetJITManager()->SetWellKnownHostTypeId(this->m_remoteThreadContextInfo, (int)typeId); + JITManager::HandleServerCallResult(hr); } #endif } @@ -3874,11 +3875,9 @@ BOOL ThreadContext::IsNativeAddress(void * pCodeAddr) return false; } HRESULT hr = JITManager::GetJITManager()->IsNativeAddr(this->m_remoteThreadContextInfo, (intptr_t)pCodeAddr, &result); - if (FAILED(hr)) - { - // TODO: OOP JIT, what to do in failure case? - Js::Throw::FatalInternalError(); - } + + // TODO: OOP JIT, can we throw here? + JITManager::HandleServerCallResult(hr); return result; } else diff --git a/lib/Runtime/Language/SourceTextModuleRecord.cpp b/lib/Runtime/Language/SourceTextModuleRecord.cpp index a2fb72e1571..12ddc2088d4 100644 --- a/lib/Runtime/Language/SourceTextModuleRecord.cpp +++ b/lib/Runtime/Language/SourceTextModuleRecord.cpp @@ -845,10 +845,11 @@ namespace Js { scriptContext->InitializeRemoteScriptContext(); } - JITManager::GetJITManager()->AddModuleRecordInfo( + HRESULT hr = JITManager::GetJITManager()->AddModuleRecordInfo( scriptContext->GetRemoteScriptAddr(), this->GetModuleId(), (intptr_t)this->GetLocalExportSlots()); + JITManager::HandleServerCallResult(hr); } #endif } From 46e3e99e34d5efd06ed6115b6cab21ae63b889f9 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 13:33:09 -0700 Subject: [PATCH 06/13] address review comments --- lib/Backend/CodeGenNumberAllocator.cpp | 1 - lib/Backend/NativeCodeGenerator.cpp | 1 - lib/Common/Memory/MemUtils.cpp | 2 -- lib/JITClient/JITManager.cpp | 3 +- lib/JITClient/JITManager.h | 1 - lib/JITIDL/ChakraJIT.idl | 1 - lib/JITServer/JITServer.cpp | 49 +++++++++++--------------- 7 files changed, 21 insertions(+), 37 deletions(-) diff --git a/lib/Backend/CodeGenNumberAllocator.cpp b/lib/Backend/CodeGenNumberAllocator.cpp index d648480267b..74fbd6e2ff6 100644 --- a/lib/Backend/CodeGenNumberAllocator.cpp +++ b/lib/Backend/CodeGenNumberAllocator.cpp @@ -334,7 +334,6 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou || bytesWritten != sizeCat) { Js::Throw::CheckAndThrowJITOperationFailed(); - Output::Print(_u("FATAL ERROR: WriteProcessMemory failed, GLE: %d\n"), GetLastError()); Js::Throw::FatalInternalError(); // TODO: don't bring down whole server process, but pass the last error to main process } diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index 25242ce0eb0..b491858a0e7 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -896,7 +896,6 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor { HRESULT hr = JITManager::GetJITManager()->RemoteCodeGenCall( workItem->GetJITData(), - scriptContext->GetThreadContext()->GetRemoteThreadContextAddr(), scriptContext->GetRemoteScriptAddr(), &jitWriteData); JITManager::HandleServerCallResult(hr); diff --git a/lib/Common/Memory/MemUtils.cpp b/lib/Common/Memory/MemUtils.cpp index 0a7866f9fd8..56114262fd1 100644 --- a/lib/Common/Memory/MemUtils.cpp +++ b/lib/Common/Memory/MemUtils.cpp @@ -25,7 +25,6 @@ Memory::ChakraMemSet(_In_ void *dst, int val, size_t sizeInBytes, HANDLE process if (!WriteProcessMemory(processHandle, dst, writeBuffer, sizeInBytes, NULL)) { Js::Throw::CheckAndThrowJITOperationFailed(); - // if it's not E_ACCESSDENIED Js::Throw::FatalInternalError(); } HeapDeleteArray(sizeInBytes, writeBuffer); @@ -49,7 +48,6 @@ Memory::ChakraMemCopy(_In_ void *dst, size_t sizeInBytes, _In_reads_bytes_(count { if (!WriteProcessMemory(processHandle, dst, src, count, NULL)) { - Output::Print(_u("FATAL ERROR: WriteProcessMemory failed, GLE: %d\n"), GetLastError()); Js::Throw::CheckAndThrowJITOperationFailed(); Js::Throw::FatalInternalError(); } diff --git a/lib/JITClient/JITManager.cpp b/lib/JITClient/JITManager.cpp index a58d940a008..1e293eb8c00 100644 --- a/lib/JITClient/JITManager.cpp +++ b/lib/JITClient/JITManager.cpp @@ -548,7 +548,6 @@ JITManager::IsNativeAddr( HRESULT JITManager::RemoteCodeGenCall( __in CodeGenWorkItemIDL *workItemData, - __in intptr_t threadContextInfoAddress, __in intptr_t scriptContextInfoAddress, __out JITOutputIDL *jitData) { @@ -557,7 +556,7 @@ JITManager::RemoteCodeGenCall( HRESULT hr = E_FAIL; RpcTryExcept { - hr = ClientRemoteCodeGen(m_rpcBindingHandle, threadContextInfoAddress, scriptContextInfoAddress, workItemData, jitData); + hr = ClientRemoteCodeGen(m_rpcBindingHandle, scriptContextInfoAddress, workItemData, jitData); } RpcExcept(RpcExceptionFilter(RpcExceptionCode())) { diff --git a/lib/JITClient/JITManager.h b/lib/JITClient/JITManager.h index 62d2df11833..123e372b335 100644 --- a/lib/JITClient/JITManager.h +++ b/lib/JITClient/JITManager.h @@ -73,7 +73,6 @@ class JITManager HRESULT RemoteCodeGenCall( __in CodeGenWorkItemIDL *workItemData, - __in intptr_t threadContextInfoAddress, __in intptr_t scriptContextInfoAddress, __out JITOutputIDL *jitData); diff --git a/lib/JITIDL/ChakraJIT.idl b/lib/JITIDL/ChakraJIT.idl index 40c0fbd6ad6..08345b0c703 100644 --- a/lib/JITIDL/ChakraJIT.idl +++ b/lib/JITIDL/ChakraJIT.idl @@ -81,7 +81,6 @@ interface IChakraJIT HRESULT RemoteCodeGen( [in] handle_t binding, - [in] CHAKRA_PTR threadContextInfoAddress, [in] CHAKRA_PTR scriptContextInfoAddress, [in] CodeGenWorkItemIDL * workItemData, [out] JITOutputIDL * jitData); diff --git a/lib/JITServer/JITServer.cpp b/lib/JITServer/JITServer.cpp index 1e9a96b5274..9cfb06348a9 100644 --- a/lib/JITServer/JITServer.cpp +++ b/lib/JITServer/JITServer.cpp @@ -132,17 +132,21 @@ ServerInitializeThreadContext( AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); ServerThreadContext * contextInfo = HeapNewNoThrow(ServerThreadContext, threadContextData); - if (contextInfo == nullptr) + if (contextInfo == nullptr) { return E_OUTOFMEMORY; } - ServerContextManager::RegisterThreadContext(contextInfo); + AutoReleaseContext autoThreadContext(contextInfo); + return ServerCallWrapper(contextInfo, [&]()->HRESULT + { + ServerContextManager::RegisterThreadContext(contextInfo); - *threadContextRoot = (intptr_t)EncodePointer(contextInfo); - *prereservedRegionAddr = (intptr_t)contextInfo->GetPreReservedVirtualAllocator()->EnsurePreReservedRegion(); - - return S_OK; + *threadContextRoot = (intptr_t)EncodePointer(contextInfo); + *prereservedRegionAddr = (intptr_t)contextInfo->GetPreReservedVirtualAllocator()->EnsurePreReservedRegion(); + + return S_OK; + }); } HRESULT @@ -285,12 +289,9 @@ ServerSetWellKnownHostTypeId( } AutoReleaseContext autoThreadContext(threadContextInfo); - return ServerCallWrapper(threadContextInfo, [&]()->HRESULT - { - threadContextInfo->SetWellKnownHostTypeId((Js::TypeId)typeId); - return S_OK; - }); + threadContextInfo->SetWellKnownHostTypeId((Js::TypeId)typeId); + return S_OK; } HRESULT @@ -374,15 +375,13 @@ ServerCleanupScriptContext( return RPC_S_INVALID_ARG; } - if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + if (ServerContextManager::IsScriptContextAlive(scriptContextInfo)) { - return E_ACCESSDENIED; + AutoReleaseContext autoScriptContext(scriptContextInfo); + scriptContextInfo->Close(); + ServerContextManager::UnRegisterScriptContext(scriptContextInfo); } - AutoReleaseContext autoScriptContext(scriptContextInfo); - - scriptContextInfo->Close(); - ServerContextManager::UnRegisterScriptContext(scriptContextInfo); return S_OK; } @@ -486,14 +485,11 @@ ServerSetIsPRNGSeeded( HRESULT ServerRemoteCodeGen( /* [in] */ handle_t binding, - /* [in] */ intptr_t threadContextInfoAddress, /* [in] */ intptr_t scriptContextInfoAddress, /* [in] */ __RPC__in CodeGenWorkItemIDL *workItemData, /* [out] */ __RPC__out JITOutputIDL *jitData) { - UNREFERENCED_PARAMETER(binding); AUTO_NESTED_HANDLED_EXCEPTION_TYPE(static_cast(ExceptionType_OutOfMemory | ExceptionType_StackOverflow)); - LARGE_INTEGER start_time = { 0 }; if (PHASE_TRACE1(Js::BackEndPhase)) { @@ -501,29 +497,24 @@ ServerRemoteCodeGen( } memset(jitData, 0, sizeof(JITOutputIDL)); - ServerThreadContext * threadContextInfo = (ServerThreadContext*)DecodePointer((void*)threadContextInfoAddress); ServerScriptContext * scriptContextInfo = (ServerScriptContext*)DecodePointer((void*)scriptContextInfoAddress); - if (threadContextInfo == nullptr || scriptContextInfo == nullptr) + if (scriptContextInfo == nullptr) { return RPC_S_INVALID_ARG; } - if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) - { - return E_ACCESSDENIED; - } - if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) { return E_ACCESSDENIED; } - AutoReleaseContext autoThreadContext(threadContextInfo); AutoReleaseContext autoScriptContext(scriptContextInfo); - return ServerCallWrapper(threadContextInfo, [&]() ->HRESULT + return ServerCallWrapper(scriptContextInfo, [&]() ->HRESULT { + ServerThreadContext * threadContextInfo = scriptContextInfo->GetThreadContext(); + NoRecoverMemoryJitArenaAllocator jitArena(L"JITArena", threadContextInfo->GetPageAllocator(), Js::Throw::OutOfMemory); JITTimeWorkItem * jitWorkItem = Anew(&jitArena, JITTimeWorkItem, workItemData); From 7e0a2be86c723202a66a89a68fc8d52678efe251 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 14:37:28 -0700 Subject: [PATCH 07/13] fix some initialization --- lib/Backend/ServerScriptContext.cpp | 3 ++- lib/Backend/ServerThreadContext.cpp | 1 + lib/Runtime/Base/ThreadContextInfo.cpp | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Backend/ServerScriptContext.cpp b/lib/Backend/ServerScriptContext.cpp index f99a01097bc..9d7c73e2bb0 100644 --- a/lib/Backend/ServerScriptContext.cpp +++ b/lib/Backend/ServerScriptContext.cpp @@ -14,7 +14,8 @@ ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData, Ser #ifdef PROFILE_EXEC m_codeGenProfiler(nullptr), #endif - m_refCount(0) + m_refCount(0), + m_isClosed(false) { #ifdef PROFILE_EXEC if (Js::Configuration::Global.flags.IsEnabled(Js::ProfileFlag)) diff --git a/lib/Backend/ServerThreadContext.cpp b/lib/Backend/ServerThreadContext.cpp index 18dc4ee125b..a7ef25e1c87 100644 --- a/lib/Backend/ServerThreadContext.cpp +++ b/lib/Backend/ServerThreadContext.cpp @@ -7,6 +7,7 @@ ServerThreadContext::ServerThreadContext(ThreadContextDataIDL * data) : m_threadContextData(*data), + m_refCount(0), m_policyManager(true), m_propertyMap(nullptr), m_pageAllocs(&HeapAllocator::Instance), diff --git a/lib/Runtime/Base/ThreadContextInfo.cpp b/lib/Runtime/Base/ThreadContextInfo.cpp index ec46331afd9..165ed3487d9 100644 --- a/lib/Runtime/Base/ThreadContextInfo.cpp +++ b/lib/Runtime/Base/ThreadContextInfo.cpp @@ -12,7 +12,8 @@ ThreadContextInfo::ThreadContextInfo() : m_isAllJITCodeInPreReservedRegion(true), - wellKnownHostTypeHTMLAllCollectionTypeId(Js::TypeIds_Undefined) + wellKnownHostTypeHTMLAllCollectionTypeId(Js::TypeIds_Undefined), + m_isClosed(false) { } From fa80b60cbccb6e12a5777052cb11797e97f285f3 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 15:44:22 -0700 Subject: [PATCH 08/13] fix an issue that we should not be changing protection un-commited page fix linux build --- lib/Common/Memory/VirtualAllocWrapper.cpp | 11 +++++++---- lib/JITClient/JITManager.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/Common/Memory/VirtualAllocWrapper.cpp b/lib/Common/Memory/VirtualAllocWrapper.cpp index 2a6f9981553..2a8f13a92db 100644 --- a/lib/Common/Memory/VirtualAllocWrapper.cpp +++ b/lib/Common/Memory/VirtualAllocWrapper.cpp @@ -30,7 +30,7 @@ LPVOID VirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DWORD allocat #endif #if defined(_CONTROL_FLOW_GUARD) - DWORD oldProtectFlags; + DWORD oldProtectFlags = 0; if (AutoSystemInfo::Data.IsCFGEnabled() && isCustomHeapAllocation) { //We do the allocation in two steps - CFG Bitmap in kernel will be created only on allocation with EXECUTE flag. @@ -51,10 +51,13 @@ LPVOID VirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DWORD allocat Js::Throw::CheckAndThrowJITOperationFailed(); } - BOOL result = VirtualProtectEx(process, address, dwSize, protectFlags, &oldProtectFlags); - if (result == FALSE && process != GetCurrentProcess()) + if ((allocationType & MEM_COMMIT) == MEM_COMMIT) // The access protection value can be set only on committed pages. { - Js::Throw::CheckAndThrowJITOperationFailed(); + BOOL result = VirtualProtectEx(process, address, dwSize, protectFlags, &oldProtectFlags); + if (result == FALSE && process != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } } } else diff --git a/lib/JITClient/JITManager.h b/lib/JITClient/JITManager.h index 123e372b335..800e9c0adb3 100644 --- a/lib/JITClient/JITManager.h +++ b/lib/JITClient/JITManager.h @@ -184,7 +184,6 @@ class JITManager HRESULT RemoteCodeGenCall( __in CodeGenWorkItemIDL *workItemData, - __in intptr_t threadContextInfoAddress, __in intptr_t scriptContextInfoAddress, __out JITOutputIDL *jitData) { Assert(false); return E_FAIL; } @@ -194,6 +193,7 @@ class JITManager static JITManager * GetJITManager() { return &s_jitManager; } + static void HandleServerCallResult(HRESULT hr); private: static JITManager s_jitManager; From 48ead56189d8d7ed332630996992a532885df329 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Tue, 4 Oct 2016 16:25:40 -0700 Subject: [PATCH 09/13] fail fast early if the protection failed --- lib/Common/Exceptions/JITOperationFailedException.h | 1 + lib/Common/Memory/VirtualAllocWrapper.cpp | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/Common/Exceptions/JITOperationFailedException.h b/lib/Common/Exceptions/JITOperationFailedException.h index dfb872d57af..325c6450819 100644 --- a/lib/Common/Exceptions/JITOperationFailedException.h +++ b/lib/Common/Exceptions/JITOperationFailedException.h @@ -2,6 +2,7 @@ // Copyright (C) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- + #pragma once namespace Js { diff --git a/lib/Common/Memory/VirtualAllocWrapper.cpp b/lib/Common/Memory/VirtualAllocWrapper.cpp index 2a8f13a92db..44ef80ad45a 100644 --- a/lib/Common/Memory/VirtualAllocWrapper.cpp +++ b/lib/Common/Memory/VirtualAllocWrapper.cpp @@ -54,9 +54,16 @@ LPVOID VirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DWORD allocat if ((allocationType & MEM_COMMIT) == MEM_COMMIT) // The access protection value can be set only on committed pages. { BOOL result = VirtualProtectEx(process, address, dwSize, protectFlags, &oldProtectFlags); - if (result == FALSE && process != GetCurrentProcess()) + if (result == FALSE) { - Js::Throw::CheckAndThrowJITOperationFailed(); + if (process != GetCurrentProcess()) + { + Js::Throw::CheckAndThrowJITOperationFailed(); + } + else + { + CustomHeap_BadPageState_fatal_error((ULONG_PTR)this); + } } } } From 9f966a8944eb22d24e0eada51e16e121a649a248 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Wed, 5 Oct 2016 11:31:59 -0700 Subject: [PATCH 10/13] harden the contexts initialization path Assertion on server side for client calling with wrong args or at wrong state to help diagnose the issue --- lib/Backend/NativeCodeGenerator.cpp | 6 -- lib/Backend/ServerThreadContext.cpp | 2 + lib/Backend/ServerThreadContext.h | 2 + lib/JITServer/JITServer.cpp | 97 +++++++++++-------- lib/Runtime/Base/ScriptContext.cpp | 3 +- lib/Runtime/Base/ScriptContext.h | 9 +- lib/Runtime/Base/ThreadContext.cpp | 3 +- lib/Runtime/Base/ThreadContext.h | 3 +- .../Language/SourceTextModuleRecord.cpp | 4 - lib/Runtime/Library/JavascriptLibrary.cpp | 7 +- 10 files changed, 76 insertions(+), 60 deletions(-) diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index b491858a0e7..ecebff15c67 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -2007,12 +2007,6 @@ NativeCodeGenerator::UpdateJITState() { if (JITManager::GetJITManager()->IsOOPJITEnabled()) { - // ensure jit contexts have been set up - if (!scriptContext->GetRemoteScriptAddr()) - { - scriptContext->InitializeRemoteScriptContext(); - } - // update all property records on server that have been changed since last jit ThreadContext::PropertyMap * pendingProps = scriptContext->GetThreadContext()->GetPendingJITProperties(); PropertyRecordIDL ** newPropArray = nullptr; diff --git a/lib/Backend/ServerThreadContext.cpp b/lib/Backend/ServerThreadContext.cpp index a7ef25e1c87..336ffccac1a 100644 --- a/lib/Backend/ServerThreadContext.cpp +++ b/lib/Backend/ServerThreadContext.cpp @@ -22,6 +22,8 @@ ServerThreadContext::ServerThreadContext(ThreadContextDataIDL * data) : #endif m_jitCRTBaseAddress((intptr_t)GetModuleHandle(UCrtC99MathApis::LibraryName)) { + m_pid = GetProcessId((HANDLE)data->processHandle); + #if !_M_X64_OR_ARM64 && _CONTROL_FLOW_GUARD m_codeGenAlloc.canCreatePreReservedSegment = data->allowPrereserveAlloc != FALSE; #endif diff --git a/lib/Backend/ServerThreadContext.h b/lib/Backend/ServerThreadContext.h index 5829e2b295e..58f5a3afc32 100644 --- a/lib/Backend/ServerThreadContext.h +++ b/lib/Backend/ServerThreadContext.h @@ -65,6 +65,8 @@ class ServerThreadContext : public ThreadContextInfo CodeGenAllocators m_codeGenAlloc; ThreadContextDataIDL m_threadContextData; + + DWORD m_pid; //save client process id for easier diagnose intptr_t m_jitChakraBaseAddress; intptr_t m_jitCRTBaseAddress; diff --git a/lib/JITServer/JITServer.cpp b/lib/JITServer/JITServer.cpp index 9cfb06348a9..27e6583d4ce 100644 --- a/lib/JITServer/JITServer.cpp +++ b/lib/JITServer/JITServer.cpp @@ -159,19 +159,16 @@ ServerCleanupThreadContext( ServerThreadContext * threadContextInfo = (ServerThreadContext*)DecodePointer((void*)threadContextRoot); if (threadContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } - if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) + if (ServerContextManager::IsThreadContextAlive(threadContextInfo)) { - return E_ACCESSDENIED; + AutoReleaseContext autoThreadContext(threadContextInfo); + threadContextInfo->Close(); + ServerContextManager::UnRegisterThreadContext(threadContextInfo); } - - AutoReleaseContext autoThreadContext(threadContextInfo); - - threadContextInfo->Close(); - ServerContextManager::UnRegisterThreadContext(threadContextInfo); - return S_OK; } @@ -187,11 +184,13 @@ ServerUpdatePropertyRecordMap( if (threadContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { + Assert(false); return E_ACCESSDENIED; } @@ -225,12 +224,14 @@ ServerAddDOMFastPathHelper( if (scriptContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) { - return RPC_S_INVALID_ARG; + Assert(false); + return E_ACCESSDENIED; } AutoReleaseContext autoScriptContext(scriptContextInfo); @@ -253,12 +254,14 @@ ServerAddModuleRecordInfo( ServerScriptContext * serverScriptContext = (ServerScriptContext*)DecodePointer((void*)scriptContextInfoAddress); if (serverScriptContext == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsScriptContextAlive(serverScriptContext)) { - return RPC_S_INVALID_ARG; + Assert(false); + return E_ACCESSDENIED; } AutoReleaseContext autoScriptContext(serverScriptContext); @@ -280,11 +283,13 @@ ServerSetWellKnownHostTypeId( if (threadContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { + Assert(false); return E_ACCESSDENIED; } @@ -307,11 +312,13 @@ ServerInitializeScriptContext( if (threadContextInfo == nullptr) { - return E_ACCESSDENIED; + Assert(false); + return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { + Assert(false); return E_ACCESSDENIED; } @@ -340,26 +347,26 @@ ServerCloseScriptContext( if (scriptContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } - if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) + if (ServerContextManager::IsScriptContextAlive(scriptContextInfo)) { - return E_ACCESSDENIED; - } - - AutoReleaseContext autoScriptContext(scriptContextInfo); + AutoReleaseContext autoScriptContext(scriptContextInfo); #ifdef PROFILE_EXEC - auto profiler = scriptContextInfo->GetCodeGenProfiler(); - if (profiler && profiler->IsInitialized()) - { - profiler->ProfilePrint(Js::Configuration::Global.flags.Profile.GetFirstPhase()); - } + auto profiler = scriptContextInfo->GetCodeGenProfiler(); + if (profiler && profiler->IsInitialized()) + { + profiler->ProfilePrint(Js::Configuration::Global.flags.Profile.GetFirstPhase()); + } #endif - scriptContextInfo->Close(); - ServerContextManager::UnRegisterScriptContext(scriptContextInfo); + scriptContextInfo->Close(); + ServerContextManager::UnRegisterScriptContext(scriptContextInfo); + } + return S_OK; } @@ -372,6 +379,7 @@ ServerCleanupScriptContext( if (scriptContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } @@ -396,20 +404,18 @@ ServerFreeAllocation( if (context == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } - if (!ServerContextManager::IsThreadContextAlive(context)) - { - return E_ACCESSDENIED; - } - AutoReleaseContext autoThreadContext(context); - return ServerCallWrapper(context, [&]()->HRESULT + if (ServerContextManager::IsThreadContextAlive(context)) { + AutoReleaseContext autoThreadContext(context); context->SetValidCallTargetForCFG((PVOID)address, false); - bool succeeded = context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)address); - return succeeded ? S_OK : E_FAIL; - }); + context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)address); + } + + return S_OK; } HRESULT @@ -424,12 +430,14 @@ ServerIsNativeAddr( if (context == nullptr) { *result = false; + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsThreadContextAlive(context)) { *result = false; + Assert(false); return E_ACCESSDENIED; } @@ -465,21 +473,19 @@ ServerSetIsPRNGSeeded( if (scriptContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) { - return RPC_S_INVALID_ARG; + Assert(false); + return E_ACCESSDENIED; } - AutoReleaseContext autoScriptContext(scriptContextInfo); - - return ServerCallWrapper(scriptContextInfo, [&]()->HRESULT - { - scriptContextInfo->SetIsPRNGSeeded(value != FALSE); - return S_OK; - }); + AutoReleaseContext autoScriptContext(scriptContextInfo); + scriptContextInfo->SetIsPRNGSeeded(value != FALSE); + return S_OK; } HRESULT @@ -501,11 +507,13 @@ ServerRemoteCodeGen( if (scriptContextInfo == nullptr) { + Assert(false); return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsScriptContextAlive(scriptContextInfo)) { + Assert(false); return E_ACCESSDENIED; } @@ -686,7 +694,7 @@ HRESULT ServerCallWrapper(ServerThreadContext* threadContextInfo, Fn fn) // like VM is in a state of removing but not completed yet hr = HRESULT_FROM_WIN32(ex.LastError); - DWORD exitCode; + DWORD exitCode = STILL_ACTIVE; if (!GetExitCodeProcess(threadContextInfo->GetProcessHandle(), &exitCode)) { Assert(false); // fail to check target process @@ -696,6 +704,12 @@ HRESULT ServerCallWrapper(ServerThreadContext* threadContextInfo, Fn fn) { threadContextInfo->Close(); ServerContextManager::UnRegisterThreadContext(threadContextInfo); + return hr; + } + else + { + // The content process is still alive, the falure most likely an OOM + return E_OUTOFMEMORY; } } @@ -708,6 +722,7 @@ HRESULT ServerCallWrapper(ServerScriptContext* scriptContextInfo, Fn fn) ServerThreadContext* threadContextInfo = scriptContextInfo->GetThreadContext(); if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { + Assert(false); return E_ACCESSDENIED; } AutoReleaseContext autoThreadContext(threadContextInfo); diff --git a/lib/Runtime/Base/ScriptContext.cpp b/lib/Runtime/Base/ScriptContext.cpp index 913998ccff5..3b01ed1e84a 100644 --- a/lib/Runtime/Base/ScriptContext.cpp +++ b/lib/Runtime/Base/ScriptContext.cpp @@ -4508,7 +4508,8 @@ void ScriptContext::RegisterPrototypeChainEnsuredToHaveOnlyWritableDataPropertie #endif this->GetThreadContext()->EnsureJITThreadContext(allowPrereserveAlloc); - JITManager::GetJITManager()->InitializeScriptContext(&contextData, this->GetThreadContext()->GetRemoteThreadContextAddr(), &m_remoteScriptContextAddr); + HRESULT hr = JITManager::GetJITManager()->InitializeScriptContext(&contextData, this->GetThreadContext()->GetRemoteThreadContextAddr(), &m_remoteScriptContextAddr); + JITManager::HandleServerCallResult(hr); } #endif diff --git a/lib/Runtime/Base/ScriptContext.h b/lib/Runtime/Base/ScriptContext.h index f8f891e2055..60349ed6e3f 100644 --- a/lib/Runtime/Base/ScriptContext.h +++ b/lib/Runtime/Base/ScriptContext.h @@ -913,7 +913,14 @@ namespace Js void SetDirectHostTypeId(TypeId typeId) {directHostTypeId = typeId; } TypeId GetDirectHostTypeId() const { return directHostTypeId; } - intptr_t GetRemoteScriptAddr() { return m_remoteScriptContextAddr; } + intptr_t GetRemoteScriptAddr() + { + if (!m_remoteScriptContextAddr) + { + InitializeRemoteScriptContext(); + } + return m_remoteScriptContextAddr; + } char16 const * GetUrl() const { return url; } void SetUrl(BSTR bstr); diff --git a/lib/Runtime/Base/ThreadContext.cpp b/lib/Runtime/Base/ThreadContext.cpp index ba0663c74cb..d1c56b0f0a7 100644 --- a/lib/Runtime/Base/ThreadContext.cpp +++ b/lib/Runtime/Base/ThreadContext.cpp @@ -2008,7 +2008,8 @@ ThreadContext::EnsureJITThreadContext(bool allowPrereserveAlloc) m_reclaimedJITProperties = HeapNew(PropertyList, &HeapAllocator::Instance); m_pendingJITProperties = propertyMap->Clone(); - JITManager::GetJITManager()->InitializeThreadContext(&contextData, &m_remoteThreadContextInfo, &m_prereservedRegionAddr); + HRESULT hr = JITManager::GetJITManager()->InitializeThreadContext(&contextData, &m_remoteThreadContextInfo, &m_prereservedRegionAddr); + JITManager::HandleServerCallResult(hr); } #endif diff --git a/lib/Runtime/Base/ThreadContext.h b/lib/Runtime/Base/ThreadContext.h index 031c1a9b73e..d33fdb30336 100644 --- a/lib/Runtime/Base/ThreadContext.h +++ b/lib/Runtime/Base/ThreadContext.h @@ -554,8 +554,9 @@ class ThreadContext sealed : static void SetJITConnectionInfo(HANDLE processHandle, void* serverSecurityDescriptor, UUID connectionId); void EnsureJITThreadContext(bool allowPrereserveAlloc); - intptr_t GetRemoteThreadContextAddr() const + intptr_t GetRemoteThreadContextAddr() { + Assert(m_remoteThreadContextInfo); return m_remoteThreadContextInfo; } #endif diff --git a/lib/Runtime/Language/SourceTextModuleRecord.cpp b/lib/Runtime/Language/SourceTextModuleRecord.cpp index 12ddc2088d4..a01c9369e34 100644 --- a/lib/Runtime/Language/SourceTextModuleRecord.cpp +++ b/lib/Runtime/Language/SourceTextModuleRecord.cpp @@ -841,10 +841,6 @@ namespace Js #if ENABLE_NATIVE_CODEGEN if (JITManager::GetJITManager()->IsOOPJITEnabled()) { - if (!scriptContext->GetRemoteScriptAddr()) - { - scriptContext->InitializeRemoteScriptContext(); - } HRESULT hr = JITManager::GetJITManager()->AddModuleRecordInfo( scriptContext->GetRemoteScriptAddr(), this->GetModuleId(), diff --git a/lib/Runtime/Library/JavascriptLibrary.cpp b/lib/Runtime/Library/JavascriptLibrary.cpp index 8d1aa048723..f20e19b53ee 100644 --- a/lib/Runtime/Library/JavascriptLibrary.cpp +++ b/lib/Runtime/Library/JavascriptLibrary.cpp @@ -6588,11 +6588,8 @@ namespace Js #if ENABLE_NATIVE_CODEGEN if (JITManager::GetJITManager()->IsOOPJITEnabled()) { - if (!GetScriptContext()->GetRemoteScriptAddr()) - { - GetScriptContext()->InitializeRemoteScriptContext(); - } - JITManager::GetJITManager()->SetIsPRNGSeeded(GetScriptContext()->GetRemoteScriptAddr(), val); + HRESULT hr = JITManager::GetJITManager()->SetIsPRNGSeeded(GetScriptContext()->GetRemoteScriptAddr(), val); + JITManager::HandleServerCallResult(hr); } #endif } From da70259f684e447a7d8b4456ec809491772dd123 Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Wed, 5 Oct 2016 12:11:01 -0700 Subject: [PATCH 11/13] fix after rebase should check exception while freeing code address fix prefast warning and linux build --- lib/Backend/ServerThreadContext.cpp | 2 ++ lib/JITServer/JITServer.cpp | 15 +++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/Backend/ServerThreadContext.cpp b/lib/Backend/ServerThreadContext.cpp index 336ffccac1a..79e790de642 100644 --- a/lib/Backend/ServerThreadContext.cpp +++ b/lib/Backend/ServerThreadContext.cpp @@ -22,7 +22,9 @@ ServerThreadContext::ServerThreadContext(ThreadContextDataIDL * data) : #endif m_jitCRTBaseAddress((intptr_t)GetModuleHandle(UCrtC99MathApis::LibraryName)) { +#if ENABLE_OOP_NATIVE_CODEGEN m_pid = GetProcessId((HANDLE)data->processHandle); +#endif #if !_M_X64_OR_ARM64 && _CONTROL_FLOW_GUARD m_codeGenAlloc.canCreatePreReservedSegment = data->allowPrereserveAlloc != FALSE; diff --git a/lib/JITServer/JITServer.cpp b/lib/JITServer/JITServer.cpp index 27e6583d4ce..54db21dedcc 100644 --- a/lib/JITServer/JITServer.cpp +++ b/lib/JITServer/JITServer.cpp @@ -313,12 +313,14 @@ ServerInitializeScriptContext( if (threadContextInfo == nullptr) { Assert(false); + *scriptContextInfoAddress = 0; return RPC_S_INVALID_ARG; } if (!ServerContextManager::IsThreadContextAlive(threadContextInfo)) { Assert(false); + *scriptContextInfoAddress = 0; return E_ACCESSDENIED; } @@ -408,14 +410,19 @@ ServerFreeAllocation( return RPC_S_INVALID_ARG; } - if (ServerContextManager::IsThreadContextAlive(context)) + if (!ServerContextManager::IsThreadContextAlive(context)) + { + Assert(false); + return E_ACCESSDENIED; + } + + return ServerCallWrapper(context, [&]()->HRESULT { AutoReleaseContext autoThreadContext(context); context->SetValidCallTargetForCFG((PVOID)address, false); context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)address); - } - - return S_OK; + return S_OK; + }); } HRESULT From 7420b3c0b623b7340253a1e2e3e9070a6cd46fdb Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Wed, 5 Oct 2016 12:17:37 -0700 Subject: [PATCH 12/13] fix osx build --- lib/Runtime/Base/ScriptContext.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Runtime/Base/ScriptContext.h b/lib/Runtime/Base/ScriptContext.h index 60349ed6e3f..888d3450a4d 100644 --- a/lib/Runtime/Base/ScriptContext.h +++ b/lib/Runtime/Base/ScriptContext.h @@ -915,10 +915,12 @@ namespace Js intptr_t GetRemoteScriptAddr() { +#if ENABLE_OOP_NATIVE_CODEGEN if (!m_remoteScriptContextAddr) { InitializeRemoteScriptContext(); } +#endif return m_remoteScriptContextAddr; } From a1963bc1a9836ff3a6184380c67d324261f0784d Mon Sep 17 00:00:00 2001 From: Lei Shi Date: Wed, 5 Oct 2016 13:04:25 -0700 Subject: [PATCH 13/13] fix initialization --- lib/Backend/NativeCodeGenerator.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index ecebff15c67..d99f2beacb8 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -2007,6 +2007,12 @@ NativeCodeGenerator::UpdateJITState() { if (JITManager::GetJITManager()->IsOOPJITEnabled()) { + // TODO: OOP JIT, move server calls to background thread to reduce foreground thread delay + if (!scriptContext->GetRemoteScriptAddr()) + { + return; + } + // update all property records on server that have been changed since last jit ThreadContext::PropertyMap * pendingProps = scriptContext->GetThreadContext()->GetPendingJITProperties(); PropertyRecordIDL ** newPropArray = nullptr;