0

I have the exact same problem as described QThread won't stop / does not process a signal but with a twist. I cannot get it to work even with QCoreApplication::processEvents() to my while loop. I followed this blog post and ended up like as follows:

MediaController.cpp

void MediaController::buttonPlayClicked(bool checked)
{
    if (checked)
    {
        m_loopThread = new QThread;
        m_playPauseworker = new PlayPauseWorker;
        m_playPauseworker->moveToThread(m_loopThread);
        connect(m_playPauseworker, &PlayPauseWorker::dataReady, this, &MediaController::onDataReady);
        connect(m_loopThread, &QThread::started, m_playPauseworker, &PlayPauseWorker::process);

        connect(m_playPauseworker, &PlayPauseWorker::finished, m_loopThread, &QThread::quit); // <-- never works
       
        connect(m_playPauseworker, &PlayPauseWorker::finished, m_playPauseworker, &PlayPauseWorker::deleteLater);
        connect(m_loopThread, &QThread::finished, m_loopThread, &QThread::deleteLater);
        m_loopThread->start();
    }
    else
    {
        m_loopThread->requestInterruption();
    }
}

The above slot is called every time play/pause checkable button is clicked. Thread and worker are created on the main thread.

PlayPauseWorker.cpp

void PlayPauseWorker::process()
{
    while (!QThread::currentThread()->isInterruptionRequested())
    {
            // heavy computations

            emit dataReady(std::tuple<QImage, QImage, QImage>>); // <-- works!
            
            QCoreApplication::processEvents(); // <-- doesn't help with quit()
    }           

    emit finished(); // <-- emits the signal but quit() not called
}

On further processing, when i access wait() to see if my thread has exited, it never returns.

In addition to quit() slot never called, i noticed my GUI becomes very choppy when the thread is running. I sometimes can/cannot click on other buttons or play with UI. I went through a lot of SO posts but couldn't figure out a way to cleanly exit the thread whenever i need to. Not sure where am i going wrong.

MarKS
  • 494
  • 6
  • 22
  • To be clear, your GUI is non-responsive once the play button is clicked? How are you able to call `requestInterruption` if the GUI is non-responsive? – m7913d Aug 17 '20 at 11:47
  • Does it work if you comment out `connect(m_playPauseworker, &PlayPauseWorker::dataReady, this, &MediaController::onDataReady);`? – m7913d Aug 17 '20 at 11:49
  • @m7913d about GUI non-responsive i edited my question. Commenting out the connect statement doesn't work. – MarKS Aug 17 '20 at 12:04
  • I've been unable to reproduce this one on my own; I put together a test (with no UI) that has a couple classes behaving similarly to your `MediaController` and `PlayPauseWorker`, and am seeing the thread exit. I'm unsure if heavy computation could screw up the signal/slot handling, but I don't believe so. Please check that you are calling `wait()` on the correct thread. I am not using `wait()` in my test, but rather `connect(thread, &QObject::destroyed, [] { std::cout << "destroyed" << std::endl; });` to see when thread is destroyed; maybe try that? – Ipiano Aug 17 '20 at 14:29
  • I also noticed in my test that `QCoreApplication::processEvents()` is not actually needed for the event to propagate to the main thread. As for the choppiness, you may be able to alleviate some of that by setting the thread priority of your side thread lower? – Ipiano Aug 17 '20 at 14:33
  • @Ipiano based on your and @m7913d suggestion when i comment out `connect(m_playPauseworker, &PlayPauseWorker::dataReady, this, &MediaController::onDataReady)` i can interrupt the thread and i could see `destroyed`message. My data in `emit dataReady(std::tuple)`. Maybe there is something wrong with this signal? – MarKS Aug 17 '20 at 15:18
  • Did you register your custom type? You need to register a custom type with qRegisterMetaType in order to use it in signals/slots, e.g. https://stackoverflow.com/questions/26364024/how-to-use-template-types-as-slot-and-signal-parameters-in-multiple-threads – talamaki Aug 17 '20 at 15:40
  • @talamaki yes i did register my custom type in ctor of `MediaController` and i could see `onDataReady` slot being called. – MarKS Aug 17 '20 at 15:52
  • @MarKS How big of images are you working with here? Emitting data cross-threads is going to copy the data at minimum once, more if you're using pass-by-value for your parameters in either the signal or slot. If the data is large, you may be blocking something up with copying those images. Maybe allocate them on the heap and pass a `shared_ptr` type with the data through the signal? Please try uncommenting the dataReady but still having the log message on destroyed. It'd be good to know if commenting out dataReady actually 'solved' the issue – Ipiano Aug 17 '20 at 15:53
  • @Ipiano My 3 images are RGB, Amplitude and Depth image of 640x480 size. I read somewhere passing-by-ref is a bad idea across threads in Qt so i am passing them by value which i hate to do so. I will try to allocate the images on the heap and pass the ptr in signal and see if it makes any difference – MarKS Aug 17 '20 at 15:57
  • You should never pass by reference through a signal/slot; however, passing by const reference is fine. See https://www.embeddeduse.com/2013/06/29/copied-or-not-copied-arguments-signals-slots/. TLDR; passing by value copies once or twice, depending on if both signal and slot do it, passing cross a thread adds an extra copy. Worst case, you're doing 3 copies. If you switch to passing a `std::shared_ptr>` then the share pointer object gets copied, not the QImage data itself. – Ipiano Aug 17 '20 at 16:00
  • I completely agree with you. Let me try this and will get back to you. Thank you for all the amazing tips. – MarKS Aug 17 '20 at 16:03
  • Note, I might be entirely wrong about how much data is copied because Qt does a bunch of data sharing magic behind the scenes, but I don't know enough about it to rely on it – Ipiano Aug 17 '20 at 16:03
  • It's also worth mentioning that technically the amount of data copied really shouldn't matter here... if this makes the issue go away, I think there's likely a race condition somewhere. While it would be good to not pass by copying a bunch of times, that may not actually be the issue – Ipiano Aug 17 '20 at 16:07
  • Thought: How much work is your slot connected to dataReady doing? If it's doing a significant amount of post-processing on the images that could be related and explain the stuttering of the main thread – Ipiano Aug 17 '20 at 16:20
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/219990/discussion-between-marks-and-ipiano). – MarKS Aug 17 '20 at 16:24
  • If/when your problem is solved, you may consider writing an answer yourself, although the question is not clearly defined (a [mcve] is missing). It may be useful to others with a similar question. – m7913d Aug 17 '20 at 20:07

1 Answers1

0

After trying out a lot of suggestions i still couldn't figure out why emit finished() doesn't quit the thread. So, as a last resort, i used this->thread->quit() to achieve the same.

But thanks to all the commenters here, i narrowed down on laggy/choppy gui issue. My emit data(std::tuple<QImage, QImage, QImage>>) was setting pixmap on QGraphicsScene on a loop without removing the already set pixmap on the scene leading to huge memory leak and ultimately crash.

MarKS
  • 494
  • 6
  • 22