PDA

View Full Version : I think, there is a problem with QNetworkReply and threads



Wurgl
9th January 2010, 22:57
I am using qt4.5.3 / Linux.

It seems that QNetworkReply is not safe to be used in threads. When you compile the following code then it crashes at various different locations after a random time somewhere inside the Qt libraries.

When I chance the code by using QObject instead of QThread as superclass (and change some calls too), this code runs fine for hours. But here with threads it crashed very soon.

As I have investigated, there is a file qnetworkreplyimpl.cpp with the function QNetworkReplyImplPrivate::feed(const QByteArray &data) and there you find at line 377/378 the following code:


char *ptr = readBuffer.reserve(data.size());
memcpy(ptr, data.constData(), data.size());

As I have investigated, data.size() returns different sizes when called in sequence inside a thread. This seems to be caused by the low level network reads which fill the buffer and which run in some other thread, e.g the main event loop. So, the QRingBuffer readBuffer reserves some space which is given with the first call, then some other thread expands the amount of available data in QByteArray data and when memcpy gets called, it filles more data than there is prepared space. ... and there is another call to data.size() in this function.

However, when I fix this, there are still crashes, since I am not very familiar with the code I cannot provide a real fix.


This is all my test source, the filename is xx.cpp. The only thing you need to do is to set up a http server with a reasonable large file. Voila, it crashes ... somewhere.


#include <Qt/qnetworkaccessmanager.h>
#include <Qt/qnetworkreply.h>
#include <Qt/qeventloop.h>
#include <Qt/qapplication.h>
#include <Qt/qthread.h>

class Hurz : public QThread
{
Q_OBJECT

public slots:
void dataAvailable();

public:
Hurz();
void run();

private:
QNetworkAccessManager *_manager;
QNetworkReply *_reply;
QNetworkRequest *_request;
};

Hurz::Hurz()
{
}

void Hurz::run()
{
_manager = new QNetworkAccessManager();

// you need to change the url to a different one which sends you "endless" data, eg. a file with almost gigabytes of data
_request = new QNetworkRequest(QUrl("http://big.server/with/extreme/large/file"));
_reply = _manager->get(*_request);
connect(_reply, SIGNAL(readyRead()), this, SLOT(dataAvailable()));
exec();
}

void Hurz::dataAvailable()
{
_reply->readAll();
}

int main(int argc, char **argv)
{
QApplication app(argc, argv);

Hurz h;
h.start();

QEventLoop evLoop;
for(;;)
evLoop.processEvents(QEventLoop::ExcludeUserInputE vents);
}

#include "xx.moc"

Rembobo
10th January 2010, 09:44
I think the main problem is that _reply object created in thread is accessed outside the thread (Hurtz h object lives in main thread therefore dataAvailable slot is executed in main thread).
You can check it with QThread::currentThreadId().
Try moveToThread (http://qt.nokia.com/doc/4.6/qobject.html#moveToThread)

numbat
10th January 2010, 10:12
The creator says: (from http://labs.trolltech.com/blogs/2007/11/24/one-more-piece-falling-into-place-network-access/)


Ankur: no, the new framework is not multi-threaded nor thread-safe, but it’s reentrant. That is, you’re allowed to use a QNeworkAccessManager only from one thread, but you can have many objects, one in each thread.

which would seem to rule out your usage (creating in one thread, reading in another).

Wurgl
10th January 2010, 12:50
I looked at the address returned by _reply->thread() (which is from class QObject) and it returns the same as the one returned by _manager->thread().

Of course the address of this->thread() (this is the instance of class Hurz) is different, but that is mentioned somewhere. QThread wich is subclass of QObject ist created from the main-thread.

However, it seems to run more stable, but crashes still, when I use


int available = _reply->bytesAvailable();
if(available == 0)
return;
_reply->read(available);


Now the crash seems to happen elsewhere, somewhere with handling signals. There is a lot written at http://doc.trolltech.com/4.5/threads.html#threads-and-qobjects which I still do not understand in all consequences.

However, ths stacktrace looks now very often like this one:


0x00002b5dc19d4356 in QNetworkReplyImplPrivate::handleNotifications (this=0x5a5250)
at ../../include/QtCore/../../src/corelib/tools/qlist.h:416
416 { T t = first(); removeFirst(); return t; }
(gdb) where
#0 0x00002b5dc19d4356 in QNetworkReplyImplPrivate::handleNotifications (this=0x5a5250)
at ../../include/QtCore/../../src/corelib/tools/qlist.h:416
#1 0x00002b5dc19d4479 in QNetworkReplyImpl::event (this=<value optimized out>, e=0x7697b8)
at access/qnetworkreplyimpl.cpp:656
#2 0x00002b5dc0cf219e in QApplicationPrivate::notify_helper (this=0x54e780, receiver=0x5963e0, e=0x602260)
at kernel/qapplication.cpp:4065
#3 0x00002b5dc0cf6d80 in QApplication::notify (this=0x7fffea096d00, receiver=0x5963e0, e=0x602260)
at kernel/qapplication.cpp:3961
#4 0x00002b5dc1767ea3 in QCoreApplication::notifyInternal (this=0x7fffea096d00, receiver=0x5963e0,
event=0x602260) at kernel/qcoreapplication.cpp:610
#5 0x00002b5dc1768f7d in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0, data=0x5a3a30)
at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:213
#6 0x00002b5dc1791903 in postEventSourceDispatch (s=<value optimized out>)
at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:218
#7 0x00002b5dc2e22ffb in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#8 0x00002b5dc2e2381c in ?? () from /usr/lib/libglib-2.0.so.0
#9 0x00002b5dc2e23d2b in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#10 0x00002b5dc1791c8e in QEventDispatcherGlib::processEvents (this=0x595640, flags=<value optimized out>)
at kernel/qeventdispatcher_glib.cpp:328
#11 0x00002b5dc1767335 in QEventLoop::processEvents (this=<value optimized out>, flags=@0x40800020)
at kernel/qeventloop.cpp:149
#12 0x00002b5dc176759c in QEventLoop::exec (this=0x40800060, flags=@0x40800070) at kernel/qeventloop.cpp:201
#13 0x00002b5dc1682fe8 in QThread::exec (this=<value optimized out>) at thread/qthread.cpp:487
...


######################


The creator says: (from http://labs.trolltech.com/blogs/2007/11/24/one-more-piece-falling-into-place-network-access/)

which would seem to rule out your usage (creating in one thread, reading in another).

Sorry, english is not my native language. Am I correct or wrong?

Wurgl
10th January 2010, 15:46
However. I will drop this code totally and write my own class to access the server.

The other side (the server) is an Intel sa1110 Chip with 206 MHz (a Mobotix webcam) and my client runs on some Opteron 244 with 1.8GHZ. The Opteron needs 100% CPU-Time just for pure reading. This is far too slow code.

BTW: It works okay, if I create the three classes in main() and just do the retrieving in the subthread.

Rembobo
11th January 2010, 07:12
In first post example _reply object is read/written in h object thread (run method...) and read/written in main thread (dataAvailable slot) at same time (without synchronization. For example some pointer became invalid while still used as valid in other thread).

The Opteron needs 100% CPU-Time just for pure reading.
Try to wait until 100000 or more bytes arrive and then read them. The finished signal to get the last portion of bytes.
Try (in this case Hurtz slots will be executed in Hurtz thread)

Hurz::Hurz()
{
moveToThread(this);
}
Some example (it crashed before using moveToThread)


#include <QNetworkAccessManager>
#include <QNetworkReply>
#include <QCoreApplication>
#include <QThread>
#include <QEventLoop>
#include <QDebug>
#include <QTime>

class Hurz : public QThread
{
Q_OBJECT

public slots:
void dataAvailable();
void slot_finished();

public:
Hurz();
void run();

private:
QNetworkAccessManager *_manager;
QNetworkReply *_reply;
QNetworkRequest *_request;
QTime m_time;
};

Hurz::Hurz()
{
moveToThread(this);
m_time = QTime::currentTime();
}

void Hurz::run()
{
qDebug() << "Hurz::run thread id is " << QThread::currentThreadId();
_manager = new QNetworkAccessManager();

// you need to change the url to a different one which sends you "endless" data, eg. a file with almost gigabytes of data
_request = new QNetworkRequest(QUrl("http://ftp.stack.nl/pub/users/dimitri/doxygen-1.6.2-setup.exe"));
_reply = _manager->get(*_request);
connect(_reply, SIGNAL(readyRead()), this, SLOT(dataAvailable()));
connect(_reply, SIGNAL(finished()), this, SLOT(slot_finished()));
exec();
}

void Hurz::dataAvailable()
{
if (_reply->size() > 100000) {
qDebug() << "Hurz::dataAvailable: " << "Thread id is" << QThread::currentThreadId();
QByteArray arr = _reply->readAll();
int ms = m_time.msecsTo(QTime::currentTime());
m_time = QTime::currentTime();
qDebug() << " " << arr.size() << "in" << ms << "ms. " << (float)arr.size() / 1024 / ms * 1000 << "kB/s";
}
}

void Hurz::slot_finished()
{
qDebug() << "Hurz::slot_finished: " << "Thread id is" << QThread::currentThreadId();
if (_reply->size() > 0) {
QByteArray arr = _reply->readAll();
int ms = m_time.msecsTo(QTime::currentTime());
m_time = QTime::currentTime();
qDebug() << " " << arr.size() << "in" << ms << "ms. " << (float)arr.size() / 1024 / ms * 1000 << "kB/s";
}
quit();
}

int main(int argc, char **argv)
{
QCoreApplication app(argc, argv);

Hurz h;
//QEventLoop evLoop;
QObject::connect(&h, SIGNAL(finished()), &app, SLOT(quit()));
h.start();

//for(;;)
// evLoop.processEvents(QEventLoop::ExcludeUserInputE vents);
//evLoop.exec(QEventLoop::ExcludeUserInputEvents);
app.exec();
}

#include "main.moc"

wysota
11th January 2010, 10:15
What do you need threads here for anyway?

By the way, I'd drop this code:

QEventLoop evLoop;
for(;;)
evLoop.processEvents(QEventLoop::ExcludeUserInputE vents);
and change it to:

app.exec();

Wurgl
12th January 2010, 15:42
Thanks!

moveToThread() was the solution for both the crash and the time-eating. My selfwritten class uses about 2-3% of CPU, Qt uses about 7-8% which is okay.

However, it seems to be a little bit strange to me, I really expected, that a subclass of QThread already lives in that thread.

wysota
12th January 2010, 16:07
QThread object is the thread controller, not the thread itself. And if QThread lived in the thread it represents, what would happen to it once the thread was finished?

faldzip
12th January 2010, 16:39
However, it seems to be a little bit strange to me, I really expected, that a subclass of QThread already lives in that thread.
Because you haven't read the Qt Documentation about QThread class :P where it is clearly written than body of run() is finally in new created thread with QThread::start() method.

Wurgl
14th January 2010, 23:55
What do you need threads here for anyway?
Reason 1: The example ist just a dummy code. The real program has multiple stages with image processing, each of these stages may be time consuming. And since I get images in an endless stream, I avoid loosing any of these images. Each stage shall run in its own thread, stage 1 is grabbing the images, stage 2 is some analysing and filtering, stage 3 is some neuronal network processing, stage 4 is storing in the file system ... and a few more stages
Reason 2: Finally it will run on a machine with 8 cores, so why not using all of them?
Reason 3: Because it is cool, when *ix-utility top shows a cpu-usage greater than 100% :-)


QThread object is the thread controller, not the thread itself. And if QThread lived in the thread it represents, what would happen to it once the thread was finished?

So a clean implementation is to create some subclasses of QObjects in QThread::run() which do the real work and not using moveToThread()?