Bug 29551 – [Qt] QtWebKit crash with Popups

• Component: Platform
• Status: RESOLVED FIXED

[Tor Arne Vestbø]

This bug report originated from Nokia internal issue QT-1210

--- Description ---

Jesper reported this one:

To reproduce:
Using qt-45, visit www.2shared.com.
Upload a file.
A javascript popup will say that your upload is complete
click ok - crash!

(gdb) bt
receiver=0xa7e4af8, method=0x0)
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qobject.cpp:2618
/home/jesper/work/depot/qt-45/src/network/access/qnetworkreplyimpl.cpp:482
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
WebCore::PageGroupLoadDeferrer::~PageGroupLoadDeferrer () from
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
---Type <return> to continue, or q <return> to quit---
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
object=0xa928698) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qobject.cpp:489
/home/jesper/work/depot/qt-45/src/corelib/kernel/qobject.cpp:1106
receiver=0xa928698, e=0xa584bc8)
    at /home/jesper/work/depot/qt-45/src/gui/kernel/qapplication.cpp:4084
receiver=0xa928698, e=0xa584bc8) at
/home/jesper/work/depot/qt-45/src/gui/kernel/qapplication.cpp:3631
receiver=0xa928698, event=0xa584bc8)
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:598
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
(receiver=0x0, event_type=0, data=0x9f6c600)
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:1236
event_type=0) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:1132
../../include/QtCore/../../../qt-45/src/corelib/kernel/qcoreapplication.h:218
/home/jesper/work/depot/qt-45/src/corelib/kernel/qeventdispatcher_glib.cpp:209
flags={i = -1077205868})
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qeventdispatcher_glib.cpp:323
(this=0x9f719d0, flags={i = -1077205820})
    at /home/jesper/work/depot/qt-45/src/gui/kernel/qguieventdispatcher_glib.cpp:202
= -1077205744}) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qeventloop.cpp:149
-1077205672}) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qeventloop.cpp:196
/home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:880
/home/jesper/work/depot/qt-45/src/gui/kernel/qapplication.cpp:3553

[Jocelyn Turcotte]

Created attachment 41034 [details]
Debug backtrace

What seems to happen is that QNetworkReplyImplPrivate::outgoingData is deleted when the backend has finished dealing with the replay.
However, the network reply is trying to access this object later after the javascript alert display during the response handling.

[Jocelyn Turcotte]

Created attachment 41035 [details]
Patch

This patch disable the deletion of the FormDataIODevide object when the http backend has finished handling the reply.
The buffers will be deleted a bit later, when the QNetworkReplyImpl is deleted.

The disadvantages I see from this patch is that we are keeping this memory allocated for a slightly longer time.
Is there any other reason that I may have missed that this object was deleted on the finish() signal?

Other solutions would be to connect the QNetworkReplyImpl on the destroyed() signal of the outgoingData member to prevent the dangling pointer.

[Simon Hausmann]

Jocelyn, the patch looks good to me. Do you think it's possible to write a layout test or unit tests for this?

[Holger Freyther]

Okay, a unit test would be nice.

[Jocelyn Turcotte]

*** Bug 29592 has been marked as a duplicate of this bug. ***

[Jocelyn Turcotte]

(In reply to comment #3)
> Jocelyn, the patch looks good to me. Do you think it's possible to write a
> layout test or unit tests for this?

I'm not sure, the test case is as follow:
- Client sends a POST request to the server with some data.
- The server respond with a text/html which displays a JavaScript alert() while loading.

I tried with a local apache and the same response as the other server, can't reproduce. I have to send the request to a remote server (maybe the latency plays a role).

I don't see a way of having a reliable and maintainable unit test for this test case. Tell me if you have an idea.

[Adam Barth]

Comment on attachment 41035 [details]
Patch

I'm impressed that you can fix a crash only by removing code.  Based on the discussion, this patch appears fine.  I'm sad you can't make a test.  You know you can use PHP sleep to simulate long load times...

[Yong Li]

Comment on attachment 41035 [details]
Patch

Let commit bot land it

[WebKit Commit Bot]

Comment on attachment 41035 [details]
Patch

Clearing flags on attachment: 41035

Committed r49789: <http://trac.webkit.org/changeset/49789>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.