Stop AppTrail's worker polling while idle

apptrail/gui.py

@@ -31,6 +31,8 @@ def __init__(self, root, history=None):
         self.events = queue.Queue()
         self.stop = threading.Event()
         self.busy = False
+        self.closed = False
+        self.poll_id = None
         self.comparison = None
         self.buttons = []
         style = ttk.Style(root)
@@ -116,7 +118,6 @@ def __init__(self, root, history=None):
         self.progress.pack(before=bottom, side='bottom', fill='x', pady=(8, 0))
         root.protocol('WM_DELETE_WINDOW', self.close)
         self.refresh_history()
-        self.poll_id = root.after(100, self.poll)
 
     def change_theme(self):
         # Checkbutton has already changed the value; toggle saves and applies it.
@@ -406,7 +407,11 @@ def worker():
                 self.events.put(('done', job()))
             except Exception as exc:
                 self.events.put(('error', str(exc)))
-        threading.Thread(target=worker, daemon=True).start()
+        try:
+            threading.Thread(target=worker, daemon=True).start()
+        except RuntimeError as exc:
+            self.events.put(('error', str(exc)))
+        self.poll_id = self.root.after(100, self.poll)
 
     def capture(self, save_as=False):
         if self.busy:
@@ -490,6 +495,9 @@ def job():
         self.run_job(job)
 
     def poll(self):
+        self.poll_id = None
+        if self.closed:
+            return
         latest = None
         for _ in range(250):
             try:
@@ -540,9 +548,13 @@ def poll(self):
                 self.refresh()
                 self.tabs.select(self.comparison_tab)
                 self.status.set('Comparison ready · Export a report to preserve the evidence')
+            break
+        if self.closed:
+            return
         if latest:
             self.status.set(latest[:150])
-        self.poll_id = self.root.after(100, self.poll)
+        if self.busy and self.poll_id is None:
+            self.poll_id = self.root.after(100, self.poll)
 
     def refresh(self):
         if not self.comparison:
@@ -583,9 +595,15 @@ def export(self):
             webbrowser.open(Path(path).resolve().as_uri())
 
     def close(self):
+        if self.closed:
+            return
+        self.closed = True
         if self.busy:
             self.stop.set()
-        self.root.after_cancel(self.poll_id)
+            self.progress.stop()
+        if self.poll_id is not None:
+            self.root.after_cancel(self.poll_id)
+            self.poll_id = None
         self.root.destroy()
 
 

tests/test_gui.py

@@ -43,6 +43,43 @@ def finish_job(self):
             time.sleep(.01)
         self.assertFalse(self.app.busy, 'Background operation did not finish')
 
+    def test_worker_polling_runs_only_until_terminal_event(self):
+        self.assertIsNone(self.app.poll_id)
+
+        def job():
+            for _ in range(251):
+                self.app.events.put(('progress', 'Working'))
+            return ('imported', [], [])
+
+        self.app.run_job(job)
+        self.assertIsNotNone(self.app.poll_id)
+        self.finish_job()
+        self.assertIsNone(self.app.poll_id)
+        self.assertEqual(self.app.status.get(), 'Imported 0 snapshot(s) · 0 failed')
+
+    def test_close_cancels_pending_worker_poll(self):
+        with patch('apptrail.gui.threading.Thread'):
+            self.app.run_job(lambda: None)
+        callback = self.app.poll_id
+        with patch.object(self.root, 'after_cancel', wraps=self.root.after_cancel) as cancel:
+            self.app.close()
+            cancel.assert_called_once_with(callback)
+        self.assertTrue(self.app.stop.is_set())
+        self.assertIsNone(self.app.poll_id)
+
+
+    def test_close_during_worker_start_error_does_not_rearm_poll(self):
+        with patch('apptrail.gui.threading.Thread.start', side_effect=RuntimeError('Cannot start worker')):
+            self.app.run_job(lambda: None)
+        with patch('apptrail.gui.messagebox.showerror', side_effect=lambda *_: self.app.close()) as error:
+            with patch.object(self.root, 'report_callback_exception') as callback_error:
+                self.finish_job()
+                callback_error.assert_not_called()
+            error.assert_called_once_with('AppTrail', 'Cannot start worker')
+        self.assertTrue(self.app.closed)
+        self.assertIsNone(self.app.poll_id)
+
+
     def test_real_checkpoint_dialog_saves_rules_and_uses_theme(self):
         from apptrail.gui import CheckpointDialog
         def fill():