From 190d977b3f998f9da3a535ad49f967d1ccc97f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Wed, 3 Sep 2025 15:27:03 -0400 Subject: [PATCH 1/9] Return to a quit/start pattern for thread management --- qtapputils/managers/taskmanagers.py | 31 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index 82b2d4f..71d52dd 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -67,6 +67,8 @@ def run_tasks(self): self.sig_task_completed.emit(task_uuid4, returned_values) self._tasks.clear() + self.thread().quit() + self.thread().wait() class TaskManagerBase(QObject): @@ -75,7 +77,6 @@ class TaskManagerBase(QObject): """ sig_run_tasks_started = Signal() sig_run_tasks_finished = Signal() - sig_run_tasks = Signal() def __init__(self, verbose: bool = False): super().__init__() @@ -133,15 +134,12 @@ def worker(self) -> WorkerBase: def set_worker(self, worker: WorkerBase): """"Install the provided worker on this manager""" self._thread = QThread() - self._worker = worker + self._worker.moveToThread(self._thread) - self._worker.sig_task_completed.connect( - self._handle_task_completed, Qt.QueuedConnection) - self.sig_run_tasks.connect( - self._worker.run_tasks, Qt.QueuedConnection) + self._thread.started.connect(self._worker.run_tasks) - self._thread.start() + self._worker.sig_task_completed.connect(self._handle_task_completed) # ---- Private API def _handle_task_completed( @@ -203,19 +201,24 @@ def _run_pending_tasks(self): print('Executing {} pending tasks...'.format( len(self._pending_tasks))) + # Even though the worker has executed all its tasks, + # we may still need to wait a little for it to stop properly. + while self._thread.isRunning(): + print("Waiting {}'s working thread to quit..." + .format(self.__class__.__name__)) + try: + qtwait(lambda: not self._thread.isRunning(), timeout=10) + except TimeoutError: + print("Error: unable to stop {}'s working thread." + .format(self.__class__.__name__)) + self._running_tasks = self._pending_tasks.copy() self._pending_tasks = [] for task_uuid4 in self._running_tasks: task, args, kargs = self._task_data[task_uuid4] self._worker.add_task(task_uuid4, task, *args, **kargs) - self.sig_run_tasks.emit() - - def close(self): - if hasattr(self, "_thread"): - qtwait(lambda: not self.is_running) - self._thread.quit() - self._thread.wait() + self._thread.start() class LIFOTaskManager(TaskManagerBase): From 69b7abe4a5fd1780582529ba0ea2800566bcecbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Wed, 3 Sep 2025 15:47:43 -0400 Subject: [PATCH 2/9] Call quit and wait on thread from manager (not worker) --- qtapputils/managers/taskmanagers.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index 71d52dd..a7a7242 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -67,8 +67,6 @@ def run_tasks(self): self.sig_task_completed.emit(task_uuid4, returned_values) self._tasks.clear() - self.thread().quit() - self.thread().wait() class TaskManagerBase(QObject): @@ -201,16 +199,8 @@ def _run_pending_tasks(self): print('Executing {} pending tasks...'.format( len(self._pending_tasks))) - # Even though the worker has executed all its tasks, - # we may still need to wait a little for it to stop properly. - while self._thread.isRunning(): - print("Waiting {}'s working thread to quit..." - .format(self.__class__.__name__)) - try: - qtwait(lambda: not self._thread.isRunning(), timeout=10) - except TimeoutError: - print("Error: unable to stop {}'s working thread." - .format(self.__class__.__name__)) + self._thread.quit() + self._thread.wait() self._running_tasks = self._pending_tasks.copy() self._pending_tasks = [] From 862b0a476fce59f3b0fe67478fa7538a537ee44f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Wed, 3 Sep 2025 16:29:06 -0400 Subject: [PATCH 3/9] Rework TaskManagerBase._handle_task_completed --- qtapputils/managers/taskmanagers.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index a7a7242..554bb8f 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -159,14 +159,19 @@ def _handle_task_completed( # Clean up completed task. self._cleanup_task(task_uuid4) - # Execute pending tasks if worker is idle. - if len(self._running_tasks) == 0: - if len(self._pending_tasks) > 0: - self._run_pending_tasks() - else: - if self.verbose: - print('All pending tasks were executed.') - self.sig_run_tasks_finished.emit() + if len(self._running_tasks) > 0: + return + + # Tells the thread's event loop to exit since all running + # tasks were completed. + self._thread.quit() + + if len(self._pending_tasks) > 0: + self._run_pending_tasks() + else: + if self.verbose: + print('All pending tasks were executed.') + self.sig_run_tasks_finished.emit() def _cleanup_task(self, task_uuid4: uuid.UUID): """Cleanup task associated with the specified UUID.""" From 7ea676403cfd25d715465bedd8e0a81ba1308f35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Wed, 3 Sep 2025 16:35:07 -0400 Subject: [PATCH 4/9] Rework TaskManagerBase._run_pending_tasks --- qtapputils/managers/taskmanagers.py | 36 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index 554bb8f..e823a06 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -199,21 +199,33 @@ def _run_tasks(self): def _run_pending_tasks(self): """Execute all pending tasks.""" - if len(self._running_tasks) == 0 and len(self._pending_tasks) > 0: - if self.verbose: - print('Executing {} pending tasks...'.format( - len(self._pending_tasks))) + # If the worker is busy, postpone running pending tasks. Pending tasks + # will be run when all running tasks are done running. + if len(self._running_tasks) > 0: + return + + # If there are no pending tasks, nothing to do. + if len(self._pending_tasks) == 0: + return + + if self.verbose: + print(f'Executing {len(self._pending_tasks)} pending tasks...') - self._thread.quit() - self._thread.wait() + # Make sur the thread has finished execution before proceeding + # with pending tasks. + self._thread.wait() + + # Move all pending tasks to running tasks. + self._running_tasks = self._pending_tasks.copy() + self._pending_tasks = [] - self._running_tasks = self._pending_tasks.copy() - self._pending_tasks = [] - for task_uuid4 in self._running_tasks: - task, args, kargs = self._task_data[task_uuid4] - self._worker.add_task(task_uuid4, task, *args, **kargs) + # Queue each running task for the worker. + for task_uuid4 in self._running_tasks: + task, args, kargs = self._task_data[task_uuid4] + self._worker.add_task(task_uuid4, task, *args, **kargs) - self._thread.start() + # Start the thread so the worker can process the tasks. + self._thread.start() class LIFOTaskManager(TaskManagerBase): From 86efef3a1ccc2dcd3da09f751e588da17e0d7a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Thu, 4 Sep 2025 09:33:00 -0400 Subject: [PATCH 5/9] Use a non blocking while waiting for qthread to close --- qtapputils/managers/taskmanagers.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index e823a06..9e92f9f 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -211,9 +211,10 @@ def _run_pending_tasks(self): if self.verbose: print(f'Executing {len(self._pending_tasks)} pending tasks...') - # Make sur the thread has finished execution before proceeding - # with pending tasks. - self._thread.wait() + # Ensure the thread is not running before starting new tasks. + # This prevents starting a thread that is already active, which can + # cause errors. + qtwait(lambda: self._tread.isRunning()) # Move all pending tasks to running tasks. self._running_tasks = self._pending_tasks.copy() From 24f2b7194eeae6c3b568da07b5aadc0406ca3a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Thu, 4 Sep 2025 09:33:28 -0400 Subject: [PATCH 6/9] Improve comments in the code --- qtapputils/managers/taskmanagers.py | 30 ++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index 9e92f9f..bafdb4e 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -148,27 +148,35 @@ def _handle_task_completed( This is the ONLY slot that should be called after a task is completed by the worker. """ - # Run the callback associated with the specified task UUID if any. - if self._task_callbacks[task_uuid4] is not None: + # Execute the callback associated with this task (if one exists). + callback = self._task_callbacks.get(task_uuid4) + if callback is not None: try: - self._task_callbacks[task_uuid4](*returned_values) + callback(*returned_values) except TypeError: # This means there is 'returned_values' is None. - self._task_callbacks[task_uuid4]() + callback() - # Clean up completed task. + # Remove references to the completed task from internal structures. self._cleanup_task(task_uuid4) + # If there are still running tasks, do not proceed further. if len(self._running_tasks) > 0: return - # Tells the thread's event loop to exit since all running - # tasks were completed. + # We quit the thread here to ensure all resources are cleaned up + # and to prevent issues with lingering events or stale object + # references. This makes the worker lifecycle simpler and more robust, + # especially in PyQt/PySide, and avoids subtle bugs that can arise + # from reusing threads across multiple batches. self._thread.quit() + # If there are pending tasks, begin processing them. if len(self._pending_tasks) > 0: self._run_pending_tasks() else: + # No pending tasks remain; notify listeners that + # all tasks are finished. if self.verbose: print('All pending tasks were executed.') self.sig_run_tasks_finished.emit() @@ -199,8 +207,8 @@ def _run_tasks(self): def _run_pending_tasks(self): """Execute all pending tasks.""" - # If the worker is busy, postpone running pending tasks. Pending tasks - # will be run when all running tasks are done running. + # If the worker is currently processing tasks, defer execution of + # pending tasks. if len(self._running_tasks) > 0: return @@ -216,11 +224,11 @@ def _run_pending_tasks(self): # cause errors. qtwait(lambda: self._tread.isRunning()) - # Move all pending tasks to running tasks. + # Move all pending tasks to the running tasks queue. self._running_tasks = self._pending_tasks.copy() self._pending_tasks = [] - # Queue each running task for the worker. + # Add each running task to the worker's queue. for task_uuid4 in self._running_tasks: task, args, kargs = self._task_data[task_uuid4] self._worker.add_task(task_uuid4, task, *args, **kargs) From 0e6890bed397b22d8fbc8c5fc9396bbc44b88610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Thu, 4 Sep 2025 09:41:14 -0400 Subject: [PATCH 7/9] Add a 'wait' method to 'TaskManagerBase' --- qtapputils/managers/taskmanagers.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index bafdb4e..ab28e3f 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -103,6 +103,16 @@ def __init__(self, verbose: bool = False): def is_running(self): return len(self._running_tasks + self._pending_tasks) > 0 + def wait(self): + """ + Waits for completion of all running and pending tasks, and for the + worker thread to fully exit its event loop. + + Note: This does not block the main GUI event loop, allowing the UI to + remain responsive while waiting. + """ + qtwait(lambda: not self.is_running and not self._thread.isRunning()) + def run_tasks( self, callback: Callable = None, returned_values: tuple = None): """ From 697eba92e6f1b5012dfae994a4a74f190c6cf81a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Thu, 4 Sep 2025 09:41:34 -0400 Subject: [PATCH 8/9] Fix the logic that wait for the thread to quit --- qtapputils/managers/taskmanagers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qtapputils/managers/taskmanagers.py b/qtapputils/managers/taskmanagers.py index ab28e3f..661821e 100644 --- a/qtapputils/managers/taskmanagers.py +++ b/qtapputils/managers/taskmanagers.py @@ -232,7 +232,8 @@ def _run_pending_tasks(self): # Ensure the thread is not running before starting new tasks. # This prevents starting a thread that is already active, which can # cause errors. - qtwait(lambda: self._tread.isRunning()) + if self._thread.isRunning(): + qtwait(lambda: not self._thread.isRunning()) # Move all pending tasks to the running tasks queue. self._running_tasks = self._pending_tasks.copy() From e448d958705c24d774069bb5ee8463f08e4d6a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Gosselin?= Date: Thu, 4 Sep 2025 09:41:44 -0400 Subject: [PATCH 9/9] Update test_taskmanagers.py --- qtapputils/managers/tests/test_taskmanagers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qtapputils/managers/tests/test_taskmanagers.py b/qtapputils/managers/tests/test_taskmanagers.py index 85936b9..3d9ee7b 100644 --- a/qtapputils/managers/tests/test_taskmanagers.py +++ b/qtapputils/managers/tests/test_taskmanagers.py @@ -52,7 +52,8 @@ def task_manager(worker, qtbot): task_manager.set_worker(worker) yield task_manager - task_manager.close() + task_manager.wait() + assert not task_manager.is_running assert not task_manager._thread.isRunning() @@ -63,7 +64,8 @@ def lifo_task_manager(worker, qtbot): task_manager.set_worker(worker) yield task_manager - task_manager.close() + task_manager.wait() + assert not task_manager.is_running assert not task_manager._thread.isRunning()