Waiting slots to be executed before quitting

Question

I've a thread that read datas

class MyThread: QThread
{
  ...
}

void MyThread::run ()
{
  uint8_t* buffer; // in my real code, it's a ring, so there is not read during write
  // ...

  while (true)
  {
    if (isInterruptionRequested())
      return;
    USB_READ(buffer);
    emit newData(buffer);
  }
}

In my UI Class I have:

connect(this, &UIClass::newData, m_thread, &MyThread::newData);

// ...

void newData(uint8_t* data)
{
  // Process data
}

void UIClass::closeEvent(QCloseEvent *event)
{
   disconnect(this, &UIClass::newData, m_thread, &MyThread::newData);
   m_thread->requestInterruption();
   m_thread->wait();
}

The problem with that if, when I click on "close", the thread is destroyed that cause the pointer data to be invalid. The signal newData is sometimes called that cause my function to work with invalid pointer and segfault. How to be sure that is not gonna happend ?

For now, I use a std::this_thread::sleep_for() with an arbitrary delay, it works, but I not find this very beautiful

That I have in my mind :
- disconnect the signal
- wait for the pendings signals to be executed
- exit

Benjamin T · Accepted Answer · 2017-01-16T14:56:15.427

The problem is that you send a pointer from one thread to another without ensuring the pointer stays valid.

You have multiple choices to solve this. Either use QSharedPointer (or similar utilities from the stl) to hold your data, doing so will ensure your pointer will remain valid (or provide you a way to detect when the pointer becomes invalid if you also use QWeakPointer). Or you could make use of QByteArray to pass the data, but this will make a copy.

Example 1

void MyThread::run ()
{
  QSharedPointer<uint8_t> buffer (new uint8_t[N]()); // Do not delete[], QSharedPointer will handle it
  ...

  emit newData(buffer);

}
void newData(QSharedPointer<uint8_t> data)
{
  // data is always valid
  // Process data
}

Example 2

void MyThread::run ()
{
  QSharedPointer<uint8_t> buffer (new uint8_t[N]());
  ...

  emit newData(buffer);

}
void newData(QWeakPointer<uint8_t> data)
{
  // data might not be valid but we can check
  QSharedPointer<uint8_t> buffer (data);
  if (!buffer)
      return;
  // Process data
}

Example 3

void MyThread::run ()
{
  uint8_t[N] buffer;
  ...

  emit newData(QByteArray(buffer, size));

}
void newData(QByteArray data)
{
  // data is valid
  // Process data
}

Thanks you, I tried the first one and it's working. What if I don't want to use the dynamic allocation but I want to use the adresse : uint8_t buffer[size] ..... emit &buffer[0]. What can I do in this case, because I don't want my pointer to be deleted (it cannot) — Titouan56, Jan 16 '17 at 14:48
If you allocate the buffer on the stack (and not dynamically on the heap) you cannot guarantee that it will be valid when the slot is called, because you cannot guarantee that the buffer will still be in the scope in the other thread. The only solution would be to make a copy like in the 3rd example. — Benjamin T, Jan 16 '17 at 14:55

score 0 · Answer 2 · answered Jan 16 '17 at 16:25

All you need to do is for the thread to outlive the user interface. That's rather easy:

class MyThread : public QThread
{
  Q_OBJECT
  RingBuffer buffer;
public:
  void run() override;
  ~MyThread() {
     requestInterruption();
     quit();
     wait();
  }
  Q_SIGNAL newData(RingBuffer *);
};

int main(int argc, char **argv) {
  QApplication app{argc, argv};
  MyThread thread;
  thread.start();
  UIClass ui;
  connect(&thread, &MyThread::newData, &ui, &UIClass::newData);
  return app.exec();
}

Waiting slots to be executed before quitting

2 Answers2

Linked