Skip to content

handle race condition in OOP JIT #1728

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chakrabot merged 4 commits into from
Oct 13, 2016
Merged

handle race condition in OOP JIT #1728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb076e6
fix issue with race condition
leirocks Oct 12, 2016
851ed0b
fix prefast warning
leirocks Oct 12, 2016
c976823
address code review comment
leirocks Oct 12, 2016
0477af7
add caller for CleanUpForProcess
leirocks Oct 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 20 additions & 18 deletions lib/Backend/ServerScriptContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
//-------------------------------------------------------------------------------------------------------

#include "Backend.h"
#if ENABLE_OOP_NATIVE_CODEGEN
#include "JITServer/JITServer.h"
#endif

ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData, ServerThreadContext* threadContextInfo) :
m_contextData(*contextData),
Expand All @@ -29,6 +32,18 @@ ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData, Ser

ServerScriptContext::~ServerScriptContext()
{
HeapDelete(m_domFastPathHelperMap);
m_moduleRecords.Map([](uint, Js::ServerSourceTextModuleRecord* record)
{
HeapDelete(record);
});

#ifdef PROFILE_EXEC
if (m_codeGenProfiler)
{
HeapDelete(m_codeGenProfiler);
}
#endif
}

intptr_t
Expand Down Expand Up @@ -274,8 +289,9 @@ ServerScriptContext::Close()
{
Assert(!IsClosed());
m_isClosed = true;

#ifdef STACK_BACK_TRACE
closingStack = StackBackTrace::Capture(&NoThrowHeapAllocator::Instance);
ServerContextManager::RecordCloseContext(this);
#endif
}

Expand All @@ -291,23 +307,9 @@ ServerScriptContext::Release()
InterlockedExchangeSubtract(&m_refCount, 1u);
if (m_isClosed && m_refCount == 0)
{
HeapDelete(m_domFastPathHelperMap);
m_moduleRecords.Map([](uint, Js::ServerSourceTextModuleRecord* record)
{
HeapDelete(record);
});

#ifdef PROFILE_EXEC
if (m_codeGenProfiler)
{
HeapDelete(m_codeGenProfiler);
}
#endif

// OOP JIT TODO: fix leak in chk build after the issue that script context closed prematurely is identified
#ifndef STACK_BACK_TRACE
HeapDelete(this);
#endif
// Not freeing here, we'll expect explicit ServerCleanupScriptContext() call to do the free
// otherwise after free, the CodeGen call can still get same scriptContext if there's another
// ServerInitializeScriptContext call
}
}

Expand Down
3 changes: 0 additions & 3 deletions lib/Backend/ServerScriptContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,4 @@ class ServerScriptContext : public ScriptContextInfo

bool m_isPRNGSeeded;
bool m_isClosed;
#ifdef STACK_BACK_TRACE
StackBackTrace* closingStack;
#endif
};
6 changes: 6 additions & 0 deletions lib/Backend/ServerThreadContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
//-------------------------------------------------------------------------------------------------------

#include "Backend.h"
#if ENABLE_OOP_NATIVE_CODEGEN
#include "JITServer/JITServer.h"
#endif

ServerThreadContext::ServerThreadContext(ThreadContextDataIDL * data) :
m_threadContextData(*data),
Expand Down Expand Up @@ -281,4 +284,7 @@ void ServerThreadContext::Release()
void ServerThreadContext::Close()
{
this->m_isClosed = true;
#ifdef STACK_BACK_TRACE
ServerContextManager::RecordCloseContext(this);
#endif
}
6 changes: 5 additions & 1 deletion lib/Backend/ServerThreadContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ class ServerThreadContext : public ThreadContextInfo

void AddRef();
void Release();
void Close();
void Close();

#ifdef STACK_BACK_TRACE
DWORD GetRuntimePid() { return m_pid; }
Copy link
Contributor

@curtisman curtisman Oct 12, 2016 •
edited by leirocks
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#ifdef STACK_BACK_TRACE? #Resolved

#endif

private:
intptr_t GetRuntimeChakraBaseAddress() const;
Expand Down
34 changes: 21 additions & 13 deletions lib/Common/Memory/MemUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,33 @@
void
Memory::ChakraMemSet(_In_ void *dst, int val, size_t sizeInBytes, HANDLE processHandle)
{
const bool isLocalProc = processHandle == GetCurrentProcess();
byte * writeBuffer;

if (isLocalProc)
byte* dest = (byte*)dst;
if (processHandle == GetCurrentProcess())
{
writeBuffer = (byte*)dst;
memset(dest, val, sizeInBytes);
}
else
{
writeBuffer = HeapNewArray(byte, sizeInBytes);
}
memset(writeBuffer, val, sizeInBytes);
if (!isLocalProc)
{
if (!WriteProcessMemory(processHandle, dst, writeBuffer, sizeInBytes, NULL))
const size_t bufferSize = 0x1000;
byte writeBuffer[bufferSize];
memset(writeBuffer, val, bufferSize < sizeInBytes ? bufferSize : sizeInBytes);

for (size_t i = 0; i < sizeInBytes / bufferSize; i++)
{
if (!WriteProcessMemory(processHandle, dest, writeBuffer, bufferSize, NULL))
{
MemoryOperationLastError::CheckProcessAndThrowFatalError(processHandle);
}
dest += bufferSize;
}

if (sizeInBytes % bufferSize > 0)
{
MemoryOperationLastError::CheckProcessAndThrowFatalError(processHandle);
if (!WriteProcessMemory(processHandle, dest, writeBuffer, sizeInBytes%bufferSize, NULL))
{
MemoryOperationLastError::CheckProcessAndThrowFatalError(processHandle);
}
}
HeapDeleteArray(sizeInBytes, writeBuffer);
}
}

Expand Down
Loading