Skip to content

Commit

Permalink
gui: fix memory leak in SongMinicardListModel (#822)
Browse files Browse the repository at this point in the history
  • Loading branch information
cosven committed Apr 14, 2024
1 parent ae5910c commit b16d0bc
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 6 deletions.
12 changes: 8 additions & 4 deletions feeluown/gui/components/player_playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@


class PlayerPlaylistModel(SongMiniCardListModel):
# FIXME: make this singleton
"""
this is a singleton class (ensured by PlayerPlaylistView)
"""

def __init__(self, playlist, *args, **kwargs):
reader = create_reader(playlist.list())
Expand Down Expand Up @@ -47,17 +49,19 @@ def on_songs_removed(self, index, count):

class PlayerPlaylistView(SongMiniCardListView):

_model = None

def __init__(self, app, *args, **kwargs):
super().__init__(*args, **kwargs)
self._app = app

self.play_song_needed.connect(self._app.playlist.play_model)
self.setModel(
PlayerPlaylistModel(
if PlayerPlaylistView._model is None:
PlayerPlaylistView._model = PlayerPlaylistModel(
self._app.playlist,
fetch_cover_wrapper(self._app),
)
)
self.setModel(PlayerPlaylistView._model)

def contextMenuEvent(self, e):
index = self.indexAt(e.pos())
Expand Down
12 changes: 10 additions & 2 deletions feeluown/gui/widgets/song_minicard_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def __init__(self, fetch_image, parent=None):
self._items = []
self.fetch_image = fetch_image
self.pixmaps = {} # {uri: (Option<pixmap>, Option<color>)}
self.rowsAboutToBeRemoved.connect(self.on_rows_about_to_be_removed)

def rowCount(self, _=QModelIndex()):
return len(self._items)
Expand All @@ -45,7 +46,7 @@ def _fetch_image_callback(self, item):
# TODO: duplicate code with ImgListModel
def cb(content):
uri = reverse(item)
if content is None:
if content is None and uri in self.pixmaps:
self.pixmaps[uri] = (self.pixmaps[uri][1], None)
return

Expand Down Expand Up @@ -87,8 +88,15 @@ def data(self, index, role=Qt.DisplayRole):
return (song, pixmap)
return None

def on_rows_about_to_be_removed(self, _, first, last):
for i in range(first, last+1):
item = self._items[i]
uri = reverse(item)
# clear pixmap cache
self.pixmaps.pop(uri, None)


class SongMiniCardListModel(BaseSongMiniCardListModel, ReaderFetchMoreMixin):
class SongMiniCardListModel(ReaderFetchMoreMixin, BaseSongMiniCardListModel):
def __init__(self, reader, fetch_image, parent=None):
super().__init__(fetch_image, parent)

Expand Down
24 changes: 24 additions & 0 deletions tests/gui/widgets/test_song_minicard_list.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import pytest
from PyQt5.QtCore import QModelIndex

from feeluown.utils import aio
from feeluown.utils.reader import create_reader
from feeluown.gui.widgets.song_minicard_list import SongMiniCardListModel


@pytest.mark.asyncio
async def test_song_mini_card_list_model_remove_pixmap(qtbot, song):
async def fetch_cover(*_): return b'image content'
model = SongMiniCardListModel(create_reader([song]), fetch_cover)
assert model.rowCount() == 0
model.fetchMore(QModelIndex())
await aio.sleep(0.1)
assert model.rowCount() == 1
model.get_pixmap_unblocking(song)
await aio.sleep(0.1)
assert len(model.pixmaps) == 1
with qtbot.waitSignal(model.rowsAboutToBeRemoved):
model.beginRemoveRows(QModelIndex(), 0, 0)
model._items.pop(0)
model.endRemoveRows()
assert len(model.pixmaps) == 0

0 comments on commit b16d0bc

Please sign in to comment.