QUdpSocket in the QThread - critical errors



  • Hello!

    I am new here and newbie in Qt. I have very big problem with blocking transmission in a thread. I want to have as fast(frequent) as possible transmission in it..

    So, I have blocking "low level" function in simplification:
    @Client::Update{
    ...
    socket.writeDatagram (TxBuff, csize, ipaddr, 7654 );
    if(socket.waitForReadyRead(200) == false) {
    return;
    }
    rlen = socket.readDatagram(RxBuff, RXBUFFSIZE);
    ...
    }@

    Next, I have my thread class:
    @class thConn : public QThread
    {
    Q_OBJECT
    public:
    thConn(QObject *parent = 0);
    ~thConn();

    Client client;
    

    private:
    void run();

    private:@
    bool abort;

    };

    Next, I have thread code:
    @thConn::thConn(QObject *parent) :
    QThread(parent)
    {
    abort = false;
    }

    thConn::~thConn()
    {
    abort = true;
    wait(); /* wait while thread is running */
    }

    void thConn::run()
    {
    while(abort == false)
    {
    msleep(1);
    if (client.Connected == true)
    {
    client.Update();
    }
    }
    }
    @

    My application leaves with this messages:
    @*** glibc detected *** /home/lukasz/Proj/UdpClient-build-desktop-Desktop_Qt_4_8_1_for_GCC__Qt_SDK__Release/UdpClient: double free or corruption (fasttop): 0xb2300df0 ***
    ======= Backtrace: =========
    /lib/i386-linux-gnu/libc.so.6(+0x75ee2)[0xb6594ee2]
    /usr/lib/i386-linux-gnu/libstdc++.so.6(_ZdlPv+0x1f)[0xb679051f]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtCore.so.4(_ZN20QEventDispatcherGlib24unregisterSocketNotifierEP15QSocketNotifier+0xc5)[0xb699e875]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtCore.so.4(_ZN15QSocketNotifier10setEnabledEb+0x3a)[0xb698b28a]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtNetwork.so.4(+0xda95a)[0xb6ba295a]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtNetwork.so.4(+0xd00e1)[0xb6b980e1]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtNetwork.so.4(+0xbd67b)[0xb6b8567b]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtNetwork.so.4(+0xdaa23)[0xb6ba2a23]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtGui.so.4(_ZN19QApplicationPrivate13notify_helperEP7QObjectP6QEvent+0xac)[0xb6d72d6c]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtGui.so.4(_ZN12QApplication6notifyEP7QObjectP6QEvent+0x166)[0xb6d79bb6]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtCore.so.4(_ZN16QCoreApplication14notifyInternalEP7QObjectP6QEvent+0x7b)[0xb696a6cb]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtCore.so.4(+0x1be5ca)[0xb699e5ca]
    /lib/i386-linux-gnu/libglib-2.0.so.0(g_main_context_dispatch+0x146)[0xb6444d86]
    /lib/i386-linux-gnu/libglib-2.0.so.0(+0x47125)[0xb6445125]
    /lib/i386-linux-gnu/libglib-2.0.so.0(g_main_context_iteration+0x41)[0xb6445201]
    /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtCore.so.4(_ZN20QEventDispatcherGlib13processEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE+0x65)[0xb699e1f5]
    ...
    b6c19000-b770d000 r-xp 00000000 08:11 1346763 /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtGui.so.4.8.1
    b770d000-b772f000 r--p 00af3000 08:11 1346763 /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtGui.so.4.8.1
    b772f000-b7739000 rw-p 00b15000 08:11 1346763 /home/lukasz/QtSDK/Desktop/Qt/4.8.1/gcc/lib/libQtGui.so.4.8.1
    b7739000-b773e000 rw-p 00000000 00:00 0
    b773e000-b773f000 r-xp 00000000 00:00 0 [vdso]
    b773f000-b775f000 r-xp 00000000 08:11 3938325 /lib/i386-linux-gnu/ld-2.15.so
    b775f000-b7760000 r--p 0001f000 08:11 3938325 /lib/i386-linux-gnu/ld-2.15.so
    b7760000-b7761000 rw-p 00020000 08:11 3938325 /lib/i386-linux-gnu/ld-2.15.so
    bf852000-bf873000 rw-p 00000000 00:00 0 [stack]@

    What I checked:

    • when I move this code to main loop and there I run it in neverening loop all works fine, so I think I have problem with thread (do I need addiotional synchronization if only one thread is using this QUdpSocket?)

    • a moment after start of application when it crashes depends on msleep timeout. When there is no sleep then application crashes just after start. msleep(1) - crashes after few seconds, msleep(100) - don't crash a few minutes.

    Edit:
    Qt4.8.1, Ubuntu 12.04 LTS

    Do you know where can be my problem?


  • Moderators

    The short version:
    You shouldn't subclass QThread. Have a look at http://doc-snapshot.qt-project.org/4.8/qthread.html for an example of the proper approach.

    The long version:
    A QThread is not a thread. A QThread is an object that manages a new thread... and it manages that thread WITHOUT living inside that thread.

    Let's say you create your thConn object in the main thread. That thConn object, and its member variables (including the Client), live in the MAIN thread -- NOT the other thread.

    The only thing that goes into the other thread is the code inside run(). Here is your problem:

    @
    void thConn::run()
    {
    // All code below execute in the other thread

    while(abort == false)
    {
        msleep(1);
        if (client.Connected == true)
        {
            // Code from the other thread is trying to update the client,
            // which lives in the main thread
            client.Update(); 
        }
    }
    

    }
    @

    In summary, your socket is processing data in the main thread, but your program tries to update it from the other thread, causing memory corruption.



  • Hello

    Thanks for quick reply and nice explanation (I am looking for an option 'give a beer' but I haven't found it yet :)). But I don't have good news. I maked exactly that what is in this doc. My class don't subclass QThread now. Communication bases only on signals and slots (doWork instead of QThread::run).

    @Dialog::Dialog(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::Dialog)
    {
    ui->setupUi(this);

    Conn *conn = new Conn;
    QThread *connThread = new QThread(this);
    
    connect(connThread, SIGNAL(started()), conn, SLOT(doWork()));
    connect(connThread, SIGNAL(finished()), conn, SLOT(deleteLater()));
    conn->moveToThread(connThread);
    
    // Starts an event loop, and emits workerThread->started()
    connThread->start();
    

    }@

    Application crashes usually a few seconds after start. I am really confused now. Threading mechanism in Qt is a bit strange for me. Btw, another question. When my class didn't inherit from QThread I can't use protected sleep in doWork(). What can I use instead of sleep (exclude mutex)?

    I have to look on blocking socket examples. Mayble I will find something interesting.

    Edit:
    I found that:
    http://lists.trolltech.com/qt-interest/2008-11/thread00342-0.html
    I have identical problem because of waitForReadyRead function. Ok now I am closer to find the solution.

    I checked this assert:
    Q_ASSERT(QThread::currentThread() == socket.thread());
    before waitForReadyRead and now I really know what you wanted to explain me :). Now I see socket's thread is main thread not conn thread. So what is the method to 'move' or 'run' this socket from my thread? I think local declaration in run method will solve that problem but is there other way?


  • Moderators

    Haha, you're welcome! No beer required ;)

    Qt provides a few different ways for programmers to write asynchronous/parallel code. QThread is only one of them -- you can see other methods at http://qt-project.org/doc/qt-4.8/thread-basics.html#when-to-use-alternatives-to-threads

    (Warning: That page is a bit old, and it contains examples that subclass QThread. Ignore those examples, but the rest of the page is quite good)

    I don't have experience with programming for UDP communication, so I don't know what's the best method for you, sorry. But, you can try event-driven programming instead of waiting in threads. For example, connect the QUdpSocket::readyRead() signal to a slot that reads the socket's data -- this lets you use the QUdpSocket in your main thread, and your program won't get blocked. The slot will only run when data is available for reading.

    Qt is an event-driven framework, which uses signals and slots a lot. sleep() isn't used much in event-driven programs; I like to use QTimer instead of sleep() (actually, I've never used sleep() in Qt!)



  • I agree with you about using readyRead() signal. I tried it first and all is ok then. But event programming in communication (for ex. QUdpSocket) is very uncomfortable and causes a code is unclear. So, I agree signal-slot mechanism is nice feature but not always.

    Just imagine this situation. I have to send few commands to the device.
    client.cmd1();
    client.cmd2();
    client.cmd3();
    All commands have to be sended in order and each function have to wait on successful (or not) previous data transaction. If I do this without blocking I have to create state machine, I don't want.

    Other problem.
    If I create slot for example like that:
    @void conn::sendCmd(void)
    {
    client->cmd1(); /* client is member of conn struct, created on begin of run method*/
    }@
    and connect it with button click signal then this code is not safe because it will not be processed in run function so it will be processed outside my conn thread (UdpSocket's parent is this conn thread).

    I am really angry on this behavior. I see only one way - remake my code and use nonblocking UdpSocket.


  • Moderators

    You're right; event-driven programming doesn't show the order of execution very well. When I write event-driven programs, I combine by source code with a good diagram to make the code easy to understand (e.g. sequence diagram, or communications diagram)

    Also, have you looked at QRunnable, or QtConcurrent? Can they do what you want?

    Below, I'll try to show you some possible solutions to your example problems. I'm not sure if they are scalable to suit your real problem though.

    [quote author="nibbit" date="1352994748"]Just imagine this situation. I have to send few commands to the device.
    client.cmd1();
    client.cmd2();
    client.cmd3();
    All commands have to be sended in order and each function have to wait on successful (or not) previous data transaction. If I do this without blocking I have to create state machine, I don't want.[/quote]You can put the 3 commands in a function in the correct order, and then run that function in a separate thread:
    @
    void myCommands()
    {
    client.cmd1();
    client.cmd2();
    client.cmd3();
    }

    ...

    QtConcurrent::run(myCommands);
    @

    [quote author="nibbit" date="1352994748"]Other problem.
    If I create slot for example like that:
    @void conn::sendCmd(void)
    {
    client->cmd1(); /* client is member of conn struct, created on begin of run method*/
    }@
    and connect it with button click signal then this code is not safe because it will not be processed in run function so it will be processed outside my conn thread (UdpSocket's parent is this conn thread).

    I am really angry on this behavior. I see only one way - remake my code and use nonblocking UdpSocket.
    [/quote]You said "run function" and "UdpSocket's parent is this conn thread", which sounds like you're subclassing QThread. Are you doing that? That's unsafe.

    But anyway, if your conn object lives in the other thread, then your code is safe.

    Signals and slots are designed to be safe across threads. Here's an example:

    @
    int main(...)
    {
    ...

    Window       gui        = new Window();
    Communicator comm       = new Communicator();
    QThread      commThread = new QThread();
    
    comm->moveToThread(commThread);
    
    // The transmit() slot will run in the commThread
    QObject::connect(&gui,  SIGNAL(commandGiven(QByteArray)),
                     &comm, SLOT(transmit(QByteArray)));
    
    // The displayReceivedData() slot will run in the main thread
    QObject::connect(&comm, SIGNAL(dataReceived(QByteArray)),
                     &gui,  SLOT(displayReceivedData(QByteArray)));
    
    ...
    

    }
    @


  • Moderators

    Implementations for my last example:
    @
    // Implementation of the GUI
    class Window : public QWidget
    {
    signals:
    void commandGiven(QByteArray command);

    public slots:
    void displayReceivedData(QByteArray data) {
    screen->append(QString(data));
    }

    public:
    Window(QWidget *parent = 0) : QWidget(parent) {
    ...
    connect(button, SIGNAL(clicked()), this, SLOT(prepareCommand()));
    }

    private slots:
    void prepareCommand() {
    QByteArray command = commandLine->text().toAscii();
    emit commandGiven(command);
    }

    private:
    QLineEdit *commandLine; // The user types in ASCII commands here
    QPushButton *button;
    QTextEdit *screen;
    Q_OBJECT
    };
    @
    @
    // The Communicator class is a wrapper for a QUdpSocket,
    // that lives in another thread.
    //
    // It can also be extended to contain data-processing code,
    // to pre/post-process data outside the main thread.
    class Communicator : public QObject
    {
    signals:
    void dataReceived(QByteArray data);

    public slots:
    void transmit(QByteArray data) {
    socket->writeDatagram(...);
    }

    public:
    Communicator(QObject *parent = 0) : QObject(parent) {
    // Make the QUdpSocket a child QObject.
    // It will be moved to the other thread when its parent is moved.
    this->socket = new QUdpSocket(this);

        connect(socket, SIGNAL(readyRead()), this, slot(processIncomingData()));
    }
    

    private slots:
    void processIncomingData() {
    socket->readDatagram(...);

        ...
    
        emit dataReceived(data);
    }
    

    private:
    QUdpSocket *socket;
    Q_OBJECT
    };
    @



  • bq. You said “run function” and “UdpSocket’s parent is this conn thread”, which sounds like you’re subclassing QThread. Are you doing that? That’s unsafe.
    But anyway, if your conn object lives in the other thread, then your code is safe.

    Yes. I know that subclassing QTHread is not recommended. But I was trying both method and there is no difference in the program activity. Unfortunatelly I can't use QRunnable and QConcurrent.

    But I wrote one nice working method (I have no memory dumps etc, but I am not sure is it safe). As I said earlier, main problem is with this socket blocking function:
    @ if(socket.waitForReadyRead(200) == false) {
    return;
    }@

    I change this code to:
    @ QTime time;
    time.start();
    while (!socket.hasPendingDatagrams() && time.elapsed() < 100);
    if (!socket.hasPendingDatagrams())
    {
    return;
    }@

    Now all is working great and really I am not sure why. In this case I am using my own blocking method instead of socket blocking method. Now I can use my socket even in the way as I want :).


  • Moderators

    [quote author="nibbit" date="1353074432"]Yes. I know that subclassing QTHread is not recommended. But I was trying both method and there is no difference in the program activity.[/quote]Well, programming rules are not fixed. As long as you know what you're doing, it's ok to break them during appropriate times :)

    [quote author="nibbit" date="1353074432"]Now all is working great and really I am not sure why. In this case I am using my own blocking method instead of socket blocking method. Now I can use my socket even in the way as I want :). [/quote]Hmm... it has become a hidden bug. Your code is still unsafe, and it can still crash if you deploy the program on a different computer, or if other parts of your code get updated. It would be wise to protect your program with mutexes, if you want to do it this way.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.