Bug 51934 – [Qt] QtWebKit crashes when using XMLHttpRequest from the unload event

• Component: Page Loading
• Status: RESOLVED FIXED

[Yael]

To reproduce the crash, open QtTestBrowser, load www.orbitz.com and close QtTestBrowser.
www.orbitz.com sends XMLHTTPRequest during the unload event and that is causing the crash. 


callstack:
0	QObject::thread	qobject.cpp	1387	0x04482945	
1	WebCore::QtNetworkReplyThreadSafeProxy::QtNetworkReplyThreadSafeProxy	QtNAMThreadSafeProxy.cpp	80	0x01e49fff	
2	WebCore::QNetworkReplyHandler::start	QNetworkReplyHandler.cpp	509	0x01e5098b	
3	WebCore::QNetworkReplyHandler::QNetworkReplyHandler	QNetworkReplyHandler.cpp	227	0x01e4eaa3	
4	WebCore::ResourceHandle::start	ResourceHandleQt.cpp	138	0x01e47809	
5	WebCore::ResourceHandle::create	ResourceHandle.cpp	70	0x01c512a4	
6	WebCore::ResourceLoader::start	ResourceLoader.cpp	163	0x01b18635	
7	WebCore::ResourceLoadScheduler::servePendingRequests	ResourceLoadScheduler.cpp	199	0x01b1b6c6	
8	WebCore::ResourceLoadScheduler::scheduleLoad	ResourceLoadScheduler.cpp	122	0x01b1b0a9	
9	WebCore::ResourceLoadScheduler::scheduleSubresourceLoad	ResourceLoadScheduler.cpp	90	0x01b1add6	
10	WebCore::DocumentThreadableLoader::loadRequest	DocumentThreadableLoader.cpp	323	0x01ae41f0	
11	WebCore::DocumentThreadableLoader::DocumentThreadableLoader	DocumentThreadableLoader.cpp	76	0x01ae2109	
12	WebCore::DocumentThreadableLoader::create	DocumentThreadableLoader.cpp	59	0x01ae185b	
13	WebCore::ThreadableLoader::create	ThreadableLoader.cpp	54	0x01b26734	
14	WebCore::XMLHttpRequest::createRequest	XMLHttpRequest.cpp	663	0x01e05335	
15	WebCore::XMLHttpRequest::send	XMLHttpRequest.cpp	543	0x01e04866	
16	WebCore::JSXMLHttpRequest::send	JSXMLHttpRequestCustom.cpp	132	0x01636136	
17	WebCore::jsXMLHttpRequestPrototypeFunctionSend	JSXMLHttpRequest.cpp	486	0x0152d35a	
18	??		0	0x08047c66	
19	JSC::JITCode::execute(JSC::RegisterFile*, JSC::ExecState*, JSC::JSGlobalData*)	/home/yael/webkit/ws1/WebKitBuild/Debug/bin/../lib/libQtWebKit.so.4	0	0x02195520	
20	JSC::Interpreter::executeCall	Interpreter.cpp	849	0x0219279a	
21	JSC::call	CallData.cpp	38	0x021bc8d6	
22	WebCore::JSMainThreadExecState::call	JSMainThreadExecState.h	48	0x015eb02a	
23	WebCore::JSEventListener::handleEvent	JSEventListener.cpp	124	0x0161dd81	
24	WebCore::EventTarget::fireEventListeners	EventTarget.cpp	342	0x0180c084	
25	WebCore::EventTarget::fireEventListeners	EventTarget.cpp	311	0x0180bf0b	
26	WebCore::DOMWindow::dispatchEvent	DOMWindow.cpp	1549	0x01b612d3	
27	WebCore::FrameLoader::stopLoading	FrameLoader.cpp	394	0x01aec33d	
28	WebCore::FrameLoader::closeURL	FrameLoader.cpp	466	0x01aec794	
29	WebCore::FrameLoader::detachFromParent	FrameLoader.cpp	2558	0x01af669f	
30	QWebPage::~QWebPage	qwebpage.cpp	1942	0x01eb7ced	
31	WebPage::~WebPage	webpage.h	44	0x0807cacc	
32	QObjectPrivate::deleteChildren	qobject.cpp	1949	0x04483796	
33	QWidget::~QWidget	qwidget.cpp	1589	0x037dd046	
34	QMainWindow::~QMainWindow	qmainwindow.cpp	329	0x03cb5bc1	
35	MainWindow::~MainWindow	mainwindow.h	41	0x0806f09a	
36	LauncherWindow::~LauncherWindow	launcherwindow.cpp	59	0x08067ddb	
37	qDeleteInEventHandler	qobject.cpp	3980	0x04487af4	
38	QObject::event	qobject.cpp	1194	0x044824e2	
39	QWidget::event	qwidget.cpp	8646	0x037efc20	
40	QMainWindow::event	qmainwindow.cpp	1417	0x03cb7e03	
41	QApplicationPrivate::notify_helper	qapplication.cpp	4396	0x037817b6	
42	QApplication::notify	qapplication.cpp	4361	0x03781622	
43	QCoreApplication::notifyInternal	qcoreapplication.cpp	732	0x0446962f	
44	QCoreApplication::sendEvent	qcoreapplication.h	215	0x0806e631	
45	QCoreApplicationPrivate::sendPostedEvents	qcoreapplication.cpp	1373	0x0446a6f9	
46	QCoreApplication::sendPostedEvents	qcoreapplication.cpp	1266	0x0446a3b1	
47	QCoreApplication::sendPostedEvents	qcoreapplication.h	220	0x0384fea2	
48	postEventSourceDispatch	qeventdispatcher_glib.cpp	277	0x044a22e0	
49	g_main_context_dispatch	/lib/libglib-2.0.so.0	0	0x0534f5e5	
50	??	/lib/libglib-2.0.so.0	0	0x053532d8	
51	g_main_context_iteration	/lib/libglib-2.0.so.0	0	0x053534b8	
52	QEventDispatcherGlib::processEvents	qeventdispatcher_glib.cpp	415	0x044a3312	
53	QGuiEventDispatcherGlib::processEvents	qguieventdispatcher_glib.cpp	204	0x0385c5fe	
54	QEventLoop::processEvents	qeventloop.cpp	149	0x044669ef	
55	QEventLoop::exec	qeventloop.cpp	201	0x04466b34	
56	QCoreApplication::exec	qcoreapplication.cpp	1009	0x04469d21	
57	QApplication::exec	qapplication.cpp	3672	0x0377ec50	
58	launcherMain	main.cpp	41	0x080707ff	
59	main	main.cpp	255	0x0807293f

[Benjamin Poulain]

Created attachment 78311 [details]
Test case

Reduction.

[Benjamin Poulain]

Created attachment 78315 [details]
Patch

[Benjamin Poulain]

It is not as bad as I thought. It is just a little but in QtTestBrowser, not an issue with WebKit.

[Kenneth Rohde Christiansen]

Comment on attachment 78315 [details]
Patch

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

> Tools/ChangeLog:12
> +        When accessing the network from the destructor of QWebPage, the network access manager
> +        was already deleted because the object WebPage was already destructed.
> +
> +        This solve the problem by linking the lifetime of the object to the web page. The object
> +        is now destructed in the destructor of QObject.

This is a bit confusing... when you say "web page" it is hard to know wether you refer to QWebPage or WebPage.

> Tools/QtTestBrowser/webpage.cpp:131
> -        m_qnamThread.reset(new QtNAMThread);
> +        if (!m_qnamThread)
> +            m_qnamThread = new QtNAMThread(this);

Wouldn't it be easier to keep the QScopedPointer and just check !m_quamThread->data() or so?

[Benjamin Poulain]

Created attachment 78317 [details]
Patch

[Benjamin Poulain]

(In reply to comment #4)
> > +        This solve the problem by linking the lifetime of the object to the web page. The object
> > +        is now destructed in the destructor of QObject.
> 
> This is a bit confusing... when you say "web page" it is hard to know wether you refer to QWebPage or WebPage.

Funny, I had the feeling the changelog wasn't great but couldn't find what is the problem. Is it better now?

 
> > Tools/QtTestBrowser/webpage.cpp:131
> > -        m_qnamThread.reset(new QtNAMThread);
> > +        if (!m_qnamThread)
> > +            m_qnamThread = new QtNAMThread(this);
> 
> Wouldn't it be easier to keep the QScopedPointer and just check !m_quamThread->data() or so?

QScopedPointer is the problem. Because they exist in WebPage. The order of destructor is: WebPage->QWebPage->QObject. If you destroy the qnam in WebPage, you have problems when accessing it in QWebPage.

I could use QWeakPointer, but I don't think that would improve anything here.

[Kenneth Rohde Christiansen]

Comment on attachment 78317 [details]
Patch

Why does QWebPage need to access it in its destructor? It seems that the nam is already set to 0 then: setNetworkAccessManager(0); in the patch

[Benjamin Poulain]

(In reply to comment #7)
> (From update of attachment 78317 [details])
> Why does QWebPage need to access it in its destructor?

Look at the backtrace from Yael or at the test case. It is simply that the page send some XHR in response to the unload event.

> It seems that the nam is already set to 0 then: setNetworkAccessManager(0); in the patch

It is set to 0 when you call 
WebPage::setQnamThreaded(false).
I am pretty sure that cannot work but it was in the original code so I kept the logic.

[Yael]

(In reply to comment #3)
> It is not as bad as I thought. It is just a little but in QtTestBrowser, not an issue with WebKit.

I am glad to hear that. Thanks for fixing it anyways.

[WebKit Commit Bot]

Comment on attachment 78317 [details]
Patch

Clearing flags on attachment: 78317

Committed r75340: <http://trac.webkit.org/changeset/75340>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.