diff --git a/modules/controlcenter/player.py b/modules/controlcenter/player.py index e144fcd..538beb9 100644 --- a/modules/controlcenter/player.py +++ b/modules/controlcenter/player.py @@ -4,6 +4,8 @@ import tempfile import urllib.parse import urllib.request from typing import List +import threading +import weakref from fabric.utils import ( bulk_connect, @@ -69,6 +71,9 @@ class PlayerBoxStack(Box): # List to store player buttons self.player_buttons: list[Button] = [] + + # Track signal connections for cleanup + self._signal_connections = [] # Create a "No media playing" placeholder self.no_media_box = self._create_no_media_box() @@ -81,13 +86,15 @@ class PlayerBoxStack(Box): self.mpris_manager = mpris_manager - bulk_connect( + # Track connections for cleanup + connections = bulk_connect( self.mpris_manager, { "player-appeared": self.on_new_player, "player-vanished": self.on_lost_player, }, ) + self._signal_connections.extend(connections) for player in self.mpris_manager.players: # type: ignore logger.info( @@ -95,6 +102,34 @@ class PlayerBoxStack(Box): ) self.on_new_player(self.mpris_manager, player) + def destroy(self): + """Clean up resources when the widget is destroyed.""" + # Disconnect all signal connections + for connection in self._signal_connections: + try: + connection.disconnect() + except Exception as e: + logger.warning(f"Failed to disconnect signal: {e}") + self._signal_connections.clear() + + # Clean up player buttons + for button in self.player_buttons: + try: + button.destroy() + except Exception: + pass + self.player_buttons.clear() + + # Clean up player boxes + for child in self.player_stack.get_children(): + if hasattr(child, 'destroy') and child != self.no_media_box: + try: + child.destroy() + except Exception: + pass + + super().destroy() + def _create_no_media_box(self): """Create a placeholder box for when no media is playing.""" fallback_cover_path = f"{data.HOME_DIR}/.current.wall" @@ -303,8 +338,22 @@ class PlayerBoxStack(Box): logger.info(f"[PLAYER_MANAGER] Player Removed {player_name}") players: List[PlayerBox] = self.player_stack.get_children() + # Find and properly destroy the player box + player_box_to_remove = None + for player_box in players: + if hasattr(player_box, 'player') and player_box.player.player_name == player_name: + player_box_to_remove = player_box + break + + if player_box_to_remove: + try: + player_box_to_remove.destroy() + except Exception as e: + logger.warning(f"Failed to destroy player box: {e}") + # Check if this was the last player - if len(players) == 1 and player_name == players[0].player.player_name: + remaining_players = [p for p in self.player_stack.get_children() if p != player_box_to_remove] + if len(remaining_players) == 0: # Show the no media box instead of hiding self.player_stack.children = [self.no_media_box] self.current_stack_pos = 0 @@ -314,6 +363,7 @@ class PlayerBoxStack(Box): if ( players and self.current_stack_pos < len(players) + and hasattr(players[self.current_stack_pos], 'player') and players[self.current_stack_pos].player.player_name == player_name ): self.current_stack_pos = max(0, self.current_stack_pos - 1) @@ -347,13 +397,15 @@ class PlayerBoxStack(Box): self.player_buttons.append(new_button) # This will automatically destroy our used button - player_box.connect( - "destroy", - lambda *_: [ - new_button.destroy(), # type: ignore - self.player_buttons.pop(self.player_buttons.index(new_button)), - ], - ) + def cleanup_button(*_): + try: + if new_button in self.player_buttons: + self.player_buttons.remove(new_button) + new_button.destroy() + except Exception as e: + logger.warning(f"Failed to cleanup button: {e}") + + player_box.connect("destroy", cleanup_button) def _update_all_player_buttons(self): """Update all player boxes with the current button state""" @@ -405,6 +457,8 @@ class PlayerBox(Box): # Memory management self.temp_artwork_files = [] # Track temp files for cleanup self.current_download_thread = None # Track current download thread + self._download_cancelled = False # Flag to cancel downloads + self._signal_connections = [] # Track signal connections self.album_cover = Box(style_classes="album-image-c") self.album_cover.set_style( @@ -605,28 +659,14 @@ class PlayerBox(Box): # self.stack_buttons_box, ], ) - # self.overlay_box = Overlay( - # child=self.outer_box, - # overlays=[ - # # self.inner_box, - # self.player_info_box, - # self.image_stack, - # Box( - # children=Image(icon_name=self.player.player_name, icon_size=18), - # h_align="end", - # v_align="start", - # style="margin-top: 5px; margin-right: 10px;", - # tooltip_text=self.player.player_name, # type: ignore - # ), - # ], - # ) self.children = [ *self.children, self.box, ] - bulk_connect( + # Track signal connections for cleanup + connections = bulk_connect( self.player, { "exit": self._on_player_exit, @@ -635,6 +675,34 @@ class PlayerBox(Box): "notify::metadata": self._on_metadata, }, ) + self._signal_connections.extend(connections) + + def destroy(self): + """Clean up all resources when the widget is destroyed.""" + # Cancel any ongoing downloads + self._download_cancelled = True + + # Disconnect all signal connections + for connection in self._signal_connections: + try: + connection.disconnect() + except Exception as e: + logger.warning(f"Failed to disconnect signal: {e}") + self._signal_connections.clear() + + # Clean up temp files + self._cleanup_temp_files() + + # Clean up UI components + try: + if hasattr(self, 'ex_mousecapture'): + self.ex_mousecapture.destroy() + if hasattr(self, 'expanded_player'): + self.expanded_player.destroy() + except Exception as e: + logger.warning(f"Failed to cleanup UI components: {e}") + + super().destroy() def __del__(self): """Ensure cleanup happens even if player exits unexpectedly.""" @@ -656,55 +724,6 @@ class PlayerBox(Box): def update_buttons(self, player_buttons, show_buttons): # """Update the stack switcher buttons in this player box""" pass - # logger.info( - # f"[PlayerBox] update_buttons called: show_buttons={ - # show_buttons - # }, num_buttons={len(player_buttons)}" - # ) - # - # # Clear existing buttons - # for child in self.stack_buttons_box.get_children(): - # child.destroy() - # - # if show_buttons and len(player_buttons) > 1: - # logger.info(f"[PlayerBox] Creating {len(player_buttons)} stack buttons") - # # Create simple dot indicators for each player - # for i, button in enumerate(player_buttons): - # # Create a more visible dot button - # dot_button = Button( - # name="player-stack-button", - # style="min-width: 16px; min-height: 16px; border-radius: 8px; margin: 4px; background-color: #666; border: 1px solid #999;", - # ) - # - # # Set active state based on original button - # if button.get_style_context().has_class("active"): - # dot_button.add_style_class("active") - # dot_button.set_style( - # "min-width: 16px; min-height: 16px; border-radius: 8px; margin: 4px; background-color: #fff; border: 1px solid #ccc;" - # ) - # - # # Connect click handler to switch to this player - # def make_click_handler(index): - # return lambda *_: self.player_stack.on_player_clicked_by_index( - # index - # ) - # - # dot_button.connect("clicked", make_click_handler(i)) - # self.stack_buttons_box.children = [ - # *self.stack_buttons_box.children, - # dot_button, - # ] - # logger.info( - # f"[PlayerBox] Added dot button {i}, total children: { - # len(self.stack_buttons_box.get_children()) - # }" - # ) - # - # self.stack_buttons_box.show_all() - # logger.info("[PlayerBox] Stack buttons box shown") - # else: - # self.stack_buttons_box.hide() - # logger.info("[PlayerBox] Stack buttons box hidden") def _on_metadata(self, *_): self._set_image() @@ -784,14 +803,21 @@ class PlayerBox(Box): local_arturl = urllib.parse.unquote(parsed.path) self._update_image(local_arturl) elif parsed.scheme in ("http", "https"): - # Cancel any existing download thread to prevent memory buildup - if self.current_download_thread is not None: - # Note: GLib.Thread doesn't have a direct cancel method, - # but we can track and ignore old downloads + # Cancel any existing download to prevent memory buildup + self._download_cancelled = True + + # Use threading.Thread instead of GLib.Thread for better control + if self.current_download_thread and self.current_download_thread.is_alive(): + # Thread will check _download_cancelled flag and exit early pass - self.current_download_thread = GLib.Thread.new( - "download-artwork", self._download_and_set_artwork, art_url + + self._download_cancelled = False + self.current_download_thread = threading.Thread( + target=self._download_and_set_artwork, + args=(art_url,), + daemon=True # Dies with main thread ) + self.current_download_thread.start() else: self._update_image(art_url) @@ -804,6 +830,10 @@ class PlayerBox(Box): temp_file_path = None try: + # Check if download was cancelled + if self._download_cancelled: + return + # Clean up old temp files first (keep only last 2 to avoid excessive cleanup) if len(self.temp_artwork_files) > 2: old_files = self.temp_artwork_files[:-2] @@ -815,13 +845,23 @@ class PlayerBox(Box): pass self.temp_artwork_files = self.temp_artwork_files[-2:] + # Check again if cancelled + if self._download_cancelled: + return + # Download artwork parsed = urllib.parse.urlparse(arturl) suffix = os.path.splitext(parsed.path)[1] or ".png" with urllib.request.urlopen(arturl, timeout=10) as response: # Add timeout + if self._download_cancelled: + return data = response.read() + # Check one more time if cancelled + if self._download_cancelled: + return + # Create temp file in cache directory instead of system temp os.makedirs(CACHE_DIR, exist_ok=True) with tempfile.NamedTemporaryFile( @@ -832,20 +872,23 @@ class PlayerBox(Box): local_arturl = temp_file_path # Track temp file for cleanup - if temp_file_path: + if temp_file_path and not self._download_cancelled: self.temp_artwork_files.append(temp_file_path) except Exception as e: - logger.warning(f"Failed to download artwork from {arturl}: {e}") + if not self._download_cancelled: + logger.warning(f"Failed to download artwork from {arturl}: {e}") # Clean up failed temp file if temp_file_path and os.path.exists(temp_file_path): try: os.unlink(temp_file_path) except Exception: pass + return - GLib.idle_add(self._update_image, local_arturl) - return None + # Only update UI if not cancelled + if not self._download_cancelled: + GLib.idle_add(self._update_image, local_arturl) def close_bluetooth(self, *args): """Close Bluetooth control center"""