JSRunLoopTimer should use WTF::RunLoop rather than custom CF code
Created attachment 403813 [details] Patch
Created attachment 403879 [details] Patch
We have testing for this now in https://bugs.webkit.org/show_bug.cgi?id=214193.
Comment on attachment 403879 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403879&action=review > Source/JavaScriptCore/runtime/JSRunLoopTimer.h:73 > + PerVMData(Manager&, RefPtr<WTF::RunLoop>); Seems like this argument should be WTF::RunLoop&. > Source/JavaScriptCore/runtime/JSRunLoopTimer.h:76 > + RefPtr<WTF::RunLoop> runLoop; Can this be Ref instead of RefPtr? > Source/JavaScriptCore/runtime/VM.h:357 > + RefPtr<WTF::RunLoop> m_runLoop; Can this be Ref instead of RefPtr? > Source/JavaScriptCore/runtime/VM.h:1080 > + WTF::RunLoop* runLoop() const { return m_runLoop.get(); } Can this return a reference rather than a pointer? It can never be null, right?
*** Bug 214193 has been marked as a duplicate of this bug. ***
Created attachment 403997 [details] Patch
Comment on attachment 403997 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403997&action=review > Source/JavaScriptCore/runtime/VM.h:1080 > + WTF::RunLoop& runLoop() const { return m_runLoop.get(); } Don’t actually need get() here. > Source/WebCore/bindings/js/CommonVM.cpp:61 > + RunLoop* runLoop = &RunLoop::current(); Seems like this could just be nullptr since current is the default.
Created attachment 404000 [details] Patch for landing
ChangeLog entry in Source/JavaScriptCore/ChangeLog contains OOPS!.
Committed r264242: <https://trac.webkit.org/changeset/264242> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404000 [details].
<rdar://problem/65366297>
Reverted r264242 for reason: Caused many crashes on iOS bots Committed r264262: <https://trac.webkit.org/changeset/264262>
(In reply to Chris Dumez from comment #12) > Reverted r264242 for reason: > > Caused many crashes on iOS bots > > Committed r264262: <https://trac.webkit.org/changeset/264262> https://build.webkit.org/results/Apple%20iOS%2013%20Simulator%20Debug%20WK2%20(Tests)/r264243%20(4626)/results.html ASSERTION FAILED: s_webRunLoop /Volumes/Data/slave/ios-simulator-13-debug/build/Source/WTF/wtf/RunLoop.cpp(82) : static WTF::RunLoop &WTF::RunLoop::web() 1 0x6c5525c49 WTFCrash 2 0x6c66c77db WTFCrashWithInfo(int, char const*, char const*, int) 3 0x6c55c9d4d WTF::RunLoop::web() 4 0x6cce5c048 WebCore::commonVMSlow() 5 0x6cb204fa3 WebCore::commonVM() 6 0x6cddc8724 WebCore::PageScriptDebugServer::PageScriptDebugServer(WebCore::Page&) 7 0x6cddc877d WebCore::PageScriptDebugServer::PageScriptDebugServer(WebCore::Page&) 8 0x6cdd8adf4 WebCore::InspectorController::InspectorController(WebCore::Page&, WebCore::InspectorClient*) 9 0x6cdd8b3d5 WebCore::InspectorController::InspectorController(WebCore::Page&, WebCore::InspectorClient*) 10 0x6ce33ce32 std::__1::__unique_if<WebCore::InspectorController>::__unique_single std::__1::make_unique<WebCore::InspectorController, WebCore::Page&, WebCore::InspectorClient*&>(WebCore::Page&, WebCore::InspectorClient*&) 11 0x6ce323bb4 decltype(auto) WTF::makeUnique<WebCore::InspectorController, WebCore::Page&, WebCore::InspectorClient*&>(WebCore::Page&, WebCore::InspectorClient*&) 12 0x6ce322836 WebCore::Page::Page(WebCore::PageConfiguration&&) 13 0x6ce32554d WebCore::Page::Page(WebCore::PageConfiguration&&) 14 0x6b194c88d std::__1::__unique_if<WebCore::Page>::__unique_single std::__1::make_unique<WebCore::Page, WebCore::PageConfiguration>(WebCore::PageConfiguration&&) 15 0x6b192351f decltype(auto) WTF::makeUnique<WebCore::Page, WebCore::PageConfiguration>(WebCore::PageConfiguration&&) 16 0x6b1920474 WebKit::WebPage::WebPage(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&) 17 0x6b191eb75 WebKit::WebPage::WebPage(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&) 18 0x6b191ea81 WebKit::WebPage::create(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&) 19 0x6b144bf84 WebKit::WebProcess::createWebPage(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&) 20 0x6b1c1787c void IPC::callMemberFunctionImpl<WebKit::WebProcess, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>, 0ul, 1ul>(WebKit::WebProcess*, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) 21 0x6b1c162e0 void IPC::callMemberFunction<WebKit::WebProcess, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>&&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&)) 22 0x6b1c0fe0e void IPC::handleMessage<Messages::WebProcess::CreateWebPage, WebKit::WebProcess, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&)>(IPC::Decoder&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&)) 23 0x6b1c0de0c WebKit::WebProcess::didReceiveWebProcessMessage(IPC::Connection&, IPC::Decoder&) 24 0x6b144c996 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 25 0x6b011b89f IPC::Connection::dispatchMessage(IPC::Decoder&) 26 0x6b011c1d0 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) 27 0x6b011c860 IPC::Connection::dispatchOneIncomingMessage() 28 0x6b013b618 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7::operator()() 29 0x6b013b52e WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7, void>::call() 30 0x6c5551a42 WTF::Function<void ()>::operator()() const 31 0x6c55c9f55 WTF::RunLoop::performWork()
Created attachment 404045 [details] Patch for landing - fixed iOS WK2 crash
Created attachment 404055 [details] Patch for landing - fixed iOS WK2 crash
Comment on attachment 404055 [details] Patch for landing - fixed iOS WK2 crash View in context: https://bugs.webkit.org/attachment.cgi?id=404055&action=review > Source/WebCore/bindings/js/CommonVM.cpp:61 > + if (RunLoop* web = RunLoop::webIfExists()) > + runLoop = web; Is it OK that an early-created VM will permanently be on the current run loop, not the web run loop?
Comment on attachment 404055 [details] Patch for landing - fixed iOS WK2 crash View in context: https://bugs.webkit.org/attachment.cgi?id=404055&action=review > Source/WebCore/bindings/js/CommonVM.cpp:60 > + if (RunLoop* web = RunLoop::webIfExists()) No need for this if statement. Could just be: runLoop = RunLoop::webIfExists(); Almost makes me wish RunLoop::webIfExists was a platform-independent function that returns nullptr on non-web-thread platforms.
> Is it OK that an early-created VM will permanently be on the current run > loop, not the web run loop? It's a bit of a brittle design, but I believe that was the previous behavior of "vm.setRunLoop(WebThreadRunLoop())".
I wonder if there's any way to get a crash log from the api-ios bot. 🤔
(In reply to Geoffrey Garen from comment #19) > I wonder if there's any way to get a crash log from the api-ios bot. 🤔 I emailed you the crash logs. There isn't any automated way currently. I or anyone from our team can manually get the logs from the bots.
Created attachment 404163 [details] Patch for landing - fixed iOS WK2 crash
Committed r264315: <https://trac.webkit.org/changeset/264315> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404163 [details].
Reopening to attach new patch.
Created attachment 404897 [details] Patch for landing - fixed WASM crash
Committed r264696: <https://trac.webkit.org/changeset/264696> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404897 [details].