Fix various leaks (#1672)

* Fix wrong hook being torn down

* Fix item models not being destroyed

* Add missing gc for FilteredDeckConfigDialog

* Add missing type annotation

* Pass calling widget as parent to QTimer

Implicitly passing `self.mw` as the parent means that the QTimer won't
get destroyed before quitting the app, which also thwarts garbage
collection of any data captured by a passed closure.

* Make `Editor._links` an instance variable

Browser is inserting a closure into this dict capturing itself. As a class
variable, it won't get destroyed, so neither will the browser.

* Make `Editor._links` funcs take instance again

* Deprecate calling progress.timer() without parent

* show caller location when printing deprecation warning (dae)
This commit is contained in:
RumovZ 2022-02-18 10:00:12 +01:00 committed by GitHub
parent 14af96d580
commit 7741475ae0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 85 additions and 46 deletions

View File

@ -1150,7 +1150,9 @@ 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)
self.mgr.mw.progress.timer(
50, lambda: self.on_done(self.log), False, parent=self
)
def show_log_to_user(parent: QWidget, log: list[DownloadLogEntry]) -> None:
@ -1404,6 +1406,7 @@ def check_for_updates(
lambda: update_info_received(future),
False,
requiresCollection=False,
parent=mgr.mw,
)
return

View File

@ -562,7 +562,7 @@ class Browser(QMainWindow):
# schedule sidebar to refresh after browser window has loaded, so the
# UI is more responsive
self.mw.progress.timer(10, self.sidebar.refresh, False)
self.mw.progress.timer(10, self.sidebar.refresh, False, parent=self.sidebar)
def showSidebar(self) -> None:
self.sidebarDockWidget.setVisible(True)
@ -899,7 +899,7 @@ class Browser(QMainWindow):
def teardownHooks(self) -> None:
gui_hooks.undo_state_did_change.remove(self.on_undo_state_change)
gui_hooks.backend_will_block.remove(self.table.on_backend_will_block)
gui_hooks.backend_did_block.remove(self.table.on_backend_will_block)
gui_hooks.backend_did_block.remove(self.table.on_backend_did_block)
gui_hooks.operation_did_execute.remove(self.on_operation_did_execute)
gui_hooks.focus_did_change.remove(self.on_focus_change)
gui_hooks.flag_label_did_change.remove(self._update_flag_labels)

View File

@ -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)
self.mw.progress.timer(100, self._on_close, False, parent=self)
def _on_replay_audio(self) -> None:
if self._state == "question":
@ -156,7 +156,7 @@ class Previewer(QDialog):
delay = 300
if elap_ms < delay:
self._timer = self.mw.progress.timer(
delay - elap_ms, self._render_scheduled, False
delay - elap_ms, self._render_scheduled, False, parent=self
)
else:
self._render_scheduled()

View File

@ -15,7 +15,7 @@ class SidebarModel(QAbstractItemModel):
def __init__(
self, sidebar: aqt.browser.sidebar.SidebarTreeView, root: SidebarItem
) -> None:
super().__init__()
super().__init__(sidebar)
self.sidebar = sidebar
self.root = root
self._cache_rows(root)

View File

@ -177,6 +177,8 @@ class SidebarTreeView(QTreeView):
# block repainting during refreshing to avoid flickering
self.setUpdatesEnabled(False)
if old_model := self.model():
old_model.deleteLater()
model = SidebarModel(self, root)
self.setModel(model)

View File

@ -34,12 +34,13 @@ class DataModel(QAbstractTableModel):
def __init__(
self,
parent: QObject,
col: Collection,
state: ItemState,
row_state_will_change_callback: Callable,
row_state_changed_callback: Callable,
) -> None:
QAbstractTableModel.__init__(self)
super().__init__(parent)
self.col: Collection = col
self.columns: dict[str, Column] = {
c.key: c for c in self.col.all_browser_columns()

View File

@ -40,6 +40,7 @@ class Table:
else CardState(self.col)
)
self._model = DataModel(
self.browser,
self.col,
self._state,
self._on_row_state_will_change,

View File

@ -499,7 +499,9 @@ class CardLayout(QDialog):
def renderPreview(self) -> None:
# schedule a preview when timing stops
self.cancelPreviewTimer()
self._previewTimer = self.mw.progress.timer(200, self._renderPreview, False)
self._previewTimer = self.mw.progress.timer(
200, self._renderPreview, False, parent=self
)
def cancelPreviewTimer(self) -> None:
if self._previewTimer:

View File

@ -123,6 +123,7 @@ class Editor:
self.last_field_index: int | None = None
# current card, for card layout
self.card: Card | None = None
self._init_links()
self.setupOuter()
self.setupWeb()
self.setupShortcuts()
@ -394,7 +395,9 @@ require("anki/ui").loaded.then(() => require("anki/NoteEditor").instances[0].too
if gui_hooks.editor_did_unfocus_field(False, self.note, ord):
# something updated the note; update it after a subsequent focus
# event has had time to fire
self.mw.progress.timer(100, self.loadNoteKeepingFocus, False)
self.mw.progress.timer(
100, self.loadNoteKeepingFocus, False, parent=self.widget
)
else:
self._check_and_update_duplicate_display_async()
else:
@ -549,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)
self.mw.progress.timer(10, callback, False, parent=self.widget)
return
self.web.evalWithCallback("saveNow(%d)" % keepFocus, lambda res: callback())
@ -1104,29 +1107,30 @@ require("anki/ui").loaded.then(() => require("anki/NoteEditor").instances[0].too
# Links from HTML
######################################################################
_links: dict[str, Callable] = dict(
fields=onFields,
cards=onCardLayout,
bold=toggleBold,
italic=toggleItalic,
underline=toggleUnderline,
super=toggleSuper,
sub=toggleSub,
clear=removeFormat,
colour=onForeground,
changeCol=onChangeCol,
cloze=onCloze,
attach=onAddMedia,
record=onRecSound,
more=onAdvanced,
dupes=showDupes,
paste=onPaste,
cutOrCopy=onCutOrCopy,
htmlEdit=onHtmlEdit,
mathjaxInline=insertMathjaxInline,
mathjaxBlock=insertMathjaxBlock,
mathjaxChemistry=insertMathjaxChemistry,
)
def _init_links(self) -> None:
self._links: dict[str, Callable] = dict(
fields=Editor.onFields,
cards=Editor.onCardLayout,
bold=Editor.toggleBold,
italic=Editor.toggleItalic,
underline=Editor.toggleUnderline,
super=Editor.toggleSuper,
sub=Editor.toggleSub,
clear=Editor.removeFormat,
colour=Editor.onForeground,
changeCol=Editor.onChangeCol,
cloze=Editor.onCloze,
attach=Editor.onAddMedia,
record=Editor.onRecSound,
more=Editor.onAdvanced,
dupes=Editor.showDupes,
paste=Editor.onPaste,
cutOrCopy=Editor.onCutOrCopy,
htmlEdit=Editor.onHtmlEdit,
mathjaxInline=Editor.insertMathjaxInline,
mathjaxBlock=Editor.insertMathjaxBlock,
mathjaxChemistry=Editor.insertMathjaxChemistry,
)
# Pasting, drag & drop, and keyboard layouts

View File

@ -50,6 +50,7 @@ class FilteredDeckConfigDialog(QDialog):
QDialog.__init__(self, mw)
self.mw = mw
mw.garbage_collect_on_dialog_finish(self)
self.col = self.mw.col
self._desired_search_1 = search
self._desired_search_2 = search_2

View File

@ -187,7 +187,9 @@ class AnkiQt(QMainWindow):
fn()
gui_hooks.main_window_did_init()
self.progress.timer(10, on_window_init, False, requiresCollection=False)
self.progress.timer(
10, on_window_init, False, requiresCollection=False, parent=self
)
def setupUI(self) -> None:
self.col = None
@ -226,6 +228,7 @@ class AnkiQt(QMainWindow):
self.setupProfileAfterWebviewsLoaded,
False,
requiresCollection=False,
parent=self,
)
return
else:
@ -911,7 +914,7 @@ title="{}" {}>{}</button>""".format(
self.col.db.rollback()
self.close()
self.progress.timer(100, quit, False)
self.progress.timer(100, quit, False, parent=self)
def setupProgress(self) -> None:
self.progress = aqt.progress.ProgressManager(self)
@ -1062,6 +1065,7 @@ title="{}" {}>{}</button>""".format(
theme_manager.apply_style_if_system_style_changed,
True,
False,
parent=self,
)
def set_theme(self, theme: Theme) -> None:
@ -1354,14 +1358,16 @@ title="{}" {}>{}</button>""".format(
def setup_timers(self) -> None:
# refresh decks every 10 minutes
self.progress.timer(10 * 60 * 1000, self.onRefreshTimer, True)
self.progress.timer(10 * 60 * 1000, self.onRefreshTimer, True, parent=self)
# check media sync every 5 minutes
self.progress.timer(5 * 60 * 1000, self.on_autosync_timer, True)
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)
self.progress.timer(
15 * 60 * 1000, self.garbage_collect_now, False, parent=self
)
# ensure Python interpreter runs at least once per second, so that
# SIGINT/SIGTERM is processed without a long delay
self.progress.timer(1000, lambda: None, True, False)
self.progress.timer(1000, lambda: None, True, False, parent=self)
def onRefreshTimer(self) -> None:
if self.state == "deckBrowser":
@ -1690,7 +1696,11 @@ title="{}" {}>{}</button>""".format(
if self.state == "startup":
# try again in a second
self.progress.timer(
1000, lambda: self.onAppMsg(buf), False, requiresCollection=False
1000,
lambda: self.onAppMsg(buf),
False,
requiresCollection=False,
parent=self,
)
return
elif self.state == "profileManager":
@ -1757,7 +1767,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
1000, self.garbage_collect_now, False, requiresCollection=False, parent=self
)
def disable_automatic_garbage_collection(self) -> None:

View File

@ -5,6 +5,7 @@ from __future__ import annotations
import time
import aqt.forms
from anki._legacy import print_deprecation_warning
from aqt.qt import *
from aqt.utils import disable_help_button, tr
@ -29,7 +30,13 @@ class ProgressManager:
# (likely due to some long-running DB operation)
def timer(
self, ms: int, func: Callable, repeat: bool, requiresCollection: bool = True
self,
ms: int,
func: Callable,
repeat: bool,
requiresCollection: bool = True,
*,
parent: QObject = None,
) -> QTimer:
"""Create and start a standard Anki timer.
@ -42,6 +49,12 @@ class ProgressManager:
timer to fire even when there is no collection, but will still
only fire when there is no current progress dialog."""
if parent is None:
print_deprecation_warning(
"to avoid memory leaks, pass an appropriate parent to progress.timer()"
)
parent = self.mw
def handler() -> None:
if requiresCollection and not self.mw.col:
# no current collection; timer is no longer valid
@ -59,7 +72,7 @@ class ProgressManager:
# retry in 100ms
self.timer(100, func, False, requiresCollection)
t = QTimer(self.mw)
t = QTimer(parent)
if not repeat:
t.setSingleShot(True)
qconnect(t.timeout, handler)

View File

@ -537,7 +537,9 @@ def ensureWidgetInScreenBoundaries(widget: QWidget) -> None:
handle = widget.window().windowHandle()
if not handle:
# window has not yet been shown, retry later
aqt.mw.progress.timer(50, lambda: ensureWidgetInScreenBoundaries(widget), False)
aqt.mw.progress.timer(
50, lambda: ensureWidgetInScreenBoundaries(widget), False, parent=widget
)
return
# ensure widget is smaller than screen bounds
@ -745,7 +747,7 @@ def tooltip(
lab.move(aw.mapToGlobal(QPoint(0 + x_offset, aw.height() - y_offset)))
lab.show()
_tooltipTimer = aqt.mw.progress.timer(
period, closeTooltip, False, requiresCollection=False
period, closeTooltip, False, requiresCollection=False, parent=aw
)
_tooltipLabel = lab

View File

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