[webkit-dev] currentThreadIsHoldingLock Heap::unprotect Assertion
Alex Christensen
achristensen at apple.com
Fri Jul 8 12:15:23 PDT 2016
Please file a bug at bugs.webkit.org, uploading a suggested patch with a test case, and cc fpizlo. We can discuss it in bugzilla instead of this mailing list.
> On Jul 8, 2016, at 11:53 AM, Vienneau, Christopher <cvienneau at ea.com> wrote:
>
> 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
> webkit-dev at lists.webkit.org <mailto:webkit-dev at lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev <https://lists.webkit.org/mailman/listinfo/webkit-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20160708/8850a40d/attachment.html>
More information about the webkit-dev
mailing list