Merge pull request #1519 from freedomofpress/ensure-threads-are-exite…

…d-before-qthread-instances-are-destroyed-3 Ensure neither sync or queue threads are likely to be mis-managed by accident
freedomofpress · Jun 22, 2022 · 41d5a47 · 41d5a47
2 parents fcd4dc9 + 7275891
commit 41d5a47
Show file tree

Hide file tree

Showing 5 changed files with 266 additions and 217 deletions.
diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py
@@ -353,22 +353,22 @@ def __init__(  # type: ignore [no-untyped-def]
 
         if export_thread is not None:
             self.export_thread = export_thread
-        else:  # pragma: no cover
+        else:
             self.export_thread = QThread()
 
         if sync_thread is not None:
             self.sync_thread = sync_thread
-        else:  # pragma: no cover
+        else:
             self.sync_thread = QThread()
 
         if main_queue_thread is not None:
             self.main_queue_thread = main_queue_thread
-        else:  # pragma: no cover
+        else:
             self.main_queue_thread = QThread()
 
         if file_download_queue_thread is not None:
             self.file_download_queue_thread = file_download_queue_thread
-        else:  # pragma: no cover
+        else:
             self.file_download_queue_thread = QThread()
 
         # Controller is unauthenticated by default
@@ -438,7 +438,7 @@ def __init__(  # type: ignore [no-untyped-def]
 
         # Background sync to keep client up-to-date with server changes
         self.api_sync = ApiSync(
-            self.api, self.session_maker, self.gpg, self.data_dir, state, self.sync_thread
+            self.api, self.session_maker, self.gpg, self.data_dir, self.sync_thread, state
         )
         self.api_sync.sync_started.connect(self.on_sync_started, type=Qt.QueuedConnection)
         self.api_sync.sync_success.connect(self.on_sync_success, type=Qt.QueuedConnection)

diff --git a/securedrop_client/queue.py b/securedrop_client/queue.py
@@ -205,20 +205,13 @@ def __init__(
         self,
         api_client: API,
         session_maker: scoped_session,
-        main_thread: Optional[QThread] = None,
-        download_file_thread: Optional[QThread] = None,
+        main_thread: QThread,
+        download_file_thread: QThread,
     ) -> None:
         super().__init__(None)
 
-        if main_thread is not None:
-            self.main_thread = main_thread
-        else:  # pragma: no cover
-            self.main_thread = QThread()
-
-        if download_file_thread is not None:
-            self.download_file_thread = download_file_thread
-        else:  # pragma: no cover
-            self.download_file_thread = QThread()
+        self.main_thread = main_thread
+        self.download_file_thread = download_file_thread
 
         self.main_queue = RunnableQueue(api_client, session_maker)
         self.download_file_queue = RunnableQueue(api_client, session_maker)

diff --git a/securedrop_client/sync.py b/securedrop_client/sync.py
@@ -30,16 +30,14 @@ def __init__(
         session_maker: scoped_session,
         gpg: GpgHelper,
         data_dir: str,
+        sync_thread: QThread,
         app_state: Optional[state.State] = None,
-        sync_thread: Optional[QThread] = None,
     ):
         super().__init__()
         self.api_client = api_client
 
         if sync_thread is not None:
             self.sync_thread = sync_thread
-        else:  # pragma: no cover
-            self.sync_thread = QThread()
 
         self.api_sync_bg_task = ApiSyncBackgroundTask(
             api_client,