| Summary: | Add assertions to help diagnose crash at WebProcessProxy::processPool() | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sihui Liu <sihui_liu> | ||||||||||
| Component: | New Bugs | Assignee: | Sihui Liu <sihui_liu> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | cdumez, commit-queue, ews-watchlist, sroberts, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=197925 | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 369795 [details]
Patch
Comment on attachment 369795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369795&action=review > Source/WebKit/UIProcess/WebProcessCache.cpp:249 > + RELEASE_ASSERT(!m_process->pageCount()); Also: RELEASE_ASSERT(!m_process->websiteDataStore().processes().contains(process.ptr()); > Source/WebKit/UIProcess/WebProcessLifetimeObserver.cpp:46 > + RELEASE_ASSERT(!process.isInProcessCache() && !process.isPrewarmed()); Please use 2 separate assertions so that we know which condition is true if it hits. > Source/WebKit/UIProcess/WebProcessLifetimeObserver.cpp:49 > + RELEASE_LOG(Loading, "WebProcessLifetimeObserver::addWebPage: webPID = %i, pageID = %" PRIu64, process.processIdentifier(), webPageProxy.pageID()); I think you need to print |this| pointer too. > Source/WebKit/UIProcess/WebProcessLifetimeObserver.cpp:65 > + RELEASE_LOG(Loading, "WebProcessLifetimeObserver::removeWebPage: webPID = %i, pageID = %" PRIu64, process.processIdentifier(), webPageProxy.pageID()); I think you need to print |this| pointer too. Created attachment 369890 [details]
Patch
(In reply to Chris Dumez from comment #3) > Comment on attachment 369795 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=369795&action=review > > > Source/WebKit/UIProcess/WebProcessCache.cpp:249 > > + RELEASE_ASSERT(!m_process->pageCount()); > > Also: > RELEASE_ASSERT(!m_process->websiteDataStore().processes().contains(process. > ptr()); Okay, processes() does not have contains. Adding a helper function. > > > Source/WebKit/UIProcess/WebProcessLifetimeObserver.cpp:46 > > + RELEASE_ASSERT(!process.isInProcessCache() && !process.isPrewarmed()); > > Please use 2 separate assertions so that we know which condition is true if > it hits. Sure. > > > Source/WebKit/UIProcess/WebProcessLifetimeObserver.cpp:49 > > + RELEASE_LOG(Loading, "WebProcessLifetimeObserver::addWebPage: webPID = %i, pageID = %" PRIu64, process.processIdentifier(), webPageProxy.pageID()); > > I think you need to print |this| pointer too. Okay. > > > Source/WebKit/UIProcess/WebProcessLifetimeObserver.cpp:65 > > + RELEASE_LOG(Loading, "WebProcessLifetimeObserver::removeWebPage: webPID = %i, pageID = %" PRIu64, process.processIdentifier(), webPageProxy.pageID()); > > I think you need to print |this| pointer too. Okay. Comment on attachment 369890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369890&action=review > Source/WebKit/UIProcess/WebProcessCache.cpp:250 > + RELEASE_ASSERT(!m_process->websiteDataStore().hasProcess(process.ptr())); RELEASE_ASSERT_WITH_MESSAGE(!m_process->websiteDataStore().hasProcess(process.ptr()), "Only processes with pages should be registered with the data store"); Comment on attachment 369890 [details] Patch Attachment 369890 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12191661 New failing tests: js/dom/custom-constructors.html Created attachment 369900 [details]
Archive of layout-test-results from ews210 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Created attachment 369980 [details]
Patch for landing
Comment on attachment 369980 [details] Patch for landing Clearing flags on attachment: 369980 Committed r245339: <https://trac.webkit.org/changeset/245339> All reviewed patches have been landed. Closing bug. After changes in https://trac.webkit.org/changeset/245339 , seeing 79 API test crashes on several queues. https://build.webkit.org/builders/Apple%20Mojave%20Release%20WK1%20%28Tests%29/builds/4988/steps/run-api-tests/logs/stdio run-api-tests TestWebKitAPI.ProcessSwap.NavigateBackAfterClientSideRedirect --debug Hits the following Assertion with r245339, passes in prior builds. ASSERTION FAILED: m_ptr /Volumes/Data/worker/liberty-debug-archive/build/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/Ref.h(120) : T *WTF::Ref<WebKit::WebProcessProxy, WTF::DumbPtrTraits<WebKit::WebProcessProxy> >::ptr() const [T = WebKit::WebProcessProxy, PtrTraits = WTF::DumbPtrTraits<WebKit::WebProcessProxy>] 1 0x1133c4429 WTFCrash 2 0x118e0ebcb WTFCrashWithInfo(int, char const*, char const*, int) 3 0x1196b53e8 WTF::Ref<WebKit::WebProcessProxy, WTF::DumbPtrTraits<WebKit::WebProcessProxy> >::ptr() const 4 0x1199219f1 WebKit::WebProcessCache::CachedProcess::CachedProcess(WTF::Ref<WebKit::WebProcessProxy, WTF::DumbPtrTraits<WebKit::WebProcessProxy> >&&) Comment on attachment 369980 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=369980&action=review > Source/WebKit/UIProcess/WebProcessCache.cpp:250 > + RELEASE_ASSERT_WITH_MESSAGE(!m_process->websiteDataStore().hasProcess(process.ptr()), "Only processes with pages should be registered with the data store"); use-after-move here. Should be m_process.ptr(), not process.ptr(). Sorry I missed that. Fixed assertion in <https://trac.webkit.org/changeset/245343>. |
-> 119 WebProcessPool& processPool() const { ASSERT(m_processPool); return *m_processPool.get(); } Thread 0 Crashed ↩:: Dispatch queue: com.apple.main-thread 0 WebKit::WebsiteDataStore::processPools(unsigned long, bool) const + 114 1 WebKit::WebsiteDataStore::plugins() const + 53 2 WebKit::WebsiteDataStore::fetchDataAndApply(WTF::OptionSet<WebKit::WebsiteDataType>, WTF::OptionSet<WebKit::WebsiteDataFetchOption>, WTF::RefPtr<WTF::WorkQueue, WTF::DumbPtrTraits<WTF::WorkQueue> >&&, WTF::Function<void (WTF::Vector<WebKit::WebsiteDataRecord, 0ul, WTF::CrashOnOverflow, 16ul>)>&&) + 2024 3 WebKit::WebsiteDataStore::fetchDataForRegistrableDomains(WTF::OptionSet<WebKit::WebsiteDataType>, WTF::OptionSet<WebKit::WebsiteDataFetchOption>, WTF::Vector<WebCore::RegistrableDomain, 0ul, WTF::CrashOnOverflow, 16ul> const&, WTF::CompletionHandler<void (WTF::Vector<WebKit::WebsiteDataRecord, 0ul, WTF::CrashOnOverflow, 16ul>&&, WTF::HashSet<WebCore::RegistrableDomain, WebCore::RegistrableDomain::RegistrableDomainHash, WTF::HashTraits<WebCore::RegistrableDomain> >&&)>&&) + 167 4 WebKit::NetworkProcessProxy::deleteWebsiteDataInUIProcessForRegistrableDomains(PAL::SessionID, WTF::OptionSet<WebKit::WebsiteDataType>, WTF::OptionSet<WebKit::WebsiteDataFetchOption>, WTF::Vector<WebCore::RegistrableDomain, 0ul, WTF::CrashOnOverflow, 16ul>, WTF::CompletionHandler<void (WTF::HashSet<WebCore::RegistrableDomain, WebCore::RegistrableDomain::RegistrableDomainHash, WTF::HashTraits<WebCore::RegistrableDomain> >&&)>&&) + 130 5 WebKit::NetworkProcessProxy::didReceiveNetworkProcessProxyMessage(IPC::Connection&, IPC::Decoder&) + 9056 It seems m_processPool is nullptr in the crash trace above.