Add progress.single_shot() (#1683)

* Add progress.single_shot()

* Fix periodic garbage collection

* Properly cleanup mediasync timers

* Revert some replacements with `single_shot()`

These timers shouldn't fire if their widget is destroyed.

* Add timer docs explaining issues and alternatives

* Apply suggestions from code review

* Tweak docstrings

qt/aqt/addons.py

@@ -1150,9 +1150,7 @@ class DownloaderInstaller(QObject):
         self.mgr.mw.progress.finish()
         # qt gets confused if on_done() opens new windows while the progress
         # modal is still cleaning up
-        self.mgr.mw.progress.timer(
-            50, lambda: self.on_done(self.log), False, parent=self
-        )
+        self.mgr.mw.progress.single_shot(50, lambda: self.on_done(self.log))
 
 
 def show_log_to_user(parent: QWidget, log: list[DownloadLogEntry]) -> None:
@@ -1401,12 +1399,10 @@ def check_for_updates(
     def update_info_received(future: Future) -> None:
         # if syncing/in profile screen, defer message delivery
         if not mgr.mw.col:
-            mgr.mw.progress.timer(
+            mgr.mw.progress.single_shot(
                 1000,
                 lambda: update_info_received(future),
                 False,
-                requiresCollection=False,
-                parent=mgr.mw,
             )
             return
 

qt/aqt/browser/previewer.py

@@ -106,7 +106,7 @@ class Previewer(QDialog):
 
     def _on_finished(self, ok: int) -> None:
         saveGeom(self, "preview")
-        self.mw.progress.timer(100, self._on_close, False, parent=self)
+        self.mw.progress.single_shot(100, self._on_close)
 
     def _on_replay_audio(self) -> None:
         if self._state == "question":

qt/aqt/editor.py

@@ -552,7 +552,7 @@ require("anki/ui").loaded.then(() => require("anki/NoteEditor").instances[0].too
         "Save unsaved edits then call callback()."
         if not self.note:
             # calling code may not expect the callback to fire immediately
-            self.mw.progress.timer(10, callback, False, parent=self.widget)
+            self.mw.progress.single_shot(10, callback)
             return
         self.web.evalWithCallback("saveNow(%d)" % keepFocus, lambda res: callback())
 

qt/aqt/main.py

@@ -187,9 +187,7 @@ class AnkiQt(QMainWindow):
             fn()
             gui_hooks.main_window_did_init()
 
-        self.progress.timer(
-            10, on_window_init, False, requiresCollection=False, parent=self
-        )
+        self.progress.single_shot(10, on_window_init, False)
 
     def setupUI(self) -> None:
         self.col = None
@@ -223,12 +221,10 @@ class AnkiQt(QMainWindow):
     def setupProfileAfterWebviewsLoaded(self) -> None:
         for w in (self.web, self.bottomWeb):
             if not w._domDone:
-                self.progress.timer(
+                self.progress.single_shot(
                     10,
                     self.setupProfileAfterWebviewsLoaded,
                     False,
-                    requiresCollection=False,
-                    parent=self,
                 )
                 return
             else:
@@ -914,7 +910,7 @@ title="{}" {}>{}</button>""".format(
             self.col.db.rollback()
             self.close()
 
-        self.progress.timer(100, quit, False, parent=self)
+        self.progress.single_shot(100, quit)
 
     def setupProgress(self) -> None:
         self.progress = aqt.progress.ProgressManager(self)
@@ -1361,7 +1357,7 @@ title="{}" {}>{}</button>""".format(
         self.progress.timer(5 * 60 * 1000, self.on_autosync_timer, True, parent=self)
         # periodic garbage collection
         self.progress.timer(
-            15 * 60 * 1000, self.garbage_collect_now, False, parent=self
+            15 * 60 * 1000, self.garbage_collect_now, True, False, parent=self
         )
         # ensure Python interpreter runs at least once per second, so that
         # SIGINT/SIGTERM is processed without a long delay
@@ -1693,12 +1689,10 @@ title="{}" {}>{}</button>""".format(
 
         if self.state == "startup":
             # try again in a second
-            self.progress.timer(
+            self.progress.single_shot(
                 1000,
                 lambda: self.onAppMsg(buf),
                 False,
-                requiresCollection=False,
-                parent=self,
             )
             return
         elif self.state == "profileManager":
@@ -1764,9 +1758,7 @@ title="{}" {}>{}</button>""".format(
 
     def deferred_delete_and_garbage_collect(self, obj: QObject) -> None:
         obj.deleteLater()
-        self.progress.timer(
-            1000, self.garbage_collect_now, False, requiresCollection=False, parent=self
-        )
+        self.progress.single_shot(1000, self.garbage_collect_now, False)
 
     def disable_automatic_garbage_collection(self) -> None:
         gc.collect()

qt/aqt/mediasync.py

@@ -78,7 +78,7 @@ class MediaSyncer:
     def _on_finished(self, future: Future) -> None:
         self._syncing = False
         if self._progress_timer:
-            self._progress_timer.stop()
+            self._progress_timer.deleteLater()
             self._progress_timer = None
         gui_hooks.media_sync_did_start_or_stop(False)
 
@@ -131,7 +131,7 @@ class MediaSyncer:
 
         def check_finished() -> None:
             if not self.is_syncing():
-                timer.stop()
+                timer.deleteLater()
                 on_finished()
 
         timer = self.mw.progress.timer(150, check_finished, True, False, parent=self.mw)

qt/aqt/progress.py

@@ -26,7 +26,7 @@ class ProgressManager:
 
     # Safer timers
     ##########################################################################
-    # A custom timer which avoids firing while a progress dialog is active
+    # Custom timers which avoid firing while a progress dialog is active
     # (likely due to some long-running DB operation)
 
     def timer(
@@ -38,7 +38,7 @@ class ProgressManager:
         *,
         parent: QObject = None,
     ) -> QTimer:
-        """Create and start a standard Anki timer.
+        """Create and start a standard Anki timer. For an alternative see `single_shot()`.
 
         If the timer fires while a progress window is shown:
         - if it is a repeating timer, it will wait the same delay again
@@ -47,16 +47,65 @@ class ProgressManager:
         If requiresCollection is True, the timer will not fire if the
         collection has been unloaded. Setting it to False will allow the
         timer to fire even when there is no collection, but will still
-        only fire when there is no current progress dialog."""
+        only fire when there is no current progress dialog.
+
+
+        Issues and alternative
+        ---
+        The created timer will only be destroyed when `parent` is destroyed.
+        This can cause memory leaks, because anything captured by `func` isn't freed either.
+        If there is no QObject that will get destroyed reasonably soon, and you have to
+        pass `mw`, you should call `deleteLater()` on the returned QTimer as soon as
+        it's served its purpose, or use `single_shot()`.
+
+        Also note that you may not be able to pass an adequate parent, if you want to
+        make a callback after a widget closes. If you passed that widget, the timer
+        would get destroyed before it could fire.
+        """
 
         if parent is None:
             print_deprecation_warning(
                 "to avoid memory leaks, pass an appropriate parent to progress.timer()"
+                " or use progress.single_shot()"
             )
             parent = self.mw
 
+        qtimer = QTimer(parent)
+        if not repeat:
+            qtimer.setSingleShot(True)
+        qconnect(qtimer.timeout, self._get_handler(func, repeat, requiresCollection))
+        qtimer.start(ms)
+        return qtimer
+
+    def single_shot(
+        self,
+        ms: int,
+        func: Callable[[], None],
+        requires_collection: bool = True,
+    ) -> None:
+        """Create and start a one-off Anki timer. For an alternative and more
+        documentation, see `timer()`.
+
+
+        Issues and alternative
+        ---
+        `single_shot()` cleans itself up, so a passed closure won't leak any memory.
+        However, if `func` references a QObject other than `mw`, which gets deleted before the
+        timer fires, an Exception is raised. To avoid this, either use `timer()` passing
+        that object as the parent, or check in `func` with `sip.isdeleted(object)` if
+        it still exists.
+
+        On the other hand, if a widget is supposed to make an external callback after it closes,
+        you likely want to use `single_shot()`, which will fire even if the calling
+        widget is already destroyed.
+        """
+        QTimer.singleShot(ms, self._get_handler(func, False, requires_collection))
+
+    def _get_handler(
+        self, func: Callable[[], None], repeat: bool, requires_collection: bool
+    ) -> Callable[[], None]:
         def handler() -> None:
-            if requiresCollection and not self.mw.col:
+            if requires_collection and not self.mw.col:
                 # no current collection; timer is no longer valid
                 print(f"Ignored progress func as collection unloaded: {repr(func)}")
                 return
@@ -70,14 +119,9 @@ class ProgressManager:
                     pass
                 else:
                     # retry in 100ms
-                    self.timer(100, func, False, requiresCollection, parent=parent)
+                    self.single_shot(100, func, requires_collection)
 
-        t = QTimer(parent)
-        if not repeat:
-            t.setSingleShot(True)
-        qconnect(t.timeout, handler)
-        t.start(ms)
-        return t
+        return handler
 
     # Creating progress dialogs
     ##########################################################################

qt/aqt/webview.py

@@ -628,7 +628,7 @@ html {{ {font} }}
 
         if qvar is None:
 
-            mw.progress.timer(1000, mw.reset, False, parent=self)
+            mw.progress.single_shot(1000, mw.reset)
             return
 
         self.setFixedHeight(int(qvar))