Calling QLabel::setText causes a write acces violation
-
Hi,
It would help to see the code of your worker thread. If you are emitting a signal of MainWindow from another thread, then it sould be move to that thread to have the right thread affinity (see "QObject::moveToThread()":http://qt-project.org/doc/qt-5/qobject.html#moveToThread). Otherwise, the signal will be processed directly and may cause concurrent accesses.
You said that MainWindow::on_updateRateChanged() is called in the main thread, so the affinity looks right, unless your thread initialization is wrong. Again, it would help to see the code.
-
[quote author="tilsitt" date="1414136773"]Hi,
It would help to see the code of your worker thread. If you are emitting a signal of MainWindow from another thread, then it sould be move to that thread to have the right thread affinity {...}[/quote]
It would seem it has indeed something to do with my worker thread. I come to this conclusion since the application only crashed when my communication handler class emits a sendHeartbeat signal to my worker thread.
My application is set up in the following manner:
CCommHandler class, provides a way of easily sending write/read and heartbeat serial messages to my serial device. It has CCommWorker as a member which is pushed onto a separate thread for blocking calls to the serial device. (see below)
@class CCommHandler : public QObject
{
Q_OBJECTpublic:
CCommHandler(void);
~CCommHandler(void);
void WriteRegister(uint8_t Address, uint32_t Data);
void ReadRegister(uint8_t Address, uint32_t* Data);public slots:
void on_refreshComPorts(void);
void on_selectedComPortChanged(QString Port);
void on_selectedBaudrateChanged(QString Baudrate);
void on_connect(void);
void on_disconnect(void);private slots:
void on_frameReceived(RSMC::EFrameResult Result, uint32_t Data);
void on_connected(void);
void on_disconnected(void);
void on_heartbeatUpdate(void);
void on_controllerConnected(unsigned int controllerNum);
void on_controllerDisconnected(unsigned int controllerNum);signals:
void ComPortsChanged(QStringList CommPorts);
void Connected(void);
void Disconnected(void);
void FrameReceived(RSMC::EFrameResult Result, uint32_t Data);void SetComPort(QString Port);
void SetBaudRate(QString Baud);
void Connect(void);
void Disconnect(void);
void SendHeartBeatSignal(float Roll, float Pitch, float Yaw, float Throttle, uint32_t* Data);
void WriteRegisterSignal(uint8_t Address, uint32_t Data);
void ReadRegisterSignal(uint8_t Address, uint32_t* Data);void ControllerConnected(unsigned int controllerNum);
void ControllerDisconected(unsigned int controllerNum);
void HeartBeatFrequencyChanged(float Frequency);protected:
void run();private:
bool mConnected;
float mPrevFreq;
QStringList mCommPorts;
QThread mWorkerThread;
QTimer mHeartbeatTimer;
CCommWorker* pCommWorker;
SimpleXbox360Controller* pController;
SimpleXbox360Controller::InputState mControllerInputState;
};@@CCommHandler::CCommHandler(void)
: mConnected(false)
, mPrevFreq(0.0f)
, pCommWorker(new CCommWorker)
, pController(new SimpleXbox360Controller(0,7849,8689,30,this))
{
mHeartbeatTimer.setTimerType(Qt::PreciseTimer);connect(&mWorkerThread, &QThread::finished, pCommWorker, &QObject::deleteLater);
connect(&mHeartbeatTimer, &QTimer::timeout, this, &CCommHandler::on_heartbeatUpdate);connect(pCommWorker, &CCommWorker::Connected, this, &CCommHandler::on_connected);
connect(pCommWorker, &CCommWorker::Disconnected, this, &CCommHandler::on_disconnected);
connect(pController, &SimpleXbox360Controller::controllerConnected, this, &CCommHandler::on_controllerConnected);
connect(pController, &SimpleXbox360Controller::controllerDisconnected, this, &CCommHandler::on_controllerDisconnected);connect(this, &CCommHandler::Connect, pCommWorker, &CCommWorker::ConnectSerial);
connect(this, &CCommHandler::Disconnect, pCommWorker, &CCommWorker::DisconnectSerial);
connect(this, &CCommHandler::SetBaudRate, pCommWorker, &CCommWorker::SetBaudRate);
connect(this, &CCommHandler::SetComPort, pCommWorker, &CCommWorker::SetComPort);connect(this, &CCommHandler::SendHeartBeatSignal, pCommWorker, &CCommWorker::SendHeartBeat);
connect(this, &CCommHandler::WriteRegisterSignal, pCommWorker, &CCommWorker::WriteRegister);
connect(this, &CCommHandler::ReadRegisterSignal, pCommWorker, &CCommWorker::ReadRegister);
connect(pCommWorker, &CCommWorker::FrameReceived, this, &CCommHandler::on_frameReceived);pCommWorker->moveToThread(&mWorkerThread);
mWorkerThread.start(QThread::TimeCriticalPriority);
mHeartbeatTimer.start(HeartbeatInterval);
}CCommHandler::~CCommHandler(void)
{
mHeartbeatTimer.stop();
mWorkerThread.quit();
mWorkerThread.wait();
delete pController;
delete pCommWorker;
}
void CCommHandler::on_controllerConnected(unsigned int controllerNum)
{
emit ControllerConnected(controllerNum);
}void CCommHandler::on_controllerDisconnected(unsigned int controllerNum)
{
emit ControllerDisconected(controllerNum);
}void CCommHandler::on_heartbeatUpdate(void)
{
pController->update();
if(mConnected)
{
uint32_t Data[3]; // Dummy
mControllerInputState = pController->getCurrentState();
float Roll = mControllerInputState.leftThumbX;
float Pitch = mControllerInputState.leftThumbY;
float Yaw = mControllerInputState.rightThumbX;
float Throttle = mControllerInputState.rightTrigger;
emit SendHeartBeatSignal(Roll, Pitch, Yaw, Throttle, Data); // When this line is commented the application does not crash anymore
}
float currFreq = pCommWorker->GetHeartBeatFrequency();
if(mPrevFreq - currFreq >= 1.0f ||
mPrevFreq - currFreq <= -1.0f)
{
mPrevFreq = currFreq;
qDebug() << "Emit thread id: " << QThread::currentThreadId();
emit HeartBeatFrequencyChanged(currFreq);
}}@
-
Commenting line 61 of CComHandler.cpp let's the problem disappear.
The signal on that line is connected to the CommWorker which consists out of the following:CComWorker.h
@class CCommWorker : public QObject
{Q_OBJECTpublic:
CCommWorker(void);
~CCommWorker(void);
float GetHeartBeatFrequency(void);public slots:
void SendHeartBeat(float Roll, float Pitch, float Yaw, float Throttle, uint32_t* Data);
void WriteRegister(uint8_t Address, uint32_t Data);
void ReadRegister(uint8_t Address, uint32_t* Data);
void SetComPort(QString Port);
void SetBaudRate(QString Baud);
void ConnectSerial(void);
void DisconnectSerial(void);signals:
void FrameReceived(RSMC::EFrameResult Result, uint32_t Data);
void Connected(void);
void Disconnected(void);private:
QSerialPort* pSerialPort;
TCallback<CCommWorker> mCallbackTx;
TCallback<CCommWorker> mCallbackRx;
TCallback<CCommWorker> mCallbackTime;
RSMC::CMultiFrameHandler* pMultiFrameHandler;
RSMC::SMultiFrameData mSendFrame;
RSMC::SMultiFrameData mRecvFrame;
QSettings mSettings;
QTime mTime;
QMutex mMutex;
QMutex mMutexLocalVar;
float mHeartbeatUpdateFrequency;
uint8_t Rx(void* Param1, void* Param2);
uint8_t Tx(void* Param1, void* Param2);
uint8_t GetTime(void* Param1, void* Param2);
void PrintSerialPortError(QSerialPort::SerialPortError Error);
};@CComWorker.cpp
@CCommWorker::CCommWorker(void)
: pSerialPort(new QSerialPort(this))
, mHeartbeatUpdateFrequency(0.0f)
{
pSerialPort->setBaudRate(EBaudRate9600);
mCallbackTx.SetCallback(this, &CCommWorker::Tx);
mCallbackRx.SetCallback(this, &CCommWorker::Rx);
mCallbackTime.SetCallback(this, &CCommWorker::GetTime);
pMultiFrameHandler = new RSMC::CMultiFrameHandler(true, &mCallbackRx, &mCallbackTx, &mCallbackTime);
}CCommWorker::~CCommWorker(void)
{
delete pSerialPort;
}uint8_t CCommWorker::Rx(void* Param1, void* Param2)
{
qApp->processEvents();
char* data = reinterpret_cast<char*>(Param1);
uint8_t length = reinterpret_cast<uint8_t>(Param2);
uint8_t bytesAvailable = pSerialPort->bytesAvailable();
if(length > bytesAvailable)
{
length = bytesAvailable;
}
uint8_t bytesRead = pSerialPort->read(data, length); // TODO: Check for available bytes, open port or does -1 catch this?if(bytesRead == -1) // An error occured
{
return 0;
}
else
{
return bytesRead;
}
}uint8_t CCommWorker::Tx(void* Param1, void* Param2)
{
qApp->processEvents();
char* data = reinterpret_cast<char*>(Param1);
uint8_t length = reinterpret_cast<uint8_t>(Param2);uint8_t bytesWritten = pSerialPort->write(data, length); // TODO: Check for open serial port needed or does -1 catch this?
pSerialPort->flush();
pSerialPort->waitForBytesWritten(-1);if(bytesWritten == -1) // An error occured
{
return 0;
}
else
{
return bytesWritten;
}
}uint8_t CCommWorker::GetTime(void* Param1, void*)
{
// TODO: implement
return 0;
}float CCommWorker::GetHeartBeatFrequency(void)
{
mMutexLocalVar.lock();
float result = mHeartbeatUpdateFrequency;
mMutexLocalVar.unlock();return result;
}void CCommWorker::SendHeartBeat(float Roll, float Pitch, float Yaw, float Throttle, uint32_t *Data)
{
if(mMutex.tryLock() == false)
{
return;
}
mSendFrame.commandHeader.Bit.cmdType = static_cast<uint8_t>(RSMC::ECmdTypeHb);
mSendFrame.commandHeader.Bit.replyWanted = 1;
union UData
{
float f;
uint32_t i;
};
uint32_t sendData[4];
UData d;
d.f = Roll;
sendData[0] = d.i;
d.f = Pitch;
sendData[1] = d.i;
d.f = Yaw;
sendData[2] = d.i;
d.f = Throttle;
sendData[3] = d.i;mSendFrame.data = sendData;
mSendFrame.dataLength = sizeof(sendData); // TODO remove this, not needed/used
RSMC::EFrameResult sendResult = pMultiFrameHandler->Push(mSendFrame);
while( sendResult == RSMC::EFrameResultPending)
{
sendResult = pMultiFrameHandler->Push(mSendFrame);
}if(sendResult != RSMC::EFrameResultOk)
{
emit FrameReceived(sendResult, mSendFrame.data[0]);
mMutex.unlock();
return;
}
mRecvFrame.data = Data;
mRecvFrame.dataLength = sizeof(Data);RSMC::EFrameResult recvResult = pMultiFrameHandler->Pull(&mRecvFrame);
while( recvResult == RSMC::EFrameResultPending)
{
recvResult = pMultiFrameHandler->Pull(&mRecvFrame);
}
emit FrameReceived(recvResult, mRecvFrame.data[0]);
mMutexLocalVar.lock();
mHeartbeatUpdateFrequency = 1000.0f/static_cast<float>(mTime.elapsed());
mMutexLocalVar.unlock();
mTime.restart();
mMutex.unlock();
}@ -
Hi,
@
if(sendResult != RSMC::EFrameResultOk)
{
emit FrameReceived(sendResult, mSendFrame.data[0]);
mMutex.unlock();
return;
}
@IIRC this might be the real problem.
You should rather take the data you need for your signal, unlock the mutex and only then emit your signal.Hope it helps
-
[quote author="SGaist" date="1414184112"]Hi,
IIRC this might be the real problem.
You should rather take the data you need for your signal, unlock the mutex and only then emit your signal.Hope it helps[/quote]
SGaist, thanks for your reply.
If I understand correctly you are saying I should switch line 3 and 4?
I tried this and it did not solve my problem. The code you quoted was never called anyway. But to be sure I commented the 2 lines where FrameReceived is emitted. It had no effect on my problem the application still crashed after some time. -
Not completely, with mSendFrame.data[0] you are still accessing a variable that should be protected.
After some time ? Do you mean that your application is running for a while and only then crash ?
-
[quote author="SGaist" date="1414275985"]
After some time ? Do you mean that your application is running for a while and only then crash ?[/quote]Yes my application runs fine without crashing at first. But when my program is connected to my Serial device and (more importantly )begins emitting the "SendHeartBeatSignal" the application still works for a random amount of time and then crashes.
-
Some race condition somewhere…
Tricky to debug. Maybe running your application through GammaRay might help find what goes wrong.
-
Yes probably a race condition.
I never heard of GammaRay I assume you mean "this":http://www.kdab.com/kdab-products/gammaray/I will give it a try when I have some time, seems that this bug will take quite some digging to find.
-
That's the one indeed.
Multithread bugs are pretty hard to find.
One thing you can also do, is to first ensure that everything works fine without any thread (QSerialPort works asynchronously) and only then move to the multithread paradigm.
-
That would be a good approach, I will take a look at it this weekend when I have some time.