From ff421df2bf1d6aa30988f7da41cb459e10673da2 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Fri, 12 Jan 2018 11:29:32 -0700 Subject: [PATCH] Fix bug #1742910 Fixes: https://launchpad.net/bugs/1742910 --- openlp/core/app.py | 2 +- openlp/core/threading.py | 18 ++++++++-------- openlp/core/ui/mainwindow.py | 7 +++---- .../functional/openlp_core/test_threading.py | 21 ++++++++++--------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/openlp/core/app.py b/openlp/core/app.py index b6b3a73f9..2a6cf8494 100644 --- a/openlp/core/app.py +++ b/openlp/core/app.py @@ -63,8 +63,8 @@ class OpenLP(QtWidgets.QApplication, LogMixin): The core application class. This class inherits from Qt's QApplication class in order to provide the core of the application. """ - args = [] + worker_threads = {} def exec(self): """ diff --git a/openlp/core/threading.py b/openlp/core/threading.py index 73e11cdf5..6f5f2b4af 100644 --- a/openlp/core/threading.py +++ b/openlp/core/threading.py @@ -50,12 +50,12 @@ def run_thread(worker, thread_name, can_start=True): """ if not thread_name: raise ValueError('A thread_name is required when calling the "run_thread" function') - main_window = Registry().get('main_window') - if thread_name in main_window.threads: + application = Registry().get('application') + if thread_name in application.worker_threads: raise KeyError('A thread with the name "{}" has already been created, please use another'.format(thread_name)) # Create the thread and add the thread and the worker to the parent thread = QtCore.QThread() - main_window.threads[thread_name] = { + application.worker_threads[thread_name] = { 'thread': thread, 'worker': worker } @@ -78,7 +78,7 @@ def get_thread_worker(thread_name): :param str thread_name: The name of the thread :returns: The worker for this thread name """ - return Registry().get('main_window').threads.get(thread_name) + return Registry().get('application').worker_threads.get(thread_name) def is_thread_finished(thread_name): @@ -88,8 +88,8 @@ def is_thread_finished(thread_name): :param str thread_name: The name of the thread :returns: True if the thread is finished, False if it is still running """ - main_window = Registry().get('main_window') - return thread_name not in main_window.threads or main_window.threads[thread_name]['thread'].isFinished() + app = Registry().get('application') + return thread_name not in app.worker_threads or app.worker_threads[thread_name]['thread'].isFinished() def make_remove_thread(thread_name): @@ -105,7 +105,7 @@ def make_remove_thread(thread_name): :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] + application = Registry().get('application') + if thread_name in application.worker_threads: + del application.worker_threads[thread_name] return remove_thread diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 54d91d36e..43357c837 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -477,7 +477,6 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): """ super(MainWindow, self).__init__() Registry().register('main_window', self) - self.threads = {} self.clipboard = self.application.clipboard() self.arguments = ''.join(self.application.args) # Set up settings sections for the main application (not for use by plugins). @@ -557,11 +556,11 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): wait_dialog.setAutoClose(False) wait_dialog.setCancelButton(None) wait_dialog.show() - for thread_name in self.threads.keys(): + for thread_name in self.application.worker_threads.keys(): log.debug('Waiting for thread %s', thread_name) self.application.processEvents() - thread = self.threads[thread_name]['thread'] - worker = self.threads[thread_name]['worker'] + thread = self.application.worker_threads[thread_name]['thread'] + worker = self.application.worker_threads[thread_name]['worker'] try: if worker and hasattr(worker, 'stop'): # If the worker has a stop method, run it diff --git a/tests/functional/openlp_core/test_threading.py b/tests/functional/openlp_core/test_threading.py index 382774e51..bd8ff4891 100644 --- a/tests/functional/openlp_core/test_threading.py +++ b/tests/functional/openlp_core/test_threading.py @@ -47,9 +47,9 @@ def test_run_thread_exists(MockRegistry): Test that trying to run a thread with a name that already exists will throw a KeyError """ # GIVEN: A mocked registry with a main window object - mocked_main_window = MagicMock() - mocked_main_window.threads = {'test_thread': MagicMock()} - MockRegistry.return_value.get.return_value = mocked_main_window + mocked_application = MagicMock() + mocked_application.worker_threads = {'test_thread': MagicMock()} + MockRegistry.return_value.get.return_value = mocked_application # WHEN: run_thread() is called try: @@ -66,18 +66,19 @@ def test_run_thread(MockRegistry, MockQThread): Test that running a thread works correctly """ # GIVEN: A mocked registry with a main window object - mocked_main_window = MagicMock() - mocked_main_window.threads = {} - MockRegistry.return_value.get.return_value = mocked_main_window + mocked_application = MagicMock() + mocked_application.worker_threads = {} + MockRegistry.return_value.get.return_value = mocked_application # WHEN: run_thread() is called run_thread(MagicMock(), 'test_thread') # THEN: The thread should be in the threads list and the correct methods should have been called - assert len(mocked_main_window.threads.keys()) == 1, 'There should be 1 item in the list of threads' - assert list(mocked_main_window.threads.keys()) == ['test_thread'], 'The test_thread item should be in the list' - mocked_worker = mocked_main_window.threads['test_thread']['worker'] - mocked_thread = mocked_main_window.threads['test_thread']['thread'] + assert len(mocked_application.worker_threads.keys()) == 1, 'There should be 1 item in the list of threads' + assert list(mocked_application.worker_threads.keys()) == ['test_thread'], \ + 'The test_thread item should be in the list' + mocked_worker = mocked_application.worker_threads['test_thread']['worker'] + mocked_thread = mocked_application.worker_threads['test_thread']['thread'] mocked_worker.moveToThread.assert_called_once_with(mocked_thread) mocked_thread.started.connect.assert_called_once_with(mocked_worker.start) expected_quit_calls = [call(mocked_thread.quit), call(mocked_worker.deleteLater)]