Bug 32300

Summary: [Qt] crash when clicking "Sign out" in GMail
Product: WebKit Reporter: David T. L. Wong <davidtlwong>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, kenneth, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
URL: http://www.gmail.com
Attachments:
Description Flags
Patch none

Description David T. L. Wong 2009-12-08 18:52:45 PST
QtLauncher and Arora segmentation fault when clicking "Sign out" in GMail.

It is found that m_resourceHandle is reset to Nil within QNetworkReplyHandler::sendResponseIfNeeded() for some reason.
Page redirection related.
Comment 1 David T. L. Wong 2009-12-08 18:58:51 PST
Back trace for QtLauncher against git://gitorious.org/qtwebkit/qtwebkit.git tag qtwebkit-4.6.0

#0  0xb7696640 in WebCore::QNetworkReplyHandler::sendResponseIfNeeded() ()
   from /home/david/browser/qtwebkit/WebKitBuild/Release/lib/libQtWebKit.so.4
#1  0xb76985d3 in WebCore::QNetworkReplyHandler::qt_metacall(QMetaObject::Call, int, void**) ()
   from /home/david/browser/qtwebkit/WebKitBuild/Release/lib/libQtWebKit.so.4
#2  0xb6393f0b in QMetaCallEvent::placeMetaCall(QObject*) ()
   from /usr/lib/libQtCore.so.4
#3  0xb63955fe in QObject::event(QEvent*) () from /usr/lib/libQtCore.so.4
#4  0xb66aff54 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
   from /usr/lib/libQtGui.so.4
#5  0xb66b767c in QApplication::notify(QObject*, QEvent*) ()
   from /usr/lib/libQtGui.so.4
#6  0xb63856cb in QCoreApplication::notifyInternal(QObject*, QEvent*) ()
   from /usr/lib/libQtCore.so.4
#7  0xb63862b2 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/libQtCore.so.4
#8  0xb638647d in QCoreApplication::sendPostedEvents(QObject*, int) ()
   from /usr/lib/libQtCore.so.4
#9  0xb63b03ff in ?? () from /usr/lib/libQtCore.so.4
#10 0xb593fe78 in g_main_context_dispatch () from /lib/libglib-2.0.so.0
#11 0xb5943720 in ?? () from /lib/libglib-2.0.so.0
#12 0xb5943853 in g_main_context_iteration () from /lib/libglib-2.0.so.0
#13 0xb63b002c in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#14 0xb6750be5 in ?? () from /usr/lib/libQtGui.so.4
#15 0xb6383c79 in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#16 0xb63840ca in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /usr/lib/libQtCore.so.4
#17 0xb638653f in QCoreApplication::exec() () from /usr/lib/libQtCore.so.4
#18 0xb66afdd7 in QApplication::exec() () from /usr/lib/libQtGui.so.4
#19 0x08059951 in main ()
Comment 2 Benjamin Poulain 2009-12-11 06:41:26 PST
I cannot reproduce this bug with Qt 4.6 trunk + Qt Webkit trunk. Nor with the webkit of Qt 4.6 trunk. The bug has probably be fixed since the 4.6.0 tag.

Could you try with the last version of Webkit/Qt?
Comment 3 David T. L. Wong 2009-12-11 08:17:24 PST
Which repository/branch/tag are you referring to? It's better if you guide me to a link.

I just tried WebKit.org svn trunk r51985 qt build, but still experiencing the same problem.

The problem is not being seen from every gmail user/configuration.
I have one colleague's account experience the logout problem, but another colleague's account doesn't.

And time clicking "Sign out" link is somewhat important too.
It is likely to crash if waiting all things loaded, especially the friend list of chat block of google talk on the left.

David
Comment 4 David T. L. Wong 2009-12-11 08:49:25 PST
What I found is
1. It enter sendResponseIfNeeded()
2. It enter last if(redirection.isValid()) block, m_resourceHandle is still valid
3. It execute "client->willSendRequest", m_resourceHandle is staill valid before entering willSendRequest()
4. Some callback trigger QNetworkReplyHandler::abort() and reset m_resourceHandle to Nil
5. Back to sendResponseIfNeeded(). Null pointer access when executing newRequest.toNetworkRequest(m_resource->getInternal()->m_frame);


I do believe it is a real bug that need to be fixed.
I tried qt 4.6.0, webkit.org svn trunk at 20 Oct 2009, all experience this problem.
Comment 5 David T. L. Wong 2009-12-13 19:53:43 PST
call trace that showing QNetworkReplyHandler::abort() is called within
QNetworkReplyHandler::sendResponseIfNeeded(). m_resourceHandle is reset to nil which cause the segmentation fault.

#0  WebCore::QNetworkReplyHandler::abort (this=0xa5c1c70)
    at ../../../webkit_dot_org_svn/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:175
#1  0xb65c37db in WebCore::ResourceHandle::cancel (this=0xa5c2678)
    at ../../../webkit_dot_org_svn/WebCore/platform/network/qt/ResourceHandleQt.cpp:148
#2  0xb63296eb in WebCore::ResourceLoader::didCancel (this=0xb7d7ac0, 
    error=@0xbfe467ac)
    at ../../../webkit_dot_org_svn/WebCore/loader/ResourceLoader.cpp:329
#3  0xb632b921 in WebCore::SubresourceLoader::didCancel (this=0xb7d7ac0, 
    error=@0xbfe467ac)
    at ../../../webkit_dot_org_svn/WebCore/loader/SubresourceLoader.cpp:234
#4  0xb6329849 in WebCore::ResourceLoader::cancel (this=0xb7d7ac0, 
    error=@0xbfe467fc)
    at ../../../webkit_dot_org_svn/WebCore/loader/ResourceLoader.cpp:350
#5  0xb632979a in WebCore::ResourceLoader::cancel (this=0xb7d7ac0)
    at ../../../webkit_dot_org_svn/WebCore/loader/ResourceLoader.cpp:340
#6  0xb62e662a in WebCore::DocumentThreadableLoader::cancel (this=0xadef7b0)
    at ../../../webkit_dot_org_svn/WebCore/loader/DocumentThreadableLoader.cpp:160
#7  0xb6590deb in WebCore::XMLHttpRequest::internalAbort (this=0x9fd7a48)
    at ../../../webkit_dot_org_svn/WebCore/xml/XMLHttpRequest.cpp:570
#8  0xb6591141 in WebCore::XMLHttpRequest::networkError (this=0x9fd7a48)
---Type <return> to continue, or q <return> to quit---
    at ../../../webkit_dot_org_svn/WebCore/xml/XMLHttpRequest.cpp:612
#9  0xb65920e3 in WebCore::XMLHttpRequest::didFailRedirectCheck (
    this=0x9fd7a48)
    at ../../../webkit_dot_org_svn/WebCore/xml/XMLHttpRequest.cpp:825
#10 0xb62e677b in WebCore::DocumentThreadableLoader::willSendRequest (
    this=0xadef7b0, loader=0xb7d7ac0, request=@0xbfe46a84)
    at ../../../webkit_dot_org_svn/WebCore/loader/DocumentThreadableLoader.cpp:173
#11 0xb632b0ed in WebCore::SubresourceLoader::willSendRequest (this=0xb7d7ac0, 
    newRequest=@0xbfe46a84, redirectResponse=@0xbfe46b28)
    at ../../../webkit_dot_org_svn/WebCore/loader/SubresourceLoader.cpp:115
#12 0xb63299f2 in WebCore::ResourceLoader::willSendRequest (this=0xb7d7ac0, 
    request=@0xbfe46a84, redirectResponse=@0xbfe46b28)
    at ../../../webkit_dot_org_svn/WebCore/loader/ResourceLoader.cpp:379
#13 0xb65c7dcb in WebCore::QNetworkReplyHandler::sendResponseIfNeeded (
    this=0xa5c1c70)
    at ../../../webkit_dot_org_svn/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:336
#14 0xb65c805f in WebCore::QNetworkReplyHandler::forwardData (this=0xa5c1c70)
    at ../../../webkit_dot_org_svn/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:351
#15 0xb65c8a07 in WebCore::QNetworkReplyHandler::qt_metacall (this=0xa5c1c70, 
    _c=QMetaObject::InvokeMetaMethod, _id=3, _a=0xbc61c68)
    at ./moc_QNetworkReplyHandler.cpp:78
#16 0xb3fa1f0b in QMetaCallEvent::placeMetaCall () from /usr/lib/libQtCore.so.4
#17 0xb3fa35fe in QObject::event () from /usr/lib/libQtCore.so.4
#18 0xb42bdf54 in QApplicationPrivate::notify_helper ()
   from /usr/lib/libQtGui.so.4
#19 0xb42c567c in QApplication::notify () from /usr/lib/libQtGui.so.4
#20 0xb3f936cb in QCoreApplication::notifyInternal ()
   from /usr/lib/libQtCore.so.4
#21 0xb3f942b2 in QCoreApplicationPrivate::sendPostedEvents ()
   from /usr/lib/libQtCore.so.4
#22 0xb3f9447d in QCoreApplication::sendPostedEvents ()
   from /usr/lib/libQtCore.so.4
#23 0xb3fbe3ff in ?? () from /usr/lib/libQtCore.so.4
#24 0xb359be78 in g_main_context_dispatch () from /lib/libglib-2.0.so.0
#25 0xb359f720 in ?? () from /lib/libglib-2.0.so.0
#26 0x098eccf8 in ?? ()
#27 0x00000000 in ?? ()
Comment 6 Benjamin Poulain 2009-12-14 02:29:59 PST
I cannot reproduce it, but given the last trace, the bug is valid.
Comment 7 Benjamin Poulain 2009-12-14 02:49:57 PST
Created attachment 44781 [details]
Patch

Simon fixed this in r29515 but r50454 introduced a regression by changing:
-    m_request = r.toNetworkRequest();
+    m_request = r.toNetworkRequest(m_resourceHandle->getInternal()->m_frame);
Comment 8 WebKit Review Bot 2009-12-14 02:54:10 PST
style-queue ran check-webkit-style on attachment 44781 [details] without any errors.
Comment 9 Kenneth Rohde Christiansen 2009-12-14 05:27:59 PST
Comment on attachment 44781 [details]
Patch

Great as always Benjamin! :-)

Btw, could you look into adding a layout test for this?
Comment 10 Benjamin Poulain 2009-12-14 06:33:23 PST
(In reply to comment #9)
> (From update of attachment 44781 [details])
> Great as always Benjamin! :-)
> 
> Btw, could you look into adding a layout test for this?

It is possible to write a autotest for it, but it would require mocking the network stack (we need a correct loading, then redirection, then a failure in the middle of the redirection).

The work is not worth it, we probably need a general way of simulating network problem to do this kind of tests.
Comment 11 Kenneth Rohde Christiansen 2009-12-14 06:38:08 PST
Maybe we should create a JIRA task for that?

> It is possible to write a autotest for it, but it would require mocking the
> network stack (we need a correct loading, then redirection, then a failure in
> the middle of the redirection).
> 
> The work is not worth it, we probably need a general way of simulating network
> problem to do this kind of tests.
Comment 12 WebKit Commit Bot 2009-12-14 12:49:59 PST
Comment on attachment 44781 [details]
Patch

Clearing flags on attachment: 44781

Committed r52112: <http://trac.webkit.org/changeset/52112>
Comment 13 WebKit Commit Bot 2009-12-14 12:50:03 PST
All reviewed patches have been landed.  Closing bug.