Hello,
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.
Thank you
Chris Vienneau
Current Code:
bool JSLock::currentThreadIsHoldingLock()
{
ASSERT(!m_hasExclusiveThread || (exclusiveThread() ==
std::this_thread::get_id()));
if (m_hasExclusiveThread)
return !!m_lockCount;
return m_ownerThreadID == std::this_thread::get_id();
}
I found the logic missing some things that I wanted really clear so I chose to
re-implement it.
Suggested currentThreadIsHoldingLock
I took a look at what happens in 15.x and came up with this version of the
function:
bool JSLock::currentThreadIsHoldingLock()
{
ASSERT(!m_hasExclusiveThread || (exclusiveThread() ==
std::this_thread::get_id()));
//+EAWebKitChange
//07/07/2016 - this logic is more strait forward and fixes an assert in
DestroyJavascriptValue after EvaluateJavaScript
if (m_hasExclusiveThread && exclusiveThread() == std::this_thread::get_id())
{
return true;
}
if (m_lockCount && m_ownerThreadID == std::this_thread::get_id())
{
return true;
}
return false;
//-EAWebKitChange
}
Additional Required Fix
The above was successful in clearing up the assert however another change was
needed to make things run properly for all tests and soaks:
void JSLock::lock(intptr_t lockCount)
{
ASSERT(lockCount > 0);
//+EAWebKitChange
//07/07/2016 - added && (m_lockCount > 0) to condition
//currentThreadIsHoldingLock doesn't really return if it is holding the
lock, but more so if it should have the lock
//this change allows us to take the lock the first time
if (currentThreadIsHoldingLock() && (m_lockCount > 0)) {
//-EAWebKitChange
m_lockCount += lockCount;
return;
}
if (!m_hasExclusiveThread) {
m_lock.lock();
m_ownerThreadID = std::this_thread::get_id();
}
ASSERT(!m_lockCount);
m_lockCount = lockCount;
didAcquireLock();
}
Test Code:
void JavascriptArrays(void*)
{
EA_TRACE_MESSAGE("Running JavascriptArrays");
JavascriptArrayBoundObject jsb;
jsb.mEAWK = mEAWK;
jsb.mView = mView;
using namespace EA::WebKit;
mView->BindJavaScriptObject("TestObject", &jsb);
eastl::string16 script(EA_CHAR16("TestObject.ArrayTest([1, true,
\"ping\"]);"));
JavascriptValue *returnValue = mEAWK->CreateJavascriptValue(mView);
mView->EvaluateJavaScript(script.data(), returnValue);
mEAWK->DestroyJavascriptValue(returnValue);
}
Assert location:
\JavaScriptCore\heap\Heap.cpp (471)
bool Heap::unprotect(JSValue k)
{
ASSERT(k);
> ASSERT(m_vm->currentThreadIsHoldingAPILock());
…
Callstack:
EAWebKitDemoUTFWin.exe!EA::Browser::BrowserClient::DebugLog(EA::WebKit::DebugLogInfo
& dli={...}) Line 254 C++
EAWebKitd.dll!EA::WebKit::DebugLogCallback(const
eastl::basic_string<char,eastl::allocator> & origMessage={...}, bool
shouldAssert=true) Line 611 C++
EAWebKitd.dll!EA::WebKit::DebugLogCallbackInternal(bool
shouldAssert=true, const char * format=0x000007feb4b15170, char *
args=0x00000000001af000) Line 629 C++
EAWebKitd.dll!vprintf_stderr_common(bool shouldAssert=true,
const char * format=0x000007feb4b15170, char * args=0x00000000001af000) Line
153 C++
EAWebKitd.dll!printf_stderr_common(bool shouldAssert=true, const
char * format=0x000007feb4b15170, ...) Line 237 C++
EAWebKitd.dll!printCallSite(const char *
file=0x000007feb4d04240, int line=471, const char *
function=0x000007feb4d04158, bool shouldAssert=true) Line 259 C++
EAWebKitd.dll!WTFReportAssertionFailure(const char *
file=0x000007feb4d04240, int line=471, const char *
function=0x000007feb4d04158, const char * assertion=0x000007feb4d03fe8) Line
281 C++
EAWebKitd.dll!JSC::Heap::unprotect(JSC::JSValue k={...}) Line
471 C++
EAWebKitd.dll!JSC::gcUnprotect(JSC::JSCell *
val=0x000007fffe453dd0) Line 38 C++
EAWebKitd.dll!JSC::gcUnprotect(JSC::JSValue value={...}) Line 62
C++
EAWebKitd.dll!EA::WebKit::JavascriptValue::Destruct() Line 495
C++
EAWebKitd.dll!EA::WebKit::JavascriptValue::~JavascriptValue()
Line 318 C++
EAWebKitd.dll!EA::WebKit::JavascriptValue::`vector deleting
destructor'(unsigned int) C++
EAWebKitd.dll!EA::WebKit::DestroyJavascriptValue(EA::WebKit::JavascriptValue *
v=0x000007fffe4b7a70) Line 1035 C++
EAWebKitd.dll!EA::WebKit::EAWebKitLib::DestroyJavascriptValue(EA::WebKit::JavascriptValue
* v=0x000007fffe4b7a70) Line 400 C++
>
> EAWebKitDemoUTFWin.exe!EAWebkitViewFunctionalTest::JavascriptArrays(void *
> __formal=0x00000000001afae0) Line 192 C++
EAWebKitDemoUTFWin.exe!EATest::Test_mfun<EAWebkitViewFunctionalTest>::TestDelegate<EAWebkitViewFunctionalTest>::Invoke(EAWebkitViewFunctionalTest
* instance=0x00000001415f2130, void * p=0x00000000001afae0) Line 72
C++
EAWebKitDemoUTFWin.exe!EATest::Test_mfun<EAWebkitViewFunctionalTest>::operator()(void
* context=0x00000000001afae0) Line 107 C++
EAWebKitDemoUTFWin.exe!EATest::TestCase::operator()(void *
context=0x00000000001afae0) Line 76 C++
EAWebKitDemoUTFWin.exe!EATest::TestSuite::ExecuteTestCase(EATest::TestCase *
tc=0x00000000002c4239, void * context=0x00000000001afae0) Line 520 C++
EAWebKitDemoUTFWin.exe!EATest::TestSuite::RunTests(const char *
suitename=0x0000000000000000, const char * testname=0x0000000000000000, void *
context=0x00000000001afae0) Line 304 C++
EAWebKitDemoUTFWin.exe!EATest::TestSuite::RunAllTests(void *
context=0x00000000001afae0) Line 147 C++
EAWebKitDemoUTFWin.exe!EATest::DoTests(int argc=1, char * *
argv=0x000007fffefb83a0, void * context=0x00000000001afae0) Line 307 C++
EAWebKitDemoUTFWin.exe!EA::Browser::RunUnitTests(EA::App::AppCommandLine &
commandLine={...}) Line 2284 C++
EAWebKitDemoUTFWin.exe!EAMain(EA::App::AppCommandLine &
commandLine={...}) Line 1123 C++
EAWebKitDemoUTFWin.exe!WinMain(HINSTANCE__ *
__formal=0x000000013fe80000, HINSTANCE__ * __formal=0x0000000000000000, char *
cmdLine=0x00000000002167c2, int __formal=10) Line 79 C++
EAWebKitDemoUTFWin.exe!invoke_main() Line 109 C++
EAWebKitDemoUTFWin.exe!__scrt_common_main_seh() Line 264
C++
EAWebKitDemoUTFWin.exe!__scrt_common_main() Line 309 C++
EAWebKitDemoUTFWin.exe!WinMainCRTStartup() Line 17 C++
kernel32.dll!BaseThreadInitThunk‑() Unknown
ntdll.dll!RtlUserThreadStart‑() Unknown
_______________________________________________
webkit-dev mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-dev
