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 <cvienn...@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@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev
> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
