Bug 106761

Summary:

[Qt] Using sleep to delay in http server reponse causes QtWebKit page loading blocked

Product:

WebKit

Reporter:

Chen Zhixiang <chenzx>

Component:

Page Loading

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

allan.jensen, jturcotte

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Test tcp server script for this case	none
Screen snapshot for this bug, gui is waiting for response, but does not read timeout	none
Patch for user customable read timeout on non-blocking socket (Works on Qt-4.8.4 Windows)	none
qeventdispatcher_unix.cpp modified also, Linux Test OK	none

Chen Zhixiang

Reported 2013-01-14 00:31:42 PST

build a simple test http server script: sleep 1000s before send any http response data to browser client The QtWebKit demos/browser loading procedure seems blocked Pause on VS2008 to view all threads, there is a thread condition wait on void* IconDatabase::syncThreadMainLoop() wtf ntdll.dll!7c92e514() [Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll] ntdll.dll!7c92df5a() kernel32.dll!7c8025db() kernel32.dll!7c802542() QtCored4.dll!QWaitConditionPrivate::wait(QWaitConditionEvent * wce=0x04eccfa8, unsigned long time=4294967295) Line 114 + 0x11 bytes C++ QtCored4.dll!QWaitCondition::wait(QMutex * mutex=0x0107fdf8, unsigned long time=4294967295) Line 175 + 0x12 bytes C++ QtWebKitd4.dll!WTF::ThreadCondition::wait(WTF::Mutex & mutex={...}) Line 254 C++ > QtWebKitd4.dll!WebCore::IconDatabase::syncThreadMainLoop() Line 1438 C++ QtWebKitd4.dll!WebCore::IconDatabase::iconDatabaseSyncThread() Line 1052 + 0x8 bytes C++ QtWebKitd4.dll!WebCore::IconDatabase::iconDatabaseSyncThreadStart(void * vIconDatabase=0x0107f7b0) Line 956 C++ QtWebKitd4.dll!WTF::threadEntryPoint(void * contextData=0x01107e80) Line 67 + 0x7 bytes C++ QtWebKitd4.dll!WTF::ThreadPrivate::run() Line 66 + 0xf bytes C++ QtCored4.dll!QThreadPrivate::start(void * arg=0x01107f78) Line 348 C++ msvcr90d.dll!_callthreadstartex() Line 348 + 0xf bytes C msvcr90d.dll!_threadstartex(void * ptd=0x01108828) Line 331 C kernel32.dll!7c80b729() I can not figure out why blocked in IconDatabase::syncThreadMainLoop() .

Attachments
Test tcp server script for this case (2.35 KB, text/plain) 2013-01-21 19:43 PST, Chen Zhixiang	no flags	Details
Screen snapshot for this bug, gui is waiting for response, but does not read timeout (18.13 KB, image/png) 2013-01-21 19:51 PST, Chen Zhixiang	no flags	Details
Patch for user customable read timeout on non-blocking socket (Works on Qt-4.8.4 Windows) (45.44 KB, application/octet-stream) 2013-01-24 02:42 PST, Chen Zhixiang	no flags	Details
qeventdispatcher_unix.cpp modified also, Linux Test OK (51.27 KB, application/octet-stream) 2013-01-25 02:01 PST, Chen Zhixiang	no flags	Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chen Zhixiang

Comment 1 2013-01-14 00:33:53 PST

The perl cgi script: #!d:/strawberry/perl/bin/perl sleep 1000 ; print "Content-type: text\/html\n\n" ; print "<body>Hello</body>\n\n" ;

Chen Zhixiang

Comment 2 2013-01-14 00:54:54 PST

i think IconDatabase::syncThreadMainLoop() should use a timeout-ed m_syncCondition.wait, for if server just doesn't reply data(This is a inverse DoS attack to client!!!), client could just take the http connection as broken.

Chen Zhixiang

Comment 3 2013-01-20 18:27:28 PST

I guess the problem does NOT relate to IconDatabase::syncThreadMainLoop(), it's a classical problem: QtWebKit uses a blocking socket for under networking, if the network or server blocks/delays, it will cause GUI client to blocked. The root solution is to use a non-blocking socket for client QtWebKit, but i hesitate whether it needs big modification? Perhaps i could use bool QAbstractSocket::waitForReadyRead(int msecs = 30000) to fix this, But due to https://bugreports.qt-project.org/browse/QTBUG-24451 , waitForReadyRead is also not reliable. So, how to ```non-blocking``` in QtWebKit's network part code?

Chen Zhixiang

Comment 4 2013-01-20 18:33:41 PST

http://lists.trolltech.com/qt-interest/2007-01/thread00209-0.html In java, if I force to close a io stream of a socket in another thread, the blocking network io thread will receive a exception -- so it breaks from a io blocking wait state. But when i migrate this idea from java to c++(qt), scenes different: i could call ::close/::closesocket to QAbstractSocket::socketDescriptor(), but nothing(exception) happened. The network io thread can't get notification! So it can't recover from a io blocking wait state.

Chen Zhixiang

Comment 5 2013-01-21 03:19:59 PST

I wrote a simple test to use QAbstractSocket::waitForReadyRead: #include<QtCore> #include<QtNetwork> int _tmain(int argc, _TCHAR* argv[]) { qDebug("%s: main begin", QTime::currentTime().toString().toAscii().data()); QTcpSocket s; s.connectToHost(QString("10.167.233.52"), 8888); bool ret = s.waitForReadyRead(30000); qDebug("%s: ret=%d", QTime::currentTime().toString().toAscii().data(), ret); return 0; } The debug output indicates QAbstractSocket::waitForReadyRead performs OK. However, QtWebKit in qt-4.8.4 seems to NOT using it, it uses a QReadNotifier which says "wrapping a socket into non-blocking mode", but, there is even no read timeout mechanism!

Jocelyn Turcotte

Comment 6 2013-01-21 03:59:51 PST

What do you mean by "The QtWebKit demos/browser loading procedure seems blocked", how did you observe it? What code do you run on the client exactly, are you loading only your perl script url, or are you first loading a page normally, and then the perl script HTTP URL? The network loading is put in a thread by QNetworkAccessManager, and TCP packets coming in should be handed to the application and the thread should return to the event loop immediately until more packets come. If what you are reporting is true, I think this is bad, but it could also be caused by code in your client.

Chen Zhixiang

Comment 7 2013-01-21 19:43:45 PST

Created attachment 183871 [details] Test tcp server script for this case

Chen Zhixiang

Comment 8 2013-01-21 19:51:35 PST

Created attachment 183873 [details] Screen snapshot for this bug, gui is waiting for response, but does not read timeout

Chen Zhixiang

Comment 9 2013-01-21 19:54:03 PST

I think there should be a default read timeout (e.g. 30secs) for QtWebKit, but currently if server delays, the browser client can't auto read timeout.

Chen Zhixiang

Comment 10 2013-01-21 22:42:08 PST

d:\qt-everywhere-opensource-src-4.8.4\src\corelib\kernel\qsocketnotifier.h: class Q_CORE_EXPORT QSocketNotifier : public QObject { Q_OBJECT Q_DECLARE_PRIVATE(QObject) public: enum Type { Read, Write, Exception }; QSocketNotifier class defines 3 types, if I can add the 4th ReadTimeout, everything will be perfect, better if the timeout value can be set indivually of each socket.

Jocelyn Turcotte

Comment 11 2013-01-22 03:57:00 PST

(In reply to comment #9) > I think there should be a default read timeout (e.g. 30secs) for QtWebKit, but currently if server delays, the browser client can't auto read timeout. I thought that you meant that the GUI is frozen/irresponsive, but it doesn't seem to be the case (at least when I tested with your server script). Isn't that what you meant with: "QtWebKit uses a blocking socket for under networking, if the network or server blocks/delays, it will cause GUI client to blocked."? If it's only the loading that you report as blocked, I think that it is fine not to have a read timeout. We have a connection timeout of 30 seconds, but if the server was able to be contacted then we should wait for it.

Chen Zhixiang

Comment 12 2013-01-22 18:02:00 PST

yes, you're right. The GUI is not being blocked, It is the under socket io thread that is being blocking because of not receiving incoming data in time. And the use can also click "Stop" to abort this http connection request. But what I really need is a "auto- read timeout" mechanism for which user can also set the timeout threshold value per socket connection. I have discovered that QtWebKit is not using QAbstactSocket::waitForReadyRead(int ms) for reading data, instead, it uses a QSocketNotifier to receive "readyRead" signals from OS to achieve "non-blocking" behaviour. Anyway, due to our internal project needs, to get a "read timeout" on non-blocking sockets, i think, to add a "ReadTimeout" type for QSocketNotifier will give user some goods. The API should be flexiable.

Jocelyn Turcotte

Comment 13 2013-01-23 06:08:24 PST

Ok, but this is then a Qt issue, QtWebKit behaves properly. Other browsers (Chrome at least) don't timeout on read either. Depending on the issue you are trying to address, you can also try to solve it at a lower layer using mechanisms like watchdog timers or TCP keepalives ( http://tldp.org/HOWTO/TCP-Keepalive-HOWTO/overview.html ) I'm resolving this issue as won't fix, please re-open if you still think there is a bug related to this in WebKit.

Chen Zhixiang

Comment 14 2013-01-23 17:51:19 PST

I can't agree with you. Both solutions you provide doesn't meets our internal project needs: （1）User can setup a timeout timer outside QtNetwork & QtWebKit, but it will be hard to monitor at each socket level, and if user want to close http request due to server response delay, he can not controll everything at detailed level; （2）TCP keepalive may be nice, but the 2 scenes are different: in my case, the under TCP connection is good, the server just didn't want to response in time, the client had to make out a method to "auto read timeout", to avoid long waiting in non-blocking mode. I've add a 4th type "ReadTimeout" in QSocketNotifier which is in QtCore, and add a subclass QReadTimeoutNotifier which is in QtNetwork, I use a QTimer to `watchdog` every socket. I patched a lot of files really. Everything is good: if the timer timeout, it will notify the socket io thread. My now problem is: Qt's class hierarchies is wierd: there is a `QNetworkReply` and a `QHttpNetworkReply`, the demos/browser uses `QNetworkReply`, but i can not find a connection how socket read timeout error reaches demos/browser with Qt's signal-slot mechanism.

Chen Zhixiang

Comment 15 2013-01-24 02:42:14 PST

Created attachment 184449 [details] Patch for user customable read timeout on non-blocking socket (Works on Qt-4.8.4 Windows) To see the effect, you must modify WebPage in demos/browser to use ErrorPageExtension: bool WebPage::extension(Extension extension, const ExtensionOption *option, ExtensionReturn *output) { if(extension == QWebPage::ErrorPageExtension) { QWebPage::ErrorPageExtensionOption* opt = (QWebPage::ErrorPageExtensionOption*)option; QWebPage::ErrorPageExtensionReturn* ret = (QWebPage::ErrorPageExtensionReturn*)output; QString text(""); switch(opt->error) { case QNetworkReply::TimeoutError: ret->content = QByteArray("<body>Read Timeout</body>"); return true; default: break; } } return false; } bool WebPage::supportsExtension(Extension extension) const { return extension==ErrorPageExtension || QWebPage::supportsExtension(extension); }

Jocelyn Turcotte

Comment 16 2013-01-24 05:21:04 PST

Did some research and here's a similar issue for Mozilla that might resemble your "internal project needs": https://bugzilla.mozilla.org/show_bug.cgi?id=407190 But I don't understand why you have to go all the way down those layers to plant your QTimer, you can manage this on top of QNetworkReply and call abort() when it times out. All this could be done in Source\WebCore\platform\network\qt\QNetworkReplyHandler.cpp, but given the few users that would benefit of it, I think that this should only be implemented if the code is simple enough to make the maintenance costs worth it.

Chen Zhixiang

Comment 17 2013-01-24 18:01:31 PST

I've tried to monitor QNetworkReplyHandler class as you said, but this method has 3 problems: （1）User has to maintain QTimer in his app, and the code become dirty; （2）use abort on QNetworkReplyHandler, it seems I can stop the socket io thread, but the upper GUI thread doesn't get notification, the WebView loading procedure is not stopped. Perhaps i could post a special message to WebView to stop loading, but, again! the code will be too nasty. （3）I prefer the "set a config, and then receive a notification callback" way, this just requires more API flexibility and more user usability. The modification to qeventdispatcher_win.cpp is very hack-ish, but it works, since the internal impl. in qeventdispatcher_unix.cpp is not the same (it used fd_set), but I've figured out a way to modify it. Just need to verify the idea...

Chen Zhixiang

Comment 18 2013-01-24 18:10:50 PST

I have seen https://bugzilla.mozilla.org/show_bug.cgi?id=407190 you provided, that's just what I need! But our internal project is built on QtWebKit, I've to find a way to do it. Pity mozilla guys didn't fully solve this problem. User controlled read timeout may interfere with existing non-blocking io operation state machine, but i thought if really there is a urge needs for this, things would be solved finally. Even stepping forward, modifying Qt core code is NOT very good, it is better to dig in deeper —— to set a new TCP/IP option, let's naming it "NONBLOCK_USER_READ_TIMEOUT", this probably will be the right solution.

Chen Zhixiang

Comment 19 2013-01-25 02:01:35 PST

Created attachment 184703 [details] qeventdispatcher_unix.cpp modified also, Linux Test OK I did sosme tricks to Linux fd_set, everything works fine!

Jocelyn Turcotte

Comment 20 2013-01-25 05:33:29 PST

(In reply to comment #17) > （1）User has to maintain QTimer in his app, and the code become dirty; I disagree with this, putting the dirty code in QtCore instead is just pushing the dirt under the carpet. You do as you wish in your internal project, but this kind of code is not attractive and won't be accepted upstream unless you accept the input given to you. Plus, may I note, that only WebKit code should be submitted here, follow http://qt-project.org/wiki/Qt-Contribution-Guidelines to know how to do so for QtCore.

Chen Zhixiang

Comment 21 2013-01-27 03:03:49 PST

I'm posting here is because qt-project.org doesn't accept bug report and is because i want to receive some help or advice, Since i've found a solution by modifying QtCore, I'm only wish to give help to those who have similar needs. You may not wish to accept such a patch, but Qt source is definitely not very flexible enough.

Jocelyn Turcotte

Comment 22 2014-02-03 03:24:22 PST

=== Bulk closing of Qt bugs === If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary. If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.

Note You need to log in before you can comment on or make changes to this bug.