segmentation fault in QTCPSocket::readAll()

Mephisky

Hi,
I encountered the segmentation fault issue when using QTCPSocket::readAll.
In GDB, it says:

#0  0x00007f38f4cc0060 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007f38f50804e4 in QRingBuffer::read(char*, long long) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#2  0x00007f38f510dcde in QIODevicePrivate::read(char*, long long, bool) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#3  0x00007f38f510e625 in QIODevice::readAll() () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#4  0x00000000004560c6 in NetworkCommander::onReadyRead (this=0x3c62b40) at ../commander/NetworkCommander.cpp:311
#5  0x00007f38f51f9d5a in QObject::event(QEvent*) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#6  0x00007f38f5daeb06 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#7  0x00007f38f5db8190 in QApplication::notify(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#8  0x00007f38f51cd93a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007f38f51d05b8 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#10 0x00007f38f5225f67 in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#11 0x00007f38f41ab17d in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00007f38f41ab400 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#13 0x00007f38f41ab4a3 in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007f38f5225565 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#15 0x00007f38f51cc4db in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#16 0x00007f38f51d4246 in QCoreApplication::exec() () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#17 0x0000000000427621 in main (argc=<optimized out>, argv=<optimized out>) at ../main.cpp:77

And i just do debugging with qtcreator, it would abort in the position of "QByteArray buff = m_socket->readAll();"

While i has checked the pointer of m_socket, and it works in "m_socket->bytesAvailable() ", i really does know what about it when readAll.

QPointer<QTcpSocket> m_socket;
QThread *m_networkThread;
QByteArray m_receiveBuffer;
QMutex m_bufferMutex;

NetworkCommander::NetworkCommander(QObject *parent)
    : QObject(parent)
    , m_socket(nullptr)
    , m_networkThread(new QThread(this))
    , m_nextRequestId(1)
{
    socketConnectRetryTimes = 0;
    m_socket = new QTcpSocket();
    m_socket->moveToThread(m_networkThread);

    connect(m_socket, &QTcpSocket::connected, this, &NetworkCommander::onConnected);
    connect(m_socket, &QTcpSocket::disconnected, this, &NetworkCommander::onDisconnected);
    connect(m_socket, &QTcpSocket::readyRead, this, &NetworkCommander::onReadyRead);

    m_networkThread->start(QThread::TimeCriticalPriority);
}
......

void NetworkCommander::onReadyRead()
{
    if (!m_socket)
        return;

    QByteArray tempBuffer;
    while (m_socket->bytesAvailable() > 0) {
        QByteArray buff = m_socket->readAll();
        if (buff.isEmpty())
            break;
        else
            tempBuffer.append(buff);
    }

    if (!tempBuffer.isEmpty()) {
        QMutexLocker locker(&m_bufferMutex);
        m_receiveBuffer.append(tempBuffer);
        // qDebug() << "Received" << tempBuffer.size() << "bytes, total buffer size:" << m_receiveBuffer.size();
    }

    processReceivedData();
}

when the process has been running some minutes, receiving data using the onReadyRead function. There would be the segmentation fault.

Christian Ehrlicher

m_socket lives in a different thread than from where you access it in NetworkCommander::onReadyRead() which is executed in the main thread.
Why use a thread here at all for the QTcpSocket? How/where do you connect to the server?
Don't use threads when you don't understand the basics https://doc.qt.io/archives/qt-5.15/thread-basics.html#qobject-and-threads

Mephisky

@Christian-Ehrlicher
Dear, you're right. the function onReadyRead() accesses m_socket which lives in other thread.
The NetworkCommander is constructed in the main qwidget. To avoid the m_socket reading/writing may block the UI thread, i move the m_socket into a new thread.
So, i think i shall move NetworkCommander totally into the new thread?, such as:

NetworkCommander::NetworkCommander(QObject *parent)
    : QObject(parent)
    , m_socket(nullptr)
    , m_networkThread(new QThread(this))
    , socketConnectRetryTimes(0)
    , m_nextRequestId(1)
{
    moveToThread(m_networkThread);

    m_socket = new QTcpSocket();
    m_socket->moveToThread(m_networkThread);

    connect(m_socket, &QTcpSocket::connected, this, &NetworkCommander::onConnected);
    connect(m_socket, &QTcpSocket::disconnected, this, &NetworkCommander::onDisconnected);
    connect(m_socket, &QTcpSocket::readyRead, this, &NetworkCommander::onReadyRead);

    m_networkThread->start(QThread::TimeCriticalPriority);
}

JonB

@Mephisky said in segmentation fault in QTCPSocket::readAll():

The NetworkCommander is constructed in the main qwidget. To avoid the m_socket reading/writing may block the UI thread, i move the m_socket into a new thread.
So, i think i shall move NetworkCommander totally into the new thread?

How would the socket reading/writing block the main thread? Have you understood how Qt QTcpSockets work and Qt's event/signal driven system? Why do you want a separate thread at all, especially with its complications? As @Christian-Ehrlicher wrote

Why use a thread here at all for the QTcpSocket?

Don't. At least start out with no separate thread and see how it goes.

Christian Ehrlicher

@Mephisky said in segmentation fault in QTCPSocket::readAll():

So, i think i shall move NetworkCommander totally into the new thread?,

Don't use a thread at all unless you have very big reaons to do so. I would guess you even call socket->connect() from the wrong thread and maybe others.

Mephisky

@JonB
I moved QTcpSocket out of a separate thread, and now it seems to be fine.
The reason why I use a separate thread is:
My software keeps sending TCP packets. I hope to implement a sync sending function in the NetworkCommander class, such as:

void NetworkCommander::sendData(const QByteArray &data, quint64 requestId)
{
    QMetaObject::invokeMethod(m_socket, [this, data, requestId]() {
        bool success = false;
        if (m_socket->state() == QAbstractSocket::ConnectedState)
        {
            qint64 result = m_socket->write(data);
            if (-1 != result)
            {
                m_socket->flush();
                success = true;
            }
        }
        emit writeCompleted(requestId, success);
    }, Qt::QueuedConnection);
}

This function might be called in business threads. Business threads blocking is acceptable.

If QTcpSocket is implemented in the main thread, I am worried that the "sendData" function will block the main thread.

Mephisky

@Christian-Ehrlicher
My connect functions is like below:

QMetaObject::invokeMethod(m_socket, [this]() {
        if (m_socket->state() != QAbstractSocket::ConnectedState) {
            m_socket->connectToHost("127.0.0.1", m_port);
        }
    }, Qt::QueuedConnection);

However, i write a sync function calling write() function like:

QMetaObject::invokeMethod(m_socket, [this, data, requestId]() {
        bool success = false;
        if (m_socket->state() == QAbstractSocket::ConnectedState)
        {
            qint64 result = m_socket->write(data);
            if (-1 != result)
            {
                m_socket->flush();
                success = true;
            }
        }
    }, Qt::QueuedConnection);

It is only used in business threads, which are allowing to be blocked.
I am doubting if it would cause main thread block?