Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Is this multithreading code safe?
QtWS25 Last Chance

Is this multithreading code safe?

Scheduled Pinned Locked Moved Solved General and Desktop
26 Posts 6 Posters 6.8k Views
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • C Offline
    C Offline
    Crag_Hack
    wrote on last edited by Crag_Hack
    #9

    One last quick question: how do I clean up after myself when closing the program with regard to the worker object and qthread?

    JKSHJ 1 Reply Last reply
    0
    • C Crag_Hack

      One last quick question: how do I clean up after myself when closing the program with regard to the worker object and qthread?

      JKSHJ Offline
      JKSHJ Offline
      JKSH
      Moderators
      wrote on last edited by
      #10

      @Crag_Hack said in Is this multithreading code safe?:

      One last quick question: how do I clean up after myself when closing the program with regard to the worker object and qthread?

      Make sure you wait for the thread to finish before you close the program. See http://doc.qt.io/qt-5/qthread.html#wait

      Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

      1 Reply Last reply
      1
      • C Offline
        C Offline
        Crag_Hack
        wrote on last edited by
        #11

        Do I need to do deletelater on the worker object and thread? If so do I block until deletelater succeeds?

        JKSHJ 1 Reply Last reply
        0
        • C Crag_Hack

          Do I need to do deletelater on the worker object and thread? If so do I block until deletelater succeeds?

          JKSHJ Offline
          JKSHJ Offline
          JKSH
          Moderators
          wrote on last edited by
          #12

          @Crag_Hack said in Is this multithreading code safe?:

          Do I need to do deletelater on the worker object and thread?

          You already call deleteLater() on your worker object when the task finishes, right?

          If so do I block until deletelater succeeds?

          Yes for the worker object, because it is deleted in another thread.

          The QThread object lives in your main thread, so just treat it the same as any other QObject in your main thread. How do you delete those?

          Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

          1 Reply Last reply
          0
          • C Offline
            C Offline
            Crag_Hack
            wrote on last edited by
            #13

            You already call deleteLater() on your worker object when the task finishes, right?

            I did...but I decided to remodel after everything discussed in this thread and reuse the same object and thread instead. Multithreading in that sentence woah.... HA :)

            The QThread object lives in your main thread, so just treat it the same as any other QObject in your main thread. How do you delete those?

            Hmmm... using a simple delete I think. Never thought about individual QT class object's relationships to QObject though I surmise they all derive from such correct?

            JKSHJ kshegunovK 2 Replies Last reply
            0
            • C Crag_Hack

              You already call deleteLater() on your worker object when the task finishes, right?

              I did...but I decided to remodel after everything discussed in this thread and reuse the same object and thread instead. Multithreading in that sentence woah.... HA :)

              The QThread object lives in your main thread, so just treat it the same as any other QObject in your main thread. How do you delete those?

              Hmmm... using a simple delete I think. Never thought about individual QT class object's relationships to QObject though I surmise they all derive from such correct?

              JKSHJ Offline
              JKSHJ Offline
              JKSH
              Moderators
              wrote on last edited by
              #14

              @Crag_Hack said in Is this multithreading code safe?:

              You already call deleteLater() on your worker object when the task finishes, right?

              I did...but I decided to remodel after everything discussed in this thread and reuse the same object and thread instead. Multithreading in that sentence woah.... HA :)

              OK. In that case, it would be easiest to delete your worker object right before you quit the worker thread (and your program). Do something like this in your main thread:

              // Make worker->deleteLater() run in the worker thread, and block until the deletion is complete
              QMetaObject::invokeMethod(worker, "deleteLater", Qt::BlockingQueuedConnection);
              
              // ...
              // At this point in your code, your worker is guaranteed to be gone.
              // ...
              
              // Stop the thread, and block until the thread is fully stopped
              workerThread->quit();
              workerThread->wait();
              
              // ...
              // At this point in your code, the worker thread is guaranteed to have stopped. You can now delete your QThread object which was managing that worker thread.
              // ...
              

              For more info, see:

              • http://doc.qt.io/qt-5/qmetaobject.html#invokeMethod
              • http://doc.qt.io/qt-5/qt.html#ConnectionType-enum

              The QThread object lives in your main thread, so just treat it the same as any other QObject in your main thread. How do you delete those?

              Hmmm... using a simple delete I think. Never thought about individual QT class object's relationships to QObject though I surmise they all derive from such correct?

              See http://doc.qt.io/qt-5/objecttrees.html

              QObjects can form parent-child relationships. For most QObject-derived classes, the last parameter in their constuctor is for specifying the parent.

              When QObject is deleted, its child objects are automatically deleted too. Conventionally, Qt developers often rely on this feature, so most QObjects don't need to be explicitly deleted. For example, make your QThread a child of your main window. The window gets deleted at shutdown, so it auto-deletes your QThread too.

              deleteLater() ensures that all signals/events for an object are processed before the object is deleted. If you are 100% that your object has nothing left to process, you can use a simple delete. If you're not sure, use deleteLater() to be safe.

              Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

              J.HilkJ 1 Reply Last reply
              2
              • JKSHJ JKSH

                @Crag_Hack said in Is this multithreading code safe?:

                You already call deleteLater() on your worker object when the task finishes, right?

                I did...but I decided to remodel after everything discussed in this thread and reuse the same object and thread instead. Multithreading in that sentence woah.... HA :)

                OK. In that case, it would be easiest to delete your worker object right before you quit the worker thread (and your program). Do something like this in your main thread:

                // Make worker->deleteLater() run in the worker thread, and block until the deletion is complete
                QMetaObject::invokeMethod(worker, "deleteLater", Qt::BlockingQueuedConnection);
                
                // ...
                // At this point in your code, your worker is guaranteed to be gone.
                // ...
                
                // Stop the thread, and block until the thread is fully stopped
                workerThread->quit();
                workerThread->wait();
                
                // ...
                // At this point in your code, the worker thread is guaranteed to have stopped. You can now delete your QThread object which was managing that worker thread.
                // ...
                

                For more info, see:

                • http://doc.qt.io/qt-5/qmetaobject.html#invokeMethod
                • http://doc.qt.io/qt-5/qt.html#ConnectionType-enum

                The QThread object lives in your main thread, so just treat it the same as any other QObject in your main thread. How do you delete those?

                Hmmm... using a simple delete I think. Never thought about individual QT class object's relationships to QObject though I surmise they all derive from such correct?

                See http://doc.qt.io/qt-5/objecttrees.html

                QObjects can form parent-child relationships. For most QObject-derived classes, the last parameter in their constuctor is for specifying the parent.

                When QObject is deleted, its child objects are automatically deleted too. Conventionally, Qt developers often rely on this feature, so most QObjects don't need to be explicitly deleted. For example, make your QThread a child of your main window. The window gets deleted at shutdown, so it auto-deletes your QThread too.

                deleteLater() ensures that all signals/events for an object are processed before the object is deleted. If you are 100% that your object has nothing left to process, you can use a simple delete. If you're not sure, use deleteLater() to be safe.

                J.HilkJ Online
                J.HilkJ Online
                J.Hilk
                Moderators
                wrote on last edited by J.Hilk
                #15

                @JKSH said in Is this multithreading code safe?:

                When QObject is deleted, its child objects are automatically deleted too. Conventionally, Qt developers often rely on this feature, so most QObjects don't need to be explicitly deleted. For example, make your QThread a child of your main window. The window gets deleted at shutdown, so it auto-deletes your QThread too.

                Hold on, I'm pretty sure, that if you make your thread a child of your main window, that you don't have a 100% seperated new thread. Something like this for example:

                QThread * thread = new QThread(this);
                worker *threadObject = new worker(this);
                connect(thread, &QThread::started, threadObject, &worker::startWork);
                
                threadObject->moveToThread(worker);
                thread->start();
                

                Both main thread and worker thread have the same currentThreadId()
                and a big calculation should also freeze your main gui.

                Edit: renamed threadObject.


                Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


                Q: What's that?
                A: It's blue light.
                Q: What does it do?
                A: It turns blue.

                kshegunovK JKSHJ 2 Replies Last reply
                0
                • C Crag_Hack

                  You already call deleteLater() on your worker object when the task finishes, right?

                  I did...but I decided to remodel after everything discussed in this thread and reuse the same object and thread instead. Multithreading in that sentence woah.... HA :)

                  The QThread object lives in your main thread, so just treat it the same as any other QObject in your main thread. How do you delete those?

                  Hmmm... using a simple delete I think. Never thought about individual QT class object's relationships to QObject though I surmise they all derive from such correct?

                  kshegunovK Offline
                  kshegunovK Offline
                  kshegunov
                  Moderators
                  wrote on last edited by kshegunov
                  #16

                  Just connect the thread's finished() signal to the worker object's deleteLater() slot and you'll be fine, e.g:

                  QThread thread;
                  thread.start();
                  
                  QObject * worker = new QObject();
                  QObject::connect(&thread, &QThread::finished, worker, &QObject::deleteLater);
                  
                  // ... more code ...
                  

                  There's no need to block the worker thread, you just need to wait for it to gracefully finish as @JKSH said. The deferred deletion events are guaranteed to be executed on thread exiting (look up the docs for the exact quotation).

                  Read and abide by the Qt Code of Conduct

                  JKSHJ 1 Reply Last reply
                  2
                  • J.HilkJ J.Hilk

                    @JKSH said in Is this multithreading code safe?:

                    When QObject is deleted, its child objects are automatically deleted too. Conventionally, Qt developers often rely on this feature, so most QObjects don't need to be explicitly deleted. For example, make your QThread a child of your main window. The window gets deleted at shutdown, so it auto-deletes your QThread too.

                    Hold on, I'm pretty sure, that if you make your thread a child of your main window, that you don't have a 100% seperated new thread. Something like this for example:

                    QThread * thread = new QThread(this);
                    worker *threadObject = new worker(this);
                    connect(thread, &QThread::started, threadObject, &worker::startWork);
                    
                    threadObject->moveToThread(worker);
                    thread->start();
                    

                    Both main thread and worker thread have the same currentThreadId()
                    and a big calculation should also freeze your main gui.

                    Edit: renamed threadObject.

                    kshegunovK Offline
                    kshegunovK Offline
                    kshegunov
                    Moderators
                    wrote on last edited by
                    #17

                    @J.Hilk said in Is this multithreading code safe?:

                    Both main thread and worker thread have the same currentThreadId()
                    and a big calculation should also freeze your main gui.

                    Of course they have. The thread objects all live in the main thread in your code and that's absolutely normal. The QThread class manages a thread, it is not a thread itself. The only thing that's executed in a new thread context is QThread::run (which you don't touch when doing the worker object anyway). The slots of your worker object are executed in the new thread (granted you don't use Qt::DirectConnection when connecting).

                    Read and abide by the Qt Code of Conduct

                    J.HilkJ 1 Reply Last reply
                    2
                    • kshegunovK kshegunov

                      @J.Hilk said in Is this multithreading code safe?:

                      Both main thread and worker thread have the same currentThreadId()
                      and a big calculation should also freeze your main gui.

                      Of course they have. The thread objects all live in the main thread in your code and that's absolutely normal. The QThread class manages a thread, it is not a thread itself. The only thing that's executed in a new thread context is QThread::run (which you don't touch when doing the worker object anyway). The slots of your worker object are executed in the new thread (granted you don't use Qt::DirectConnection when connecting).

                      J.HilkJ Online
                      J.HilkJ Online
                      J.Hilk
                      Moderators
                      wrote on last edited by
                      #18

                      @kshegunov
                      thanks, that supports my point. I beleive ? :-)

                      Changing my sample to not having parents:

                      QThread * thread = new QThread();
                      worker *threadObject = new worker();
                      connect(thread, &QThread::started, threadObject, &worker::startWork);
                      
                      threadObject->moveToThread(worker);
                      thread->start();
                      

                      Separates them, but one has to handle the delete 'manualy' with

                      connect(&thread, &QThread::finished, worker, &QObject::deleteLater);
                      

                      Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


                      Q: What's that?
                      A: It's blue light.
                      Q: What does it do?
                      A: It turns blue.

                      kshegunovK 1 Reply Last reply
                      0
                      • J.HilkJ J.Hilk

                        @kshegunov
                        thanks, that supports my point. I beleive ? :-)

                        Changing my sample to not having parents:

                        QThread * thread = new QThread();
                        worker *threadObject = new worker();
                        connect(thread, &QThread::started, threadObject, &worker::startWork);
                        
                        threadObject->moveToThread(worker);
                        thread->start();
                        

                        Separates them, but one has to handle the delete 'manualy' with

                        connect(&thread, &QThread::finished, worker, &QObject::deleteLater);
                        
                        kshegunovK Offline
                        kshegunovK Offline
                        kshegunov
                        Moderators
                        wrote on last edited by
                        #19

                        @J.Hilk said in Is this multithreading code safe?:

                        thanks, that supports my point. I beleive ? :-)

                        Which point you mean? I was responding to this:

                        a big calculation should also freeze your main gui.

                        which is certainly not correct. The big calculation you do in a slot in the worker object is executed in a separate thread, so it doesn't block the GUI thread.

                        Changing my sample to not having parents

                        Parents don't matter here, except for the moveToThread call - you can't move a child object to a thread different from the parent object's thread.

                        Separates them, but one has to handle the delete 'manualy' with

                        Surely C++ requires you to manage your memory, so "manually" handling deletions is not something new. I sometimes even create the worker object on the stack so the runtime frees it for me. E.g.:

                        int main(int argc, char ** argv)
                        {
                            QCoreApplication app(argc, argv);
                        
                            QObject worker;
                            QThread thread;
                        
                            worker.moveToThread(&thread);
                            thread.start();
                        
                            // ... other stuff
                        
                            QMetaObject::invokeMethod(&app, "quit", Qt::QueuedConnection);
                            QObject::connect(&app, &QCoreApplication::aboutToQuit, &thread, &QThread::quit);
                        
                            int ret = QCoreApplication::exec();
                        
                            thread.wait(); // ... and no `delete` is called at all, the stack unwinding will take care of the worker object.
                        
                            return ret;
                        }
                        

                        Read and abide by the Qt Code of Conduct

                        1 Reply Last reply
                        2
                        • J.HilkJ J.Hilk

                          @JKSH said in Is this multithreading code safe?:

                          When QObject is deleted, its child objects are automatically deleted too. Conventionally, Qt developers often rely on this feature, so most QObjects don't need to be explicitly deleted. For example, make your QThread a child of your main window. The window gets deleted at shutdown, so it auto-deletes your QThread too.

                          Hold on, I'm pretty sure, that if you make your thread a child of your main window, that you don't have a 100% seperated new thread. Something like this for example:

                          QThread * thread = new QThread(this);
                          worker *threadObject = new worker(this);
                          connect(thread, &QThread::started, threadObject, &worker::startWork);
                          
                          threadObject->moveToThread(worker);
                          thread->start();
                          

                          Both main thread and worker thread have the same currentThreadId()
                          and a big calculation should also freeze your main gui.

                          Edit: renamed threadObject.

                          JKSHJ Offline
                          JKSHJ Offline
                          JKSH
                          Moderators
                          wrote on last edited by JKSH
                          #20

                          @J.Hilk said in Is this multithreading code safe?:

                          Hold on, I'm pretty sure, that if you make your thread a child of your main window, that you don't have a 100% seperated new thread.

                          Why?

                          Parent-child relationships only describe "ownership", and determines the order of automatically-deleted objects. These have absolutely no bearing on what threads get created in a program.

                          If a QThread instance is a child of a QMainWindow instance, that just means the QMainWindow "owns" the QThread instance, and will delete it.

                          Something like this for example:

                          QThread * thread = new QThread(this);
                          worker *threadObject = new worker(this);
                          connect(thread, &QThread::started, threadObject, &worker::startWork);
                          
                          threadObject->moveToThread(worker);
                          thread->start();
                          

                          Both main thread and worker thread have the same currentThreadId()
                          and a big calculation should also freeze your main gui.

                          QObject::currentThreadId() is a static function that tells you, "Which is the thread that called the currentThreadId() function?" It does not tell you "Which thread does a particular object live in?"

                          Add this to the end of your code snippet:

                          qDebug() << qApp->thread(); // Reports the thread affinity of your QApplication/QGuiApplication/QCoreApplication instance
                          qDebug() << thread->thread(); // Reprts the thread affinity of your QThread instance
                          qDebug() << threadObject->thread(); // Reports the thread affinity of your worker object instance
                          

                          You'll find that threadObject lives in a different thread than qApp.

                          You'll also find that thread lives in the same thread as qApp, and that thread lives in a different thread than threadObject. (Yep, you read that right. Let this sink in slowly. A QThread is not a thread!)

                          Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

                          J.HilkJ 1 Reply Last reply
                          4
                          • kshegunovK kshegunov

                            Just connect the thread's finished() signal to the worker object's deleteLater() slot and you'll be fine, e.g:

                            QThread thread;
                            thread.start();
                            
                            QObject * worker = new QObject();
                            QObject::connect(&thread, &QThread::finished, worker, &QObject::deleteLater);
                            
                            // ... more code ...
                            

                            There's no need to block the worker thread, you just need to wait for it to gracefully finish as @JKSH said. The deferred deletion events are guaranteed to be executed on thread exiting (look up the docs for the exact quotation).

                            JKSHJ Offline
                            JKSHJ Offline
                            JKSH
                            Moderators
                            wrote on last edited by
                            #21

                            @kshegunov said in Is this multithreading code safe?:

                            Just connect the thread's finished() signal to the worker object's deleteLater() slot and you'll be fine, e.g:

                            QThread thread;
                            thread.start();
                            
                            QObject * worker = new QObject();
                            QObject::connect(&thread, &QThread::finished, worker, &QObject::deleteLater);
                            
                            // ... more code ...
                            

                            There's no need to block the worker thread, you just need to wait for it to gracefully finish as @JKSH said. The deferred deletion events are guaranteed to be executed on thread exiting (look up the docs for the exact quotation).

                            There's one thing I've been meaning to investigate, but I haven't gotten around to: Is the deferred deletion guaranteed to finish before QThread::wait() returns? In other words: If the worker object's destructor takes a long time to run, is there a risk of the main thread quitting before the worker object is fully deleted?

                            Qt Doc Search for browsers: forum.qt.io/topic/35616/web-browser-extension-for-improved-doc-searches

                            kshegunovK 1 Reply Last reply
                            0
                            • JKSHJ JKSH

                              @J.Hilk said in Is this multithreading code safe?:

                              Hold on, I'm pretty sure, that if you make your thread a child of your main window, that you don't have a 100% seperated new thread.

                              Why?

                              Parent-child relationships only describe "ownership", and determines the order of automatically-deleted objects. These have absolutely no bearing on what threads get created in a program.

                              If a QThread instance is a child of a QMainWindow instance, that just means the QMainWindow "owns" the QThread instance, and will delete it.

                              Something like this for example:

                              QThread * thread = new QThread(this);
                              worker *threadObject = new worker(this);
                              connect(thread, &QThread::started, threadObject, &worker::startWork);
                              
                              threadObject->moveToThread(worker);
                              thread->start();
                              

                              Both main thread and worker thread have the same currentThreadId()
                              and a big calculation should also freeze your main gui.

                              QObject::currentThreadId() is a static function that tells you, "Which is the thread that called the currentThreadId() function?" It does not tell you "Which thread does a particular object live in?"

                              Add this to the end of your code snippet:

                              qDebug() << qApp->thread(); // Reports the thread affinity of your QApplication/QGuiApplication/QCoreApplication instance
                              qDebug() << thread->thread(); // Reprts the thread affinity of your QThread instance
                              qDebug() << threadObject->thread(); // Reports the thread affinity of your worker object instance
                              

                              You'll find that threadObject lives in a different thread than qApp.

                              You'll also find that thread lives in the same thread as qApp, and that thread lives in a different thread than threadObject. (Yep, you read that right. Let this sink in slowly. A QThread is not a thread!)

                              J.HilkJ Online
                              J.HilkJ Online
                              J.Hilk
                              Moderators
                              wrote on last edited by
                              #22

                              @JKSH @kshegunov

                              well, I'm not resisten to learning, hopefully.
                              So thanks for the detailed exmplanation, I'll have to read everything up once more I believe.

                              Complicated enough topic as it is.


                              Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


                              Q: What's that?
                              A: It's blue light.
                              Q: What does it do?
                              A: It turns blue.

                              1 Reply Last reply
                              0
                              • JKSHJ JKSH

                                @kshegunov said in Is this multithreading code safe?:

                                Just connect the thread's finished() signal to the worker object's deleteLater() slot and you'll be fine, e.g:

                                QThread thread;
                                thread.start();
                                
                                QObject * worker = new QObject();
                                QObject::connect(&thread, &QThread::finished, worker, &QObject::deleteLater);
                                
                                // ... more code ...
                                

                                There's no need to block the worker thread, you just need to wait for it to gracefully finish as @JKSH said. The deferred deletion events are guaranteed to be executed on thread exiting (look up the docs for the exact quotation).

                                There's one thing I've been meaning to investigate, but I haven't gotten around to: Is the deferred deletion guaranteed to finish before QThread::wait() returns? In other words: If the worker object's destructor takes a long time to run, is there a risk of the main thread quitting before the worker object is fully deleted?

                                kshegunovK Offline
                                kshegunovK Offline
                                kshegunov
                                Moderators
                                wrote on last edited by
                                #23

                                @JKSH said in Is this multithreading code safe?:

                                Is the deferred deletion guaranteed to finish before QThread::wait() returns?

                                Yes, it is. The deferred deletions are treated in a special way so they're processed just before the thread actually quits (which implies they're processed after the finished() signal is emitted).

                                In other words: If the worker object's destructor takes a long time to run, is there a risk of the main thread quitting before the worker object is fully deleted?

                                Not if you wait() on the thread. Not calling wait() is another kettle of fish altogether and introduces even more complications, although it may be necessary sometimes when managing multiple threads and you want to wait for them all.

                                Read and abide by the Qt Code of Conduct

                                1 Reply Last reply
                                1
                                • C Offline
                                  C Offline
                                  Crag_Hack
                                  wrote on last edited by Crag_Hack
                                  #24

                                  Hi here's the final modified code (signal/slot data pass instead of direct via worker method call and also reuse of thread and worker object). Sorry for being such a perfectionst but I don't want to take any risks with anybody's data considering this is a backup program.

                                  Worker is a class derived from QObject that contains the worker thread code (just backing up files). The thread and worker object are created in the program's QWidget main gui thread constructor. advancedCheckedBackup is called by clicking the backup button which then processes the backup jobs. The setupworker function just sets up the signals and slots for the worker object and thread. The jobList variable is a list of custom BackupJob objects that contain backup job information (source, destination, etc).

                                  Is this code safe?

                                  When using signal slot communication between threads it relies on the copy constructor to create a copy of the object passed to another thread right? If so my BackupJob objects get copied as reentrant objects properly right? (they only have reentrant data members except for one pointer (not sure if it's reentrant) to IVssBackupComponents for volume shadow copy service - this pointer is only used nonconcurrently inside each program backup job and freed upon completion of the job)

                                  Also one signal not in the below code is emitted by my worker thread and passes a single backupjob object back to the main thread - this is safe by the same reasoning as previously mentioned regarding the passing of backup jobs to the worker thread right?

                                  I think this entire scenario of using threads to implement a non-gui-blocking worker thread for prolonged operations without concurrent data modification is an excellent candidate for a thread example in the thread documentation here http://doc.qt.io/qt-5/threads.html What do you think? Donno if you guys can influence such.

                                  Thanks!

                                  Replicator::Replicator(QWidget *parent) :
                                      QWidget(parent), wizard(0)
                                  {
                                      workerThread = new QThread(this);
                                      workerThread->start();
                                      worker = new Worker;
                                      worker->moveToThread(workerThread);
                                      setupWorker();
                                  }
                                  
                                  void Replicator::setupWorker()
                                  {
                                      connect(workerThread, SIGNAL(finished()), worker, SLOT(deleteLater()));
                                  
                                      connect(this, SIGNAL(backup(QList<BackupJob>)), worker, SLOT(backup(QList<BackupJob>)));
                                      connect(worker, SIGNAL(finished(bool)), this, SLOT(backupDone(bool)));
                                  }
                                  
                                  void Replicator::advancedCheckedBackup()
                                  {
                                      pendingQueue = mainScreen->getJobList();
                                      QList<BackupJob> jobList;
                                      for (int backupCount = 0; backupCount < pendingQueue.size(); backupCount++)
                                      {
                                          jobList << advancedJobs[pendingQueue[backupCount]];
                                      }
                                  
                                      emit backup(jobList);
                                  }
                                  
                                  void Worker::backup(QList<BackupJob> jobs)
                                  {
                                      this->jobs = jobs;
                                  
                                  	***backup code here***
                                  }
                                  
                                  Replicator::~Replicator()
                                  {
                                      workerThread->quit();
                                      workerThread->wait();
                                  }
                                  
                                  kshegunovK 1 Reply Last reply
                                  0
                                  • C Crag_Hack

                                    Hi here's the final modified code (signal/slot data pass instead of direct via worker method call and also reuse of thread and worker object). Sorry for being such a perfectionst but I don't want to take any risks with anybody's data considering this is a backup program.

                                    Worker is a class derived from QObject that contains the worker thread code (just backing up files). The thread and worker object are created in the program's QWidget main gui thread constructor. advancedCheckedBackup is called by clicking the backup button which then processes the backup jobs. The setupworker function just sets up the signals and slots for the worker object and thread. The jobList variable is a list of custom BackupJob objects that contain backup job information (source, destination, etc).

                                    Is this code safe?

                                    When using signal slot communication between threads it relies on the copy constructor to create a copy of the object passed to another thread right? If so my BackupJob objects get copied as reentrant objects properly right? (they only have reentrant data members except for one pointer (not sure if it's reentrant) to IVssBackupComponents for volume shadow copy service - this pointer is only used nonconcurrently inside each program backup job and freed upon completion of the job)

                                    Also one signal not in the below code is emitted by my worker thread and passes a single backupjob object back to the main thread - this is safe by the same reasoning as previously mentioned regarding the passing of backup jobs to the worker thread right?

                                    I think this entire scenario of using threads to implement a non-gui-blocking worker thread for prolonged operations without concurrent data modification is an excellent candidate for a thread example in the thread documentation here http://doc.qt.io/qt-5/threads.html What do you think? Donno if you guys can influence such.

                                    Thanks!

                                    Replicator::Replicator(QWidget *parent) :
                                        QWidget(parent), wizard(0)
                                    {
                                        workerThread = new QThread(this);
                                        workerThread->start();
                                        worker = new Worker;
                                        worker->moveToThread(workerThread);
                                        setupWorker();
                                    }
                                    
                                    void Replicator::setupWorker()
                                    {
                                        connect(workerThread, SIGNAL(finished()), worker, SLOT(deleteLater()));
                                    
                                        connect(this, SIGNAL(backup(QList<BackupJob>)), worker, SLOT(backup(QList<BackupJob>)));
                                        connect(worker, SIGNAL(finished(bool)), this, SLOT(backupDone(bool)));
                                    }
                                    
                                    void Replicator::advancedCheckedBackup()
                                    {
                                        pendingQueue = mainScreen->getJobList();
                                        QList<BackupJob> jobList;
                                        for (int backupCount = 0; backupCount < pendingQueue.size(); backupCount++)
                                        {
                                            jobList << advancedJobs[pendingQueue[backupCount]];
                                        }
                                    
                                        emit backup(jobList);
                                    }
                                    
                                    void Worker::backup(QList<BackupJob> jobs)
                                    {
                                        this->jobs = jobs;
                                    
                                    	***backup code here***
                                    }
                                    
                                    Replicator::~Replicator()
                                    {
                                        workerThread->quit();
                                        workerThread->wait();
                                    }
                                    
                                    kshegunovK Offline
                                    kshegunovK Offline
                                    kshegunov
                                    Moderators
                                    wrote on last edited by
                                    #25

                                    @Crag_Hack said in Is this multithreading code safe?:

                                    Is this code safe?

                                    It looks okay to me.

                                    When using signal slot communication between threads it relies on the copy constructor to create a copy of the object passed to another thread right?

                                    Yes.

                                    If so my BackupJob objects get copied as reentrant objects properly right?

                                    Yes.

                                    they only have reentrant data members except for one pointer (not sure if it's reentrant) to IVssBackupComponents for volume shadow copy service

                                    The pointer is reentrant. Whether the object it is pointing to is thread-safe however is another matter altogether.

                                    this pointer is only used nonconcurrently inside each program backup job and freed upon completion of the job.

                                    What about the object the pointer holds a reference to? The point is the pointer may very well be reentrant, but the object it points to might not be thread safe, which would imply you can't call functions of that object in a thread-safe manner.

                                    Also one signal not in the below code is emitted by my worker thread and passes a single backupjob object back to the main thread - this is safe by the same reasoning as previously mentioned regarding the passing of backup jobs to the worker thread right?

                                    Yes.

                                    Donno if you guys can influence such.

                                    Anyone, including you, can write an example and submit the source code for review; that's the point of Qt being offered as open-source (under LGPL) as well as the commercial licensing. :)

                                    Read and abide by the Qt Code of Conduct

                                    1 Reply Last reply
                                    1
                                    • C Offline
                                      C Offline
                                      Crag_Hack
                                      wrote on last edited by
                                      #26

                                      The pointer is reentrant. Whether the object it is pointing to is thread-safe however is another matter altogether.

                                      The pointer is one of these guys used for Volume Shadow Copy Service. Prior to control passing to the worker object the pointer is uninitialized not even null, during the work done by the worker object in the other thread the pointer is initialized through CreateVssBackupComponents, then it goes through a lengthy setup process to set up the volume shadow copy service options, then when the worker no longer needs it it is released through IVssBackupComponents::Release. It never passes from gui thread to worker thread or from worker thread to gui thread.

                                      So it's completely isolated to each job dispatched to the worker object. Sounds safe right?

                                      Thanks so much!

                                      1 Reply Last reply
                                      0

                                      • Login

                                      • Login or register to search.
                                      • First post
                                        Last post
                                      0
                                      • Categories
                                      • Recent
                                      • Tags
                                      • Popular
                                      • Users
                                      • Groups
                                      • Search
                                      • Get Qt Extensions
                                      • Unsolved