Refactor threading to a centralised location which can keep track of all the threads

openlp/core/threading.py

@@ -24,26 +24,28 @@ The :mod:`openlp.core.threading` module contains some common threading code
 """
 from PyQt5 import QtCore
 
+from openlp.core.common.registry import Registry
 
-def run_thread(parent, worker, prefix='', auto_start=True):
+
+def run_thread(worker, thread_name, can_start=True):
     """
     Create a thread and assign a worker to it. This removes a lot of boilerplate code from the codebase.
 
-    :param object parent: The parent object so that the thread and worker are not orphaned.
     :param QObject worker: A QObject-based worker object which does the actual work.
-    :param str prefix: A prefix to be applied to the attribute names.
+    :param str thread_name: The name of the thread, used to keep track of the thread.
-    :param bool auto_start: Automatically start the thread. Defaults to True.
+    :param bool can_start: Start the thread. Defaults to True.
     """
-    # Set up attribute names
+    if not thread_name:
-    thread_name = 'thread'
+        raise ValueError('A thread_name is required when calling the "run_thread" function')
-    worker_name = 'worker'
+    main_window = Registry().get('main_window')
-    if prefix:
+    if thread_name in main_window.threads:
-        thread_name = '_'.join([prefix, thread_name])
+        raise KeyError('A thread with the name "{}" has already been created, please use another'.format(thread_name))
-        worker_name = '_'.join([prefix, worker_name])
     # Create the thread and add the thread and the worker to the parent
     thread = QtCore.QThread()
-    setattr(parent, thread_name, thread)
+    main_window.threads[thread_name] = {
-    setattr(parent, worker_name, worker)
+        'thread': thread,
+        'worker': worker
+    }
     # Move the worker into the thread's context
     worker.moveToThread(thread)
     # Connect slots and signals
@@ -51,5 +53,25 @@ def run_thread(parent, worker, prefix='', auto_start=True):
     worker.quit.connect(thread.quit)
     worker.quit.connect(worker.deleteLater)
     thread.finished.connect(thread.deleteLater)
-    if auto_start:
+    thread.finished.connect(make_remove_thread(thread_name))
+    if can_start:
         thread.start()
+
+
+def make_remove_thread(thread_name):
+    """
+    Create a function to remove the thread once the thread is finished.
+
+    :param str thread_name: The name of the thread which should be removed from the thread registry.
+    :returns function: A function which will remove the thread from the thread registry.
+    """
+    def remove_thread():
+        """
+        Stop and remove a registered thread
+
+        :param str thread_name: The name of the thread to stop and remove
+        """
+        main_window = Registry().get('main_window')
+        if thread_name in main_window.threads:
+            del main_window.threads[thread_name]
+    return remove_thread

openlp/core/ui/mainwindow.py

@@ -482,8 +482,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         """
         super(MainWindow, self).__init__()
         Registry().register('main_window', self)
-        self.version_thread = None
+        self.threads = {}
-        self.version_worker = None
         self.clipboard = self.application.clipboard()
         self.arguments = ''.join(self.application.args)
         # Set up settings sections for the main application (not for use by plugins).
@@ -1005,25 +1004,31 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         if not self.application.is_event_loop_active:
             event.ignore()
             return
-        # Sometimes the version thread hasn't finished, let's wait for it
+        # Sometimes the threads haven't finished, let's wait for them
-        try:
-            if self.version_thread and self.version_thread.isRunning():
         wait_dialog = QtWidgets.QProgressDialog('Waiting for some things to finish...', '', 0, 0, self)
         wait_dialog.setWindowModality(QtCore.Qt.WindowModal)
         wait_dialog.setAutoClose(False)
         wait_dialog.setCancelButton(None)
         wait_dialog.show()
-                retry = 0
+        for thread_name in self.threads.keys():
-                while self.version_thread.isRunning() and retry < 50:
             self.application.processEvents()
-                    self.version_thread.wait(100)
+            thread = self.threads[thread_name]['thread']
+            try:
+                if thread and thread.isRunning():
+                    # If the thread is running, let's wait 5 seconds for it
+                    retry = 0
+                    while thread.isRunning() and retry < 50:
+                        # Make the GUI responsive while we wait
+                        self.application.processEvents()
+                        thread.wait(100)
                         retry += 1
-                if self.version_thread.isRunning():
+                    if thread.isRunning():
-                    self.version_thread.terminate()
+                        # If the thread is still running after 5 seconds, kill it
-                wait_dialog.close()
+                        thread.terminate()
             except RuntimeError:
                 # Ignore the RuntimeError that is thrown when Qt has already deleted the C++ thread object
                 pass
+        wait_dialog.close()
         # If we just did a settings import, close without saving changes.
         if self.settings_imported:
             self.clean_up(False)

openlp/plugins/songs/forms/songselectform.py

@@ -27,10 +27,10 @@ from time import sleep
 
 from PyQt5 import QtCore, QtWidgets
 
-from openlp.core.common import is_win
 from openlp.core.common.i18n import translate
-from openlp.core.common.registry import Registry
+from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.settings import Settings
+from openlp.core.threading import run_thread
 from openlp.plugins.songs.forms.songselectdialog import Ui_SongSelectDialog
 from openlp.plugins.songs.lib.songselect import SongSelectImport
 
@@ -74,7 +74,7 @@ class SearchWorker(QtCore.QObject):
         self.found_song.emit(song)
 
 
-class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
+class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties):
     """
     The :class:`SongSelectForm` class is the SongSelect dialog.
     """
@@ -90,8 +90,6 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
         """
         Initialise the SongSelectForm
         """
-        self.thread = None
-        self.worker = None
         self.song_count = 0
         self.song = None
         self.set_progress_visible(False)
@@ -311,17 +309,11 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
         search_history = self.search_combobox.getItems()
         Settings().setValue(self.plugin.settings_section + '/songselect searches', '|'.join(search_history))
         # Create thread and run search
-        self.thread = QtCore.QThread()
+        worker = SearchWorker(self.song_select_importer, self.search_combobox.currentText())
-        self.worker = SearchWorker(self.song_select_importer, self.search_combobox.currentText())
+        worker.show_info.connect(self.on_search_show_info)
-        self.worker.moveToThread(self.thread)
+        worker.found_song.connect(self.on_search_found_song)
-        self.thread.started.connect(self.worker.start)
+        worker.finished.connect(self.on_search_finished)
-        self.worker.show_info.connect(self.on_search_show_info)
+        run_thread(worker, 'songselect')
-        self.worker.found_song.connect(self.on_search_found_song)
-        self.worker.finished.connect(self.on_search_finished)
-        self.worker.quit.connect(self.thread.quit)
-        self.worker.quit.connect(self.worker.deleteLater)
-        self.thread.finished.connect(self.thread.deleteLater)
-        self.thread.start()
 
     def on_stop_button_clicked(self):
         """
@@ -408,16 +400,3 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
         """
         self.search_progress_bar.setVisible(is_visible)
         self.stop_button.setVisible(is_visible)
-
-    @property
-    def application(self):
-        """
-        Adds the openlp to the class dynamically.
-        Windows needs to access the application in a dynamic manner.
-        """
-        if is_win():
-            return Registry().get('application')
-        else:
-            if not hasattr(self, '_application'):
-                self._application = Registry().get('application')
-            return self._application