[Webkit-unassigned] [Bug 159579] New: currentThreadIsHoldingLock Heap::unprotect Assertion
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jul 8 13:23:06 PDT 2016
https://bugs.webkit.org/show_bug.cgi?id=159579
Bug ID: 159579
Summary: currentThreadIsHoldingLock Heap::unprotect Assertion
Classification: Unclassified
Product: WebKit
Version: WebKit Local Build
Hardware: PC
OS: Windows 7
Status: NEW
Severity: Normal
Priority: P2
Component: JavaScriptCore
Assignee: webkit-unassigned at lists.webkit.org
Reporter: chris.vno at outlook.com
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
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160708/0e714f14/attachment-0001.html>
More information about the webkit-unassigned
mailing list