More progress on Qt6.9 thread bug workaround.

Author: Mysticial

Common/Cpp/Concurrency/Backends/Thread_Qt.tpp

@@ -7,6 +7,7 @@
 #include <QCoreApplication>
 #include <QThread>
 #include "Common/Cpp/Containers/Pimpl.tpp"
+#include "Common/Cpp/Concurrency/Qt6.9ThreadBugWorkaround.h"
 #include "Common/Cpp/Concurrency/Thread.h"
 
 //#include <iostream>
@@ -22,7 +23,7 @@ namespace PokemonAutomation{
 struct Thread::Data : public QThread{
     Data(std::function<void()>&& function)
         : m_function(std::move(function))
-        , m_finished(false)
+//        , m_finished(false)
     {
         start();
     }
@@ -37,13 +38,34 @@ struct Thread::Data : public QThread{
         //  What the fuck?!?! This hangs on Qt6.9 even after run() returns!
         wait();
 
+#if 0
+#ifndef PA_ENABLE_QT_ADOPTION_WORKAROUND
+        wait();
+#else
+        while (m_finished.load(std::memory_order_acquire));
+        if (wait(1000)){
+            return;
+        }
+        std::cout << "QThread did not quit in a timely manner... It is likely hung..." << std::endl;
+        abort();
+#endif
+#endif
+
+        //
+        //  On further investigation, wait() hangs because the Qt6.9 thread
+        //  adoption bug affects not just std::thread, but its own as well.
+        //
+        //  When this QThread ends, the handle adopts one of the video backend
+        //  threads which do not join. Thus it hangs here.
+        //
+
 //        cout << "finished: " << this << endl;
     }
 
     virtual void run() override{
 //        cout << "starting: " << this << endl;
         m_function();
-        m_finished.store(true, std::memory_order_acquire);
+//        m_finished.store(true, std::memory_order_acquire);
 //        cout << "exiting: " << this << endl;
     }
 
@@ -83,6 +105,14 @@ void Thread::join(){
         return;
     }
 
+#ifdef PA_ENABLE_QT_ADOPTION_WORKAROUND
+    //  Exit the thread.
+    m_data->quit();
+
+    //  But instead of waiting and joining (which will hang), we leak it intentionally.
+    m_data.release();
+#endif
+
     // Clear the Pimpl data, marking this Thread as joined/empty
     // After this, operator bool() returns false and join() becomes no-op
     m_data.clear();

Common/Cpp/Containers/Pimpl.h

@@ -36,6 +36,12 @@ class Pimpl{
     template <class... Args>
     void reset(Args&&... args);
 
+    Type* release(){
+        Type* ret = m_ptr;
+        m_ptr = nullptr;
+        return ret;
+    }
+
 
 public:
     operator bool() const{ return m_ptr != nullptr; }

Common/Qt/QtThreadPool.cpp

@@ -4,6 +4,7 @@
  *
  */
 
+#include "Common/Cpp/Concurrency/Qt6.9ThreadBugWorkaround.h"
 #include "Common/Cpp/Concurrency/SpinPause.h"
 #include "QtThreadPool.h"
 
@@ -203,6 +204,12 @@ QtEventThreadPool::~QtEventThreadPool(){
     stop();
 }
 void QtEventThreadPool::stop(){
+#ifdef PA_ENABLE_QT_ADOPTION_WORKAROUND
+    //  Leak all the threads because they can't be joined without hanging.
+    for (std::unique_ptr<QtEventThread>& thread : m_threads){
+        thread.release();
+    }
+#endif
     m_threads.clear();
     m_available_threads.clear();
 }
@@ -244,8 +251,10 @@ QtEventThread& QtEventThreadPool::get_thread(){
     std::lock_guard<Mutex> lg(m_lock);
     if (m_available_threads.empty()){
         m_available_threads.reserve(m_threads.size() + 1);
-        auto& new_thread = m_threads.emplace_back();
-        m_available_threads.emplace_back(&new_thread);
+        QtEventThread* new_thread = m_threads.emplace_back(
+            std::make_unique<QtEventThread>()
+        ).get();
+        m_available_threads.emplace_back(new_thread);
     }
     QtEventThread* ret = m_available_threads.back();
 //    cout << "QtEventThreadPool: " << m_threads.size() << ", using = " << ret << endl;

Common/Qt/QtThreadPool.h

@@ -114,7 +114,7 @@ class QtEventThreadPool : public QObject{
 
     Mutex m_lock;
 
-    std::deque<QtEventThread> m_threads;
+    std::vector<std::unique_ptr<QtEventThread>> m_threads;
     std::vector<QtEventThread*> m_available_threads;
 };
 

SerialPrograms/Source/CommonFramework/AudioPipeline/IO/AudioFileLoader.cpp

@@ -24,7 +24,11 @@
 
 namespace PokemonAutomation{
 
-AudioFileLoader::AudioFileLoader(QObject* parent, const std::string& filename, const QAudioFormat& audioFormat):
+AudioFileLoader::AudioFileLoader(
+    QObject* parent,
+    const std::string& filename,
+    const QAudioFormat& audioFormat
+):
     QObject(parent), m_filename(filename), m_audioFormat(audioFormat) {}
 
 AudioFileLoader::~AudioFileLoader(){

SerialPrograms/Source/CommonFramework/AudioPipeline/IO/AudioFileLoader.h

@@ -43,7 +43,11 @@ class AudioFileLoader: public QObject{
     Q_OBJECT
 
 public:
-    AudioFileLoader(QObject* parent, const std::string& filename, const QAudioFormat& audioFormat);
+    AudioFileLoader(
+        QObject* parent,
+        const std::string& filename,
+        const QAudioFormat& audioFormat
+    );
     virtual ~AudioFileLoader();
 
     // Start loading and decoding audio samples.
@@ -126,7 +130,12 @@ class AudioDecoderWorker: public QObject{
 Q_OBJECT
 
 public:
-    AudioDecoderWorker(QObject* parent, const std::string& filename, const QAudioFormat& audioFormat, std::vector<char>& decodedBuffer);
+    AudioDecoderWorker(
+        QObject* parent,
+        const std::string& filename,
+        const QAudioFormat& audioFormat,
+        std::vector<char>& decodedBuffer
+    );
     virtual ~AudioDecoderWorker();
 
     void start();

SerialPrograms/Source/CommonFramework/Main.cpp

@@ -234,8 +234,18 @@ int main(int argc, char *argv[]){
     global_watchdog().stop();
     static_cast<FileWindowLogger&>(global_logger_raw()).stop();
 
-    //  When we actually migrate to Qt 6.9+, we may need to move the exit(0)
-    //  call here since joining *any* threads may hang.
+//
+//  Workaround Qt 6.9 thread-adoption bug on Windows.
+//      https://github.com/PokemonAutomation/Arduino-Source/issues/570
+//      https://bugreports.qt.io/browse/QTBUG-131892
+//
+//  Joining threads on Qt 6.9+ Windows will hang!
+//
+//#ifdef PA_ENABLE_QT_ADOPTION_WORKAROUND
+//    cout << "Qt6.9 bug workaround: abort() to avoid thread join hangs." << endl;
+//    abort();
+//    exit(ret);
+//#endif
 
     //  Force stop the thread pools.
     //  This is where all the threads in the program are joined.
@@ -250,17 +260,5 @@ int main(int argc, char *argv[]){
 
     cout << "Exiting main()..." << endl;
 
-
-//
-//  Workaround Qt 6.9 thread-adoption bug on Windows.
-//      https://github.com/PokemonAutomation/Arduino-Source/issues/570
-//      https://bugreports.qt.io/browse/QTBUG-131892
-//
-//  Program will hang after main() without this!
-//
-#ifdef PA_ENABLE_QT_ADOPTION_WORKAROUND
-    exit(ret);
-#endif
-
     return ret;
 }