| Summary: | [PSON] ASSERTION FAILED: m_uncommittedState.state == State::Committed | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
| Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, beidson, commit-queue, ggaren, koivisto, rniwa, ross.kirsling, ryanhaddad, tsavell, 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=193740 | ||||||||
| Bug Depends on: | 191795, 191797 | ||||||||
| Bug Blocks: | 191761 | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Dumez
2018-11-16 15:20:26 PST
I managed to make the assertion a bit more likely to reproduce like so:
diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
index ff3fd087426..7f741e2b199 100644
--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
@@ -1969,6 +1969,10 @@ window.addEventListener('load', function(event) {
TEST(ProcessSwap, LoadUnload)
{
+ for (int i=0; i < 30; ++i) {
+ @autoreleasepool {
+ receivedMessages = adoptNS([@[] mutableCopy]);
+
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
[processPoolConfiguration setProcessSwapsOnNavigation:YES];
auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
@@ -2024,6 +2028,8 @@ TEST(ProcessSwap, LoadUnload)
EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html - load", receivedMessages.get()[4]);
EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html - unload", receivedMessages.get()[5]);
EXPECT_WK_STREQ(@"pson://www.apple.com/main.html - load", receivedMessages.get()[6]);
+ }
+ }
}
When it asserts the state is Finished instead of Committed.
(In reply to Chris Dumez from comment #1) > I managed to make the assertion a bit more likely to reproduce like so: > diff --git > a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm > b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm > index ff3fd087426..7f741e2b199 100644 > --- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm > +++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm > @@ -1969,6 +1969,10 @@ window.addEventListener('load', function(event) { > > TEST(ProcessSwap, LoadUnload) > { > + for (int i=0; i < 30; ++i) { > + @autoreleasepool { > + receivedMessages = adoptNS([@[] mutableCopy]); > + > auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration > alloc] init]); > [processPoolConfiguration setProcessSwapsOnNavigation:YES]; > auto processPool = adoptNS([[WKProcessPool alloc] > _initWithConfiguration:processPoolConfiguration.get()]); > @@ -2024,6 +2028,8 @@ TEST(ProcessSwap, LoadUnload) > EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html - load", > receivedMessages.get()[4]); > EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html - unload", > receivedMessages.get()[5]); > EXPECT_WK_STREQ(@"pson://www.apple.com/main.html - load", > receivedMessages.get()[6]); > + } > + } > } > > When it asserts the state is Finished instead of Committed. So it means that when didFinishLoad() is called, one of the following has already been called: - didFinishLoad() - reset() - didFailProvisionalLoad() - didFailLoad() I have also seen this variant:
ASSERTION FAILED: m_uncommittedState.state == State::Provisional
/Volumes/Data/WebKit/OpenSource/Source/WebKit/UIProcess/PageLoadState.cpp(285) : void WebKit::PageLoadState::didCommitLoad(const Transaction::Token &, WebKit::WebCertificateInfo &, bool)
_RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
1 0x107fa80d9 WTFCrash
2 0x10cddf2fb WTFCrashWithInfo(int, char const*, char const*, int)
3 0x10d4abcc8 WebKit::PageLoadState::didCommitLoad(WebKit::PageLoadState::Transaction::Token const&, WebKit::WebCertificateInfo&, bool)
4 0x10d68a7b6 WebKit::WebPageProxy::didCommitLoadForFrame(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&)
5 0x10dfad143 void IPC::callMemberFunctionImpl<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&), std::__1::tuple<unsigned long long, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul>(WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&), std::__1::tuple<unsigned long long, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul>)
6 0x10dfabc60 void IPC::callMemberFunction<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&), std::__1::tuple<unsigned long long, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul> >(std::__1::tuple<unsigned long long, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData>&&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&))
7 0x10df8a3e5 void IPC::handleMessage<Messages::WebPageProxy::DidCommitLoadForFrame, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&)>(IPC::Decoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(unsigned long long, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, std::optional<WebCore::HasInsecureContent>, WebKit::UserData const&))
8 0x10df7fd16 WebKit::WebPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
9 0x10ce8ec0a IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
10 0x10d46b2e4 WebKit::ChildProcessProxy::dispatchMessage(IPC::Connection&, IPC::Decoder&)
11 0x10d76341a WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
12 0x10ce267cc IPC::Connection::dispatchMessage(IPC::Decoder&)
13 0x10ce18bb1 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
14 0x10ce23c21 IPC::Connection::dispatchIncomingMessages()
15 0x10ce480b2 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()()
16 0x10ce47fd9 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call()
17 0x107fcf87d WTF::Function<void ()>::operator()() const
18 0x108028413 WTF::RunLoop::performWork()
19 0x108028da4 WTF::RunLoop::performWork(void*)
20 0x7fff4ed19195 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
21 0x7fff4ed1913b __CFRunLoopDoSource0
22 0x7fff4ecfcbd5 __CFRunLoopDoSources0
23 0x7fff4ecfc17e __CFRunLoopRun
24 0x7fff4ecfba68 CFRunLoopRunSpecific
25 0x7fff511246ba -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
26 0x1071d8736 TestWebKitAPI::Util::run(bool*)
27 0x107045ab7 ProcessSwap_LoadUnload_Test::TestBody()
28 0x10739fcfe void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
29 0x1073477eb void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
30 0x107347716 testing::Test::Run()
31 0x1073493b2 testing::TestInfo::Run()
LEAK: 1 WebPage
I wonder if we're trying to load a new page while we're still pre-warming a process? e.g. right after we've committed a page load. In happens in tests that use back/forward navigations, I suspect this is caused by: 1. Load origin A in P1 2. Load origin B in P2 (Process Swap) -> Causes P1 to get navigated to about:blank for suspension 3. Navigate back before P1 has finished navigating to about:blank. P1 tells the WebPageProxy didFinishLoad or didCommitLoad for 'about:blank', which it does not expect since it did not request that load. Created attachment 355183 [details]
WIP Patch
Proof of concept.
Created attachment 355197 [details]
Patch
Comment on attachment 355197 [details] Patch Clearing flags on attachment: 355197 Committed r238353: <https://trac.webkit.org/changeset/238353> All reviewed patches have been landed. Closing bug. WinCairo (the bots for which all suddenly disconnected this morning and haven't come back online 😰) has a broken build with the following error:
> FAILED: Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource25.cpp.obj
> ...
> C:\GitHub\neko\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Ref.h(60): error C2039: 'deref': is not a member of 'WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>'
> C:\GitHub\neko\Source\WebKit\UIProcess/WebPageProxy.cpp(2723): note: see declaration of 'WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>'
> C:\GitHub\neko\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Ref.h(54): note: while compiling class template member function 'WTF::Ref<WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>,WTF::DumbPtrTraits<T>>::~Ref(void)'
> with
> [
> T=WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>
> ]
> C:\GitHub\neko\Source\WebKit\UIProcess/WebPageProxy.cpp(2712): note: see reference to function template instantiation 'WTF::Ref<WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>,WTF::DumbPtrTraits<T>>::~Ref(void)' being compiled
> with
> [
> T=WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>
> ]
> C:\GitHub\neko\Source\WebKit\UIProcess/WebPageProxy.cpp(2700): note: see reference to class template instantiation 'WTF::Ref<WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>,WTF::DumbPtrTraits<T>>' being compiled
> with
> [
> T=WebKit::WebPageProxy::continueNavigationInNewProcess::<lambda_a497dfd3aa776334b389d665d3f26b45>
> ]
> c:\github\neko\webkitbuild\release\derivedsources\forwardingheaders\javascriptcore\CodeBlock.h(950): note: see reference to class template instantiation 'WTF::Poisoned<JSC::CodeBlockPoison,JSC::VM *,void>' being compiled
That looks like some kind of a compiler bug... I guess someone who has access to VS.net has to debug that. (In reply to Ryosuke Niwa from comment #12) > That looks like some kind of a compiler bug... I guess someone who has > access to VS.net has to debug that. Thankfully, it looks like this issue has precedent. https://trac.webkit.org/changeset/233195 https://trac.webkit.org/changeset/230162 Committed r238358: <https://trac.webkit.org/changeset/238358> |