Segmentation fault or overflow when streaming from a QSerialPort

QtExchange

I was encountered with this strange kind of runtime error. I have a Qt-SerialPort application that connects the workstation with a sensor/actor device. It runs perfectly fine when I'm in polling mode (i.e.: start a new message for each request - for instance query every ms the sensor values). The device provides a streaming mode aswell, which shall increase the update rate because of the less control procedures, since it doesn't even stop sending at all.

The problem

When I'm into this mode it takes only seconds until it crashes with the only true Indication: "Segmenation fault" (sometimes it takes more, sometimes less time before the error jacks in). Formerly I utilized the qextserialport where I had no problem at all, so I suspect that I'm doing something wrong with the Qt5 library(s).

The solution approach

When GDBing it always crashes somewhere else, often at a QByteArray assertion but always different, that's why I'm not sure if this is eventually missleading. I've read that this possibly might be caused of a buffer overflow and that I shall provide sufficient space with the setReadBufferSize(). Unfortunately the error is persistent, whether I set it to "0", the max value of the demandend qint64, or somewhere in between, clearing the buffer also didn't help.

Here is my .h:

class WiredDev : public QSerialPort
{   
    Q_OBJECT
    static int timeout_ms;
public:
    WiredDev(QString portName);

    QByteArray readDeviceData(int timeout_ms=1000);
    bool sendData(QByteArray msg, int curTimeout_ms = timeout_ms);
    bool isOnline();

public slots:
    void initCommunication();
    void receiveMessage();
    void transmitMessage();

signals:
    void messageReadyToSend(void);

private:
 /* some variables */
}

Concerning parts of the .cpp

WiredDev::WiredDev(QString portName)
{
    this->setPortName(portName);
    this->setParity(QSerialPort::NoParity);
    this->setFlowControl(QSerialPort::NoFlowControl);
    this->setDataBits(QSerialPort::Data8);
    this->setStopBits(QSerialPort::OneStop);
    this->setBaudRate(QSerialPort::Baud115200);
  
    this->setReadBufferSize(std::numeric_limits<qint64>::max()); 
}

sending

void WiredDev::transmitMessage()
{
    mutex.lock();
    if (this->isWritable())
    {
        this->clear(); // clears the buffer for both incoming/outgoing msg (?)
        receivedMessage.clear();
        this->write(transmitMessage);
        this->clear(QSerialPort::Input);
        transmitMessage.clear();
    }
    mutex.unlock();
}

receiving

void WiredDev::receiveMessage()
{
    mutex.lock();
    if (this->isOnline() && this->isReadable())
    {
        curMsg = this->readAll();
        this->clear(QSerialPort::Output);
        receivedMessage.append(curMsg);
    }
    mutex.unlock();
}

Init

void WiredDev::initCommunication()
{
    if(this->open(QSerialPort::ReadWrite))
    {
        QThread::msleep(250);

        connect(this,SIGNAL(readyRead()),this,SLOT(receiveMessage()));
        connect(this,SIGNAL(messageReadyToSend()),this,SLOT(transmitMessage()));
    }
}

As it is part of a bigger program, here's how I create the instance:

wiredThread = new QThread;
wiredDev = new wiredDev( this->portname);
this->wiredDev->moveToThread(wiredThread);
QObject::connect(wiredThread,SIGNAL(started()),wiredDev,SLOT(initCommunication()));

Please excuse for not providing the whole code for I'm not allowed by the forum to do so, but I guess these are the concerning parts.

Thanks for any hints.
Kind Regards

Todd Morehouse

I've never tried using QSerialPort and QThread together. Do you need this? Can you try without it and see if you still get the same error?

I never initialize the serial port like that, did you triple check the settings? I've always had problems with it.
Try QSerialPortInfo if you haven't. You can initialize with just the portname and QSerialPortInfo will grab the rest of the information for you.

Try something like this.

//Grab all available ports into QSerialPortInfo object
QList<QSerialPortInfo> portInfoList = QSerialPortInfo::availablePorts();

//Iterate through each port found for the right port
for(int i = 0; i < portInfoList.length(); i++){
	//Check if the port info is for the right port
	if (portInfoList.at(i) == "Insert Port Name Here")
		this.setPort(portIfoList.at(i));//If so set the port with that info
}

//The rest of your port initialization.

I just quickly typed that up in notepad at work, so I probably have a few mistakes (hopefully not a lot), but that gives you an idea on how it should work.

Just trying to check over the first two things I would look at.

Hope it helps!

References,

kuzulis

Don't use threads, you doing it wrong (check your thread's identifiers, check on warning messages from console)
qextserialport is not qtserialport, and it works differently

QtExchange

thanks for your reply

@Todd-Morehouse
I don't have any connection/communication problems. It's just he's having an overflow after a few seconds and crashes then. I.e.: everything works as expected until the segfault

@kuzulis
obviously something is wrong, but that didn't help to identify the error

Edit: I only receive this when in streaming mode (when endless messages arrive at the serialport), it works like charm when polling - the naive approach would be clearing the input/output buffer but that didn't solve it

Edit2: I have absolutly no problem whether in streaming nor polling when I'm using the exact same implementation for wireless communication (QTcpServer/QTcpSocket) - so I guess the error must be on QSerialPort's side, as the thread runs harmlessly with the wireless connection

QtExchange

Update: it turns out when increasing the pirority and stacksize it doesn't crash anymore (at least for the observed time). However I'm not content with this solution as if there was a memory leak it actually still should be persistent, just at a later point now.

wiredThread->setPriority(QThread::TimeCriticalPriority);
wiredThread->setStackSize(10000000);

QtExchange

This post is deleted!

kshegunov

@QtExchange said in Segmentation fault or overflow when streaming from a QSerialPort:

wiredThread->setPriority(QThread::TimeCriticalPriority);
wiredThread->setStackSize(10000000);

This really doesn't mean much. On Linux you don't even have thread priorities. My suggestion is to clean up your code - remove the unnecessary mutexes, the clears and such, and just use the serial port as it was intended. For example:

void WiredDev::receiveMessage()
{
    // The port has already signaled it's ready to be read
    receivedMessage.append(readAll()); 
}

I have a project deployed with QSerialPort (a daemon) that's been running thousands of hours without issue, so you should search the problem in your code. Additionally, @kuzulis is correct - there's no gain here in threading the serial port.

QtExchange

Update:

I believe the error is because of a simultaneously memory access (on receivedMessage), between the "receiving part" of the QSerialPort and the "processing part" of my library.

Since I'm using additionaly mutexs for variables/containers that are used on both sides (which actually never should interfere) the error didn't occur anymore. However since I implemented the QMutexes my program became very slow (like 1-2 s between an update), so i have to look how to accelerate that part.

So either the problem is solved because of the mutexes, or the problem is just suspended because of the lower processing speed. I'm investigating

kshegunov

@QtExchange said in Segmentation fault or overflow when streaming from a QSerialPort:

Since I'm using additionaly mutexs for variables/containers that are used on both sides (which actually never should interfere) the error didn't occur anymore. However since I implemented the QMutexes my program became very slow (like 1-2 s between an update), so i have to look how to accelerate that part.

Well you shouldn't need any mutexes nor to share the data (in most of the cases), you should transfer whatever it is you want between the threads with the help of the signal-slot mechanism.

Rondog

The QMutex should not be the problem but what you do between the lock() and unlock() commands could easily cause issues with performance as it can block other threads.

The mutex should only be used to prevent any shared data from being simultaneously read and written. In your example you lock the mutex around the Tx, Rx functions. I know the QSerialPort has buffers and all that but you are blocking while the mutex is locked and serial is very slow which is likely why you have a 1-2 s delay between updates (?).

If you are using two buffers (looks like transmitMessage and receivedMessage are likely QByteArrays used as buffers) you should have read and write functions to get or set the data in these buffers as opposed to locking all the functions that might access the data in these variables.

QByteArray Get_TX_Buffer(void)
{
    QByteArray tx_buffer;
    mutex.lock();
    tx_buffer = transmitMessage;
    mutex.unlock();
    return tx_buffer;
}

void Set_RX_Buffer(const QByteArray &rx_buffer)
{
    mutex.lock();
    receivedMessage.append(rx_buffer);
    mutex.unlock();
}

You might be better off not using a QThread (?). I have done serial communications using a QThread as it was more appropriate in my case but it all depends on the situation I suppose.

QtExchange

@kshegunov
The trick is one QByteArray receives the data (on signal slot basis), where the other container (which is just a copy of the receiver-QByteArray) is parsing the received Data, as the symbols come in a random order. I could think of having an extra signal-slot that exchanges the data between both containers whenever it is demanded, but it sounds like I have to double check if I'm actually dealing with the most current data.

@Rondog There's an extra mutex for the device access, and another one for the shared message container. I'm using QThread because of maintenance reasons, where multiple different device communications (Serial, TCP/IP ...) shall be as generic as possible and because I'm dealing with QElapsedTimer, that needs the thread.

Unfortunately I can't reach the code in the next week, I'll keep it updated on new events.

kshegunov

@QtExchange said in Segmentation fault or overflow when streaming from a QSerialPort:

The trick is one QByteArray receives the data (on signal slot basis)

QByteArray can receive nothing, it's an array of bytes - a container, nothing more, nothing less.

where the other container (which is just a copy of the receiver-QByteArray) is parsing the received Data, as the symbols come in a random order.

??!
How can you parse something that doesn't have any order? And where is this other container?

I could think of having an extra signal-slot that exchanges the data between both containers whenever it is demanded, but it sounds like I have to double check if I'm actually dealing with the most current data.

I don't follow. Why would be this even needed ...?

There's an extra mutex for the device access, and another one for the shared message container.

Qt imposes thread affinity on QObject instances, you don't need a mutex for device access, and you shouldn't share the message container.

and because I'm dealing with QElapsedTimer, that needs the thread.

In what universe QElapsedTimer needs a thread ...?! It's like saying that an integer needs a thread ... I don't get it.