<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">Hello,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I’ve recently noticed that our version of WebKit based off of 188436 asserts when running the scenario below, but our older version based off of 157437 does not. After doing some investigation I found that the implementation of JSLock
changed significantly between the two versions. The tip of trunk version is pretty much what we have in 188436 as well; the code for currentThreadIsHoldingLock() I find questionable. Please feel free to suggest how we might improperly be using the system
to get in the assert scenario, or discuss the merit of the changes I suggest.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thank you<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Chris Vienneau<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>Current Code:<o:p></o:p></b></p>
<p class="MsoNormal">bool JSLock::currentThreadIsHoldingLock()<o:p></o:p></p>
<p class="MsoNormal">{<o:p></o:p></p>
<p class="MsoNormal"> ASSERT(!m_hasExclusiveThread || (exclusiveThread() == std::this_thread::get_id()));<o:p></o:p></p>
<p class="MsoNormal"> if (m_hasExclusiveThread)<o:p></o:p></p>
<p class="MsoNormal"> return !!m_lockCount;<o:p></o:p></p>
<p class="MsoNormal"> return m_ownerThreadID == std::this_thread::get_id();<o:p></o:p></p>
<p class="MsoNormal">}<o:p></o:p></p>
<p class="MsoNormal">I found the logic missing some things that I wanted really clear so I chose to re-implement it.
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>Suggested currentThreadIsHoldingLock<o:p></o:p></b></p>
<p class="MsoNormal">I took a look at what happens in 15.x and came up with this version of the function:<o:p></o:p></p>
<p class="MsoNormal">bool JSLock::currentThreadIsHoldingLock()<o:p></o:p></p>
<p class="MsoNormal">{<o:p></o:p></p>
<p class="MsoNormal"> ASSERT(!m_hasExclusiveThread || (exclusiveThread() == std::this_thread::get_id()));<o:p></o:p></p>
<p class="MsoNormal"> //+EAWebKitChange<o:p></o:p></p>
<p class="MsoNormal"> //07/07/2016 - this logic is more strait forward and fixes an assert in DestroyJavascriptValue after EvaluateJavaScript
<o:p></o:p></p>
<p class="MsoNormal"> if (m_hasExclusiveThread && exclusiveThread() == std::this_thread::get_id())<o:p></o:p></p>
<p class="MsoNormal"> {<o:p></o:p></p>
<p class="MsoNormal"> return true;<o:p></o:p></p>
<p class="MsoNormal"> } <o:p></o:p></p>
<p class="MsoNormal"> if (m_lockCount && m_ownerThreadID == std::this_thread::get_id())<o:p></o:p></p>
<p class="MsoNormal"> {<o:p></o:p></p>
<p class="MsoNormal"> return true;<o:p></o:p></p>
<p class="MsoNormal"> } <o:p></o:p></p>
<p class="MsoNormal"> return false;<o:p></o:p></p>
<p class="MsoNormal"> //-EAWebKitChange<o:p></o:p></p>
<p class="MsoNormal">}<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>Additional Required Fix<o:p></o:p></b></p>
<p class="MsoNormal">The above was successful in clearing up the assert however another change was needed to make things run properly for all tests and soaks:<o:p></o:p></p>
<p class="MsoNormal">void JSLock::lock(intptr_t lockCount)<o:p></o:p></p>
<p class="MsoNormal">{<o:p></o:p></p>
<p class="MsoNormal"> ASSERT(lockCount > 0);<o:p></o:p></p>
<p class="MsoNormal"> //+EAWebKitChange<o:p></o:p></p>
<p class="MsoNormal"> //07/07/2016 - added && (m_lockCount > 0) to condition<o:p></o:p></p>
<p class="MsoNormal"> //currentThreadIsHoldingLock doesn't really return if it is holding the lock, but more so if it should have the lock<o:p></o:p></p>
<p class="MsoNormal"> //this change allows us to take the lock the first time<o:p></o:p></p>
<p class="MsoNormal"> if (currentThreadIsHoldingLock() && (m_lockCount > 0)) {<o:p></o:p></p>
<p class="MsoNormal"> //-EAWebKitChange<o:p></o:p></p>
<p class="MsoNormal"> m_lockCount += lockCount;<o:p></o:p></p>
<p class="MsoNormal"> return;<o:p></o:p></p>
<p class="MsoNormal"> }<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"> if (!m_hasExclusiveThread) {<o:p></o:p></p>
<p class="MsoNormal"> m_lock.lock();<o:p></o:p></p>
<p class="MsoNormal"> m_ownerThreadID = std::this_thread::get_id();<o:p></o:p></p>
<p class="MsoNormal"> }<o:p></o:p></p>
<p class="MsoNormal"> ASSERT(!m_lockCount);<o:p></o:p></p>
<p class="MsoNormal"> m_lockCount = lockCount;<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"> didAcquireLock();<o:p></o:p></p>
<p class="MsoNormal">}<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>Test Code:<o:p></o:p></b></p>
<p class="MsoNormal"> void JavascriptArrays(void*)<o:p></o:p></p>
<p class="MsoNormal"> {<o:p></o:p></p>
<p class="MsoNormal"> EA_TRACE_MESSAGE("Running JavascriptArrays");<o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal"> JavascriptArrayBoundObject jsb;<o:p></o:p></p>
<p class="MsoNormal"> jsb.mEAWK = mEAWK;<o:p></o:p></p>
<p class="MsoNormal"> jsb.mView = mView;<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"> using namespace EA::WebKit;<o:p></o:p></p>
<p class="MsoNormal"> mView->BindJavaScriptObject("TestObject", &jsb);<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"> eastl::string16 script(EA_CHAR16("TestObject.ArrayTest([1, true, \"ping\"]);"));<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"> JavascriptValue *returnValue = mEAWK->CreateJavascriptValue(mView);<o:p></o:p></p>
<p class="MsoNormal"> mView->EvaluateJavaScript(script.data(), returnValue);<o:p></o:p></p>
<p class="MsoNormal"> mEAWK->DestroyJavascriptValue(returnValue);<o:p></o:p></p>
<p class="MsoNormal"> }<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>Assert location:<o:p></o:p></b></p>
<p class="MsoNormal">\JavaScriptCore\heap\Heap.cpp (471)<o:p></o:p></p>
<p class="MsoNormal">bool Heap::unprotect(JSValue k)<o:p></o:p></p>
<p class="MsoNormal">{<o:p></o:p></p>
<p class="MsoNormal"> ASSERT(k);<o:p></o:p></p>
<p class="MsoNormal">> ASSERT(m_vm->currentThreadIsHoldingAPILock());<o:p></o:p></p>
<p class="MsoNormal">…<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>Callstack:<o:p></o:p></b></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EA::Browser::BrowserClient::DebugLog(EA::WebKit::DebugLogInfo & dli={...}) Line 254 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::DebugLogCallback(const eastl::basic_string<char,eastl::allocator> & origMessage={...}, bool shouldAssert=true) Line 611 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::DebugLogCallbackInternal(bool shouldAssert=true, const char * format=0x000007feb4b15170, char * args=0x00000000001af000) Line 629 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!vprintf_stderr_common(bool shouldAssert=true, const char * format=0x000007feb4b15170, char * args=0x00000000001af000) Line 153 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!printf_stderr_common(bool shouldAssert=true, const char * format=0x000007feb4b15170, ...) Line 237 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!printCallSite(const char * file=0x000007feb4d04240, int line=471, const char * function=0x000007feb4d04158, bool shouldAssert=true) Line 259 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!WTFReportAssertionFailure(const char * file=0x000007feb4d04240, int line=471, const char * function=0x000007feb4d04158, const char * assertion=0x000007feb4d03fe8) Line 281 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!JSC::Heap::unprotect(JSC::JSValue k={...}) Line 471 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!JSC::gcUnprotect(JSC::JSCell * val=0x000007fffe453dd0) Line 38 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!JSC::gcUnprotect(JSC::JSValue value={...}) Line 62 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::JavascriptValue::Destruct() Line 495 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::JavascriptValue::~JavascriptValue() Line 318 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::JavascriptValue::`vector deleting destructor'(unsigned int) C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::DestroyJavascriptValue(EA::WebKit::JavascriptValue * v=0x000007fffe4b7a70) Line 1035 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitd.dll!EA::WebKit::EAWebKitLib::DestroyJavascriptValue(EA::WebKit::JavascriptValue * v=0x000007fffe4b7a70) Line 400 C++<o:p></o:p></p>
<p class="MsoNormal">> EAWebKitDemoUTFWin.exe!EAWebkitViewFunctionalTest::JavascriptArrays(void * __formal=0x00000000001afae0) Line 192 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::Test_mfun<EAWebkitViewFunctionalTest>::TestDelegate<EAWebkitViewFunctionalTest>::Invoke(EAWebkitViewFunctionalTest * instance=0x00000001415f2130, void * p=0x00000000001afae0) Line 72
C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::Test_mfun<EAWebkitViewFunctionalTest>::operator()(void * context=0x00000000001afae0) Line 107 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::TestCase::operator()(void * context=0x00000000001afae0) Line 76 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::TestSuite::ExecuteTestCase(EATest::TestCase * tc=0x00000000002c4239, void * context=0x00000000001afae0) Line 520 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::TestSuite::RunTests(const char * suitename=0x0000000000000000, const char * testname=0x0000000000000000, void * context=0x00000000001afae0) Line 304 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::TestSuite::RunAllTests(void * context=0x00000000001afae0) Line 147 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EATest::DoTests(int argc=1, char * * argv=0x000007fffefb83a0, void * context=0x00000000001afae0) Line 307 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EA::Browser::RunUnitTests(EA::App::AppCommandLine & commandLine={...}) Line 2284 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!EAMain(EA::App::AppCommandLine & commandLine={...}) Line 1123 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!WinMain(HINSTANCE__ * __formal=0x000000013fe80000, HINSTANCE__ * __formal=0x0000000000000000, char * cmdLine=0x00000000002167c2, int __formal=10) Line 79 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!invoke_main() Line 109 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!__scrt_common_main_seh() Line 264 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!__scrt_common_main() Line 309 C++<o:p></o:p></p>
<p class="MsoNormal"> EAWebKitDemoUTFWin.exe!WinMainCRTStartup() Line 17 C++<o:p></o:p></p>
<p class="MsoNormal"> kernel32.dll!BaseThreadInitThunk‑() Unknown<o:p></o:p></p>
<p class="MsoNormal"> ntdll.dll!RtlUserThreadStart‑() Unknown<o:p></o:p></p>
</div>
</body>
</html>