chakra-core · chakrabot · Oct 5, 2016 · Oct 4, 2016 · Oct 4, 2016 · Oct 4, 2016
diff --git a/lib/Backend/CodeGenNumberAllocator.cpp b/lib/Backend/CodeGenNumberAllocator.cpp
@@ -333,7 +333,7 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou
             if (!WriteProcessMemory(hProcess, (void*)number, pLocalNumber, sizeCat, &bytesWritten)
                 || bytesWritten != sizeCat)
             {
-                Output::Print(_u("FATAL ERROR: WriteProcessMemory failed, GLE: %d\n"), GetLastError());
+                Js::Throw::CheckAndThrowJITOperationFailed();
                 Js::Throw::FatalInternalError(); // TODO: don't bring down whole server process, but pass the last error to main process
             }
 
@@ -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/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
@@ -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/EmitBuffer.cpp b/lib/Backend/EmitBuffer.cpp
@@ -290,6 +290,13 @@ EmitBufferAllocation* EmitBufferManager<SyncObject>::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
@@ -894,24 +894,11 @@ 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();
-        default:
-            Js::Throw::FatalInternalError();
-        }
+        JITManager::HandleServerCallResult(hr);
     }
     else
     {
@@ -2020,23 +2007,11 @@ NativeCodeGenerator::UpdateJITState()
 {
     if (JITManager::GetJITManager()->IsOOPJITEnabled())
     {
-        // ensure jit contexts have been set up
+        // TODO: OOP JIT, move server calls to background thread to reduce foreground thread delay
         if (!scriptContext->GetRemoteScriptAddr())
         {
-            scriptContext->InitializeRemoteScriptContext();
-        }
-
-        bool allowPrereserveAlloc = true;
-#if !_M_X64_OR_ARM64
-        if (this->scriptContext->webWorkerId != Js::Constants::NonWebWorkerContextId)
-        {
-            allowPrereserveAlloc = false;
+            return;
         }
-#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();
@@ -2079,11 +2054,9 @@ NativeCodeGenerator::UpdateJITState()
             props.reclaimedPropertyIdArray = reclaimedPropArray;
             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 +2065,8 @@ NativeCodeGenerator::UpdateJITState()
             {
                 HeapDeleteArray(reclaimedCount, reclaimedPropArray);
             }
+
+            JITManager::HandleServerCallResult(hr);
         }
     }
 }
@@ -3195,6 +3170,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
@@ -3227,6 +3203,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;
         }
@@ -3669,3 +3646,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();
+    }
+}
diff --git a/lib/Backend/ServerScriptContext.cpp b/lib/Backend/ServerScriptContext.cpp
@@ -5,16 +5,17 @@
 
 #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),
+    m_isClosed(false)
 {
 #ifdef PROFILE_EXEC
     if (Js::Configuration::Global.flags.IsEnabled(Js::ProfileFlag))
@@ -279,21 +280,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 +327,4 @@ ServerScriptContext::GetCodeGenProfiler() const
     return nullptr;
 #endif
 }
+
diff --git 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,19 +62,22 @@ 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
     Js::ScriptContextProfiler * m_codeGenProfiler;
 #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
@@ -7,6 +7,7 @@
 
 ServerThreadContext::ServerThreadContext(ThreadContextDataIDL * data) :
     m_threadContextData(*data),
+    m_refCount(0),
     m_policyManager(true),
     m_propertyMap(nullptr),
     m_pageAllocs(&HeapAllocator::Instance),
@@ -21,6 +22,10 @@ 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;
 #endif
@@ -260,3 +265,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
@@ -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;
@@ -61,7 +66,10 @@ class ServerThreadContext : public ThreadContextInfo
 
     ThreadContextDataIDL m_threadContextData;
 
-    ThreadContext * m_threadContext;
+    DWORD m_pid; //save client process id for easier diagnose
+
     intptr_t m_jitChakraBaseAddress;
     intptr_t m_jitCRTBaseAddress;
+    uint m_refCount;
+
 };
diff --git a/lib/Common/Common.h b/lib/Common/Common.h
@@ -83,6 +83,7 @@ template<> struct IntMath<int64> { 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
@@ -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
@@ -47,6 +47,7 @@
     <ClInclude Include="ExceptionCheck.h" />
     <ClInclude Include="InScriptExceptionBase.h" />
     <ClInclude Include="InternalErrorException.h" />
+    <ClInclude Include="JITOperationFailedException.h" />
     <ClInclude Include="NotImplementedException.h" />
     <ClInclude Include="OperationAbortedException.h" />
     <ClInclude Include="OutOfMemoryException.h" />
@@ -58,4 +59,4 @@
   </ItemGroup>
   <Import Project="$(BuildConfigPropsPath)Chakra.Build.targets" Condition="exists('$(BuildConfigPropsPath)Chakra.Build.targets')" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
-</Project>
+</Project>