Bug 53211 – Need a way to spin a run loop while waiting for a reply to a synchronous CoreIPC message

• Component: WebKit2
• Status: RESOLVED DUPLICATE

[Adam Roben (:aroben)]

In order to fix bug 51352, we'll need a way to spin a message loop while waiting for a reply to a synchronous CoreIPC message.

[Anders Carlsson]

<rdar://problem/8922494>

[Adam Roben (:aroben)]

Created attachment 80650 [details]
WIP patch for discussion (depends on bug 53209)

With this patch and the patch for bug 53209, we spin a run loop while waiting for synchronous message replies. However, we seem to hit assertions/crashes pretty easily with this patch due to unexpected reentrancy in WebCore.

[Adam Roben (:aroben)]

Comment on attachment 80650 [details]
WIP patch for discussion (depends on bug 53209)

View in context: https://bugs.webkit.org/attachment.cgi?id=80650&action=review

> Source/WebKit2/Platform/CoreIPC/Connection.h:261
> +    bool m_isWaitingForSyncReply;
> +    bool m_shouldDispatchMessagesAfterReceivingSyncReply;

These members were added to try to fix an assertion I was hitting in Connection::processIncomingMessage:

        ASSERT(pendingSyncReply.syncRequestID == arguments->destinationID());

However, the assertion still seems to happen in some cases (e.g., when loading cuteoverload.com).

[Adam Roben (:aroben)]

(In reply to comment #3)
> (From update of attachment 80650 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80650&action=review
> 
> > Source/WebKit2/Platform/CoreIPC/Connection.h:261
> > +    bool m_isWaitingForSyncReply;
> > +    bool m_shouldDispatchMessagesAfterReceivingSyncReply;
> 
> These members were added to try to fix an assertion I was hitting in Connection::processIncomingMessage:
> 
>         ASSERT(pendingSyncReply.syncRequestID == arguments->destinationID());
> 
> However, the assertion still seems to happen in some cases (e.g., when loading cuteoverload.com).

Here's the backtrace on the main thread when that assertion fails on the Connection's WorkQueue. I guess we need to defer loading or something?


 	ntdll.dll!_ZwWaitForMultipleObjects@20()  + 0x15 bytes	
 	ntdll.dll!_ZwWaitForMultipleObjects@20()  + 0x15 bytes	
 	kernel32.dll!_WaitForMultipleObjectsExImplementation@20()  + 0x8e bytes	
 	kernel32.dll!_WaitForMultipleObjectsExImplementation@20()  + 0x8e bytes	
 	user32.dll!_RealMsgWaitForMultipleObjectsEx@20()  + 0xe2 bytes	
>	WebKit.dll!RunLoop::runUntil(const CoreIPC::BinarySemaphore * semaphore=0x00630c0c, double absoluteTime=11296496739.844786)  Line 85 + 0x32 bytes	C++
 	WebKit.dll!CoreIPC::Connection::waitForSyncReply(unsigned __int64 syncRequestID=16, double timeout=10000000000.000000)  Line 269 + 0x17 bytes	C++
 	WebKit.dll!CoreIPC::Connection::sendSyncMessage(CoreIPC::MessageID messageID={...}, unsigned __int64 syncRequestID=16, WTF::PassOwnPtr<CoreIPC::ArgumentEncoder> encoder={...}, double timeout=10000000000.000000)  Line 203 + 0x1d bytes	C++
 	WebKit.dll!CoreIPC::Connection::sendSync<Messages::WebPageProxy::DecidePolicyForMIMEType>(const Messages::WebPageProxy::DecidePolicyForMIMEType & message={...}, const CoreIPC::Arguments3<bool &,unsigned __int64 &,unsigned __int64 &> & reply={...}, unsigned __int64 destinationID=1, double timeout=10000000000.000000)  Line 323 + 0x36 bytes	C++
 	WebKit.dll!CoreIPC::MessageSender<WebKit::WebPage>::sendSync<Messages::WebPageProxy::DecidePolicyForMIMEType>(const Messages::WebPageProxy::DecidePolicyForMIMEType & message={...}, const CoreIPC::Arguments3<bool &,unsigned __int64 &,unsigned __int64 &> & reply={...}, unsigned __int64 destinationID=1, double timeout=10000000000.000000)  Line 68	C++
 	WebKit.dll!CoreIPC::MessageSender<WebKit::WebPage>::sendSync<Messages::WebPageProxy::DecidePolicyForMIMEType>(const Messages::WebPageProxy::DecidePolicyForMIMEType & message={...}, const CoreIPC::Arguments3<bool &,unsigned __int64 &,unsigned __int64 &> & reply={...}, double timeout=10000000000.000000)  Line 60	C++
 	WebKit.dll!WebKit::WebFrameLoaderClient::dispatchDecidePolicyForMIMEType(void (WebCore::PolicyAction)* function=0x5b886730, const WTF::String & MIMEType={...}, const WebCore::ResourceRequest & request={...})  Line 627 + 0x91 bytes	C++
 	WebKit.dll!WebCore::PolicyChecker::checkContentPolicy(const WTF::String & MIMEType={...}, void (void *, WebCore::PolicyAction)* function=0x5b9e14f0, void * argument=0x0407fb08)  Line 104 + 0x48 bytes	C++
 	WebKit.dll!WebCore::MainResourceLoader::didReceiveResponse(const WebCore::ResourceResponse & r={...})  Line 408	C++
 	WebKit.dll!WebCore::ResourceLoader::didReceiveResponse(WebCore::ResourceHandle * __formal=0x057b7098, const WebCore::ResourceResponse & response={...})  Line 422 + 0x13 bytes	C++
 	WebKit.dll!WebCore::didReceiveResponse(_CFURLConnection * conn=0x03c5de68, _CFURLResponse * cfResponse=0x03b36f30, const void * clientInfo=0x057b7098)  Line 199 + 0x3d bytes	C++
 	CFNetwork.dll!URLConnectionClient::_clientSendDidReceiveResponse(_CFURLResponse * response=0x03b36f30, URLConnectionClient::ClientConnectionEventQueue * preQ=0x0035e5e8)  Line 1128 + 0x2f bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x0035e768, long count=3)  Line 2154	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x0035e900, long count=3)  Line 2192 + 0x19 bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x0035ea5c, long count=1)  Line 2192 + 0x19 bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x0035ebd0, long count=1)  Line 2192 + 0x19 bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x0035ed44, long count=1)  Line 2192 + 0x19 bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e=0x03ad20e0, long count=1)  Line 2192 + 0x19 bytes	C++
 	CFNetwork.dll!URLConnectionClient::processEvents()  Line 330 + 0x21 bytes	C++
 	CFNetwork.dll!URLConnectionWndProc(HWND__ * hWnd=0x0006038c, unsigned int message=1231, unsigned int wParam=63299176, long lParam=0)  Line 107	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x23 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchMessageWorker@8()  + 0xed bytes	
 	user32.dll!_DispatchMessageW@4()  + 0xf bytes	
 	WebKit.dll!RunLoop::runUntil(const CoreIPC::BinarySemaphore * semaphore=0x00630c0c, double absoluteTime=11296496739.844482)  Line 99 + 0xc bytes	C++
 	WebKit.dll!CoreIPC::Connection::waitForSyncReply(unsigned __int64 syncRequestID=15, double timeout=10000000000.000000)  Line 269 + 0x17 bytes	C++
 	WebKit.dll!CoreIPC::Connection::sendSyncMessage(CoreIPC::MessageID messageID={...}, unsigned __int64 syncRequestID=15, WTF::PassOwnPtr<CoreIPC::ArgumentEncoder> encoder={...}, double timeout=10000000000.000000)  Line 203 + 0x1d bytes	C++
 	WebKit.dll!CoreIPC::Connection::sendSync<Messages::WebPageProxy::CanRunBeforeUnloadConfirmPanel>(const Messages::WebPageProxy::CanRunBeforeUnloadConfirmPanel & message={...}, const CoreIPC::Arguments1<bool &> & reply={...}, unsigned __int64 destinationID=1, double timeout=10000000000.000000)  Line 323 + 0x36 bytes	C++
 	WebKit.dll!WebKit::WebChromeClient::canRunBeforeUnloadConfirmPanel()  Line 254 + 0x76 bytes	C++
 	WebKit.dll!WebCore::Chrome::canRunBeforeUnloadConfirmPanel()  Line 266 + 0x17 bytes	C++
 	WebKit.dll!WebCore::FrameLoader::shouldClose()  Line 2878 + 0xe bytes	C++
 	WebKit.dll!WebCore::FrameLoader::continueLoadAfterNavigationPolicy(const WebCore::ResourceRequest & __formal={...}, WTF::PassRefPtr<WebCore::FormState> formState={...}, bool shouldContinue=true)  Line 2943 + 0x10 bytes	C++
 	WebKit.dll!WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void * argument=0x03ad2720, const WebCore::ResourceRequest & request={...}, WTF::PassRefPtr<WebCore::FormState> formState={...}, bool shouldContinue=true)  Line 2872	C++
 	WebKit.dll!WebCore::PolicyCallback::call(bool shouldContinue=true)  Line 102 + 0x34 bytes	C++
 	WebKit.dll!WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction policy=PolicyUse)  Line 161	C++
 	WebKit.dll!WebKit::WebFrame::didReceivePolicyDecision(unsigned __int64 listenerID=10, WebCore::PolicyAction action=PolicyUse, unsigned __int64 downloadID=0)  Line 213 + 0x1d bytes	C++
 	WebKit.dll!WebKit::WebPage::didReceivePolicyDecision(unsigned __int64 frameID=6, unsigned __int64 listenerID=10, unsigned int policyAction=0, unsigned __int64 downloadID=0)  Line 1100	C++
 	WebKit.dll!CoreIPC::callMemberFunction<WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(unsigned __int64,unsigned __int64,unsigned int,unsigned __int64),unsigned __int64,unsigned __int64,unsigned int,unsigned __int64>(const CoreIPC::Arguments4<unsigned __int64,unsigned __int64,unsigned int,unsigned __int64> & args={...}, WebKit::WebPage * object=0x00631608, void (unsigned __int64, unsigned __int64, unsigned int, unsigned __int64)* function=0x5ae323b0)  Line 37 + 0x32 bytes	C++
 	WebKit.dll!CoreIPC::handleMessage<Messages::WebPage::DidReceivePolicyDecision,WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(unsigned __int64,unsigned __int64,unsigned int,unsigned __int64)>(CoreIPC::ArgumentDecoder * argumentDecoder=0x040e5ed8, WebKit::WebPage * object=0x00631608, void (unsigned __int64, unsigned __int64, unsigned int, unsigned __int64)* function=0x5ae323b0)  Line 222 + 0x15 bytes	C++
 	WebKit.dll!WebKit::WebPage::didReceiveWebPageMessage(CoreIPC::Connection * __formal=0x00630a88, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x040e5ed8)  Line 124 + 0x2f bytes	C++
 	WebKit.dll!WebKit::WebPage::didReceiveMessage(CoreIPC::Connection * connection=0x00630a88, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x040e5ed8)  Line 1652	C++
 	WebKit.dll!WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection * connection=0x00630a88, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x040e5ed8)  Line 537	C++
 	WebKit.dll!CoreIPC::Connection::dispatchMessages()  Line 455 + 0x31 bytes	C++
 	WebKit.dll!MemberFunctionWorkItem0<CoreIPC::Connection>::execute()  Line 76 + 0x10 bytes	C++
 	WebKit.dll!RunLoop::performWork()  Line 63 + 0x1a bytes	C++
 	WebKit.dll!RunLoop::wndProc(HWND__ * hWnd=0x0002039c, unsigned int message=1025, unsigned int wParam=6360784, long lParam=0)  Line 61	C++
 	WebKit.dll!RunLoop::RunLoopWndProc(HWND__ * hWnd=0x0002039c, unsigned int message=1025, unsigned int wParam=6360784, long lParam=0)  Line 43 + 0x18 bytes	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x23 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchMessageWorker@8()  + 0xed bytes	
 	user32.dll!_DispatchMessageW@4()  + 0xf bytes	
 	WebKit.dll!RunLoop::runUntil(const CoreIPC::BinarySemaphore * semaphore=0x00000000, double absoluteTime=1.7976931348623157e+308)  Line 99 + 0xc bytes	C++
 	WebKit.dll!RunLoop::run()  Line 72 + 0x1c bytes	C++
 	WebKit.dll!WebKit::WebProcessMain(const WebKit::CommandLine & commandLine={...})  Line 82	C++
 	WebKit.dll!WebKitMain(const WebKit::CommandLine & commandLine={...})  Line 48 + 0x9 bytes	C++
 	WebKit.dll!WebKitMain(HINSTANCE__ * hInstance=0x01070000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x00402768, int nCmdShow=10)  Line 172 + 0x9 bytes	C++
 	WebKit2WebProcess.exe!wWinMain(HINSTANCE__ * hInstance=0x01070000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x00402768, int nCmdShow=10)  Line 44 + 0x18 bytes	C++
 	WebKit2WebProcess.exe!__tmainCRTStartup()  Line 589 + 0x1c bytes	C
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes

[Adam Barth]

I'm sure you're aware, but this one of the most delicate parts of getting this stuff to work reliably.  You want to have very tight control over what kinds of messages you're willing to process while waiting for a reply to a synchronous IPC message.

[Adam Roben (:aroben)]

(In reply to comment #4)

The load that we're receiving a response for is a subresource load. It is not the main resource load for which we're waiting to find out if we can run a beforeunload panel.

[Adam Roben (:aroben)]

(In reply to comment #6)
> (In reply to comment #4)
> 
> The load that we're receiving a response for is a subresource load.

This is not quite right. It is for a frame's main resource load.

> It is not the main resource load for which we're waiting to find out if we can run a beforeunload panel.

This is still true.

The two loads are for two subframes of the main frame. One of them is waiting on the beforeunload panel, the other is waiting on the content policy check.

[Adam Roben (:aroben)]

Sam, Anders: Any thoughts here?

[Anders Carlsson]

(In reply to comment #8)
> Sam, Anders: Any thoughts here?

Hmm, in this particular case, could we not dispatch win32 messages to the CFNetwork window?

[Adam Roben (:aroben)]

(In reply to comment #9)
> (In reply to comment #8)
> > Sam, Anders: Any thoughts here?
> 
> Hmm, in this particular case, could we not dispatch win32 messages to the CFNetwork window?

Yes, we could try that. But we'll also want to prevent timers (at least WebCore's timers) from firing, and there are probably other things we haven't thought of, too.

[Adam Roben (:aroben)]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Sam, Anders: Any thoughts here?
> > 
> > Hmm, in this particular case, could we not dispatch win32 messages to the CFNetwork window?
> 
> Yes, we could try that. But we'll also want to prevent timers (at least WebCore's timers) from firing, and there are probably other things we haven't thought of, too.

Deferring loading and pausing WebCore's shared timer seems to make the obvious bugs go away. Who knows what problems we'll find next!

[Adam Roben (:aroben)]

Created attachment 80683 [details]
New WIP patch

Here's a patch that does just that (in the worst way possible).

[Adam Roben (:aroben)]

I'm currently hoping that we can get away with only dispatching messages to web process windows that are descendants of UI process windows. And hopefully we can get away with only dispatching sent (as opposed to posted) messages.

[Adam Roben (:aroben)]

This got fixed as part of bug 58239.

*** This bug has been marked as a duplicate of bug 58239 ***