diff --git a/plugins/soccer-scoreboard/manager.py b/plugins/soccer-scoreboard/manager.py index 0062e9ac..0ba825cd 100644 --- a/plugins/soccer-scoreboard/manager.py +++ b/plugins/soccer-scoreboard/manager.py @@ -823,7 +823,16 @@ def _initialize_league_registry(self) -> None: This design allows the display logic to iterate through leagues in priority order without hardcoding league names throughout the codebase. + + Built into a local and swapped in with a single assignment. The display + thread iterates ``self._league_registry`` (via + ``_get_enabled_leagues_for_mode``) while ``on_config_change`` rebuilds it + on the ConfigService-Watcher thread; mutating the live dict in place let + a frame observe it empty or half-populated, or raise "dictionary changed + size during iteration". Rebinding the attribute is atomic, so a reader + gets either the whole old registry or the whole new one. """ + registry: Dict[str, Dict[str, Any]] = {} # Add predefined leagues to registry for league_key in PREDEFINED_LEAGUE_KEYS: attr_tuple = PREDEFINED_LEAGUE_ATTR_MAP.get(league_key) @@ -831,7 +840,7 @@ def _initialize_league_registry(self) -> None: continue live_attr, recent_attr, upcoming_attr = attr_tuple - self._league_registry[league_key] = { + registry[league_key] = { 'enabled': self.league_enabled.get(league_key, False), 'priority': PREDEFINED_LEAGUE_PRIORITIES.get(league_key, 99), 'live_priority': self.league_live_priority.get(league_key, False), @@ -857,7 +866,7 @@ def _initialize_league_registry(self) -> None: recent_attr = f'custom_{safe_key}_recent' upcoming_attr = f'custom_{safe_key}_upcoming' - self._league_registry[league_code] = { + registry[league_code] = { 'enabled': self.league_enabled.get(league_code, False), 'priority': custom_priorities.get(league_code, 50), 'live_priority': self.league_live_priority.get(league_code, False), @@ -869,11 +878,14 @@ def _initialize_league_registry(self) -> None: } } + # Publish the finished registry in one atomic rebind (see docstring). + self._league_registry = registry + # Log registry state for debugging - enabled_leagues = [lid for lid, data in self._league_registry.items() if data['enabled']] - custom_count = len([lid for lid, data in self._league_registry.items() if data.get('is_custom', False)]) + enabled_leagues = [lid for lid, data in registry.items() if data['enabled']] + custom_count = len([lid for lid, data in registry.items() if data.get('is_custom', False)]) self.logger.info( - f"League registry initialized: {len(self._league_registry)} league(s) registered " + f"League registry initialized: {len(registry)} league(s) registered " f"({custom_count} custom), {len(enabled_leagues)} enabled: " f"{[LEAGUE_NAMES.get(lid, lid) for lid in enabled_leagues]}" ) @@ -898,8 +910,17 @@ def _get_enabled_leagues_for_mode(self, mode_type: str) -> List[str]: """ enabled_leagues = [] + # One snapshot for the whole selection. _initialize_league_registry + # publishes a replacement registry by rebinding the attribute, so + # re-reading self._league_registry further down (the sort key, the debug + # line) could bind a NEWER registry than the one just iterated -- and a + # custom league collected from the old one may be absent from it, which + # is a KeyError in the sort. Reading once makes the whole selection + # consistent with a single registry. + registry = self._league_registry + # Iterate through all registered leagues - for league_id, league_data in self._league_registry.items(): + for league_id, league_data in registry.items(): # Check if league is enabled if not league_data.get('enabled', False): continue @@ -924,11 +945,11 @@ def _get_enabled_leagues_for_mode(self, mode_type: str) -> List[str]: enabled_leagues.append(league_id) # Sort by priority (lower number = higher priority) - enabled_leagues.sort(key=lambda lid: self._league_registry[lid].get('priority', 999)) + enabled_leagues.sort(key=lambda lid: registry[lid].get('priority', 999)) self.logger.debug( f"Enabled leagues for {mode_type} mode: {enabled_leagues} " - f"(priorities: {[self._league_registry[lid].get('priority') for lid in enabled_leagues]})" + f"(priorities: {[registry[lid].get('priority') for lid in enabled_leagues]})" ) return enabled_leagues @@ -990,13 +1011,17 @@ def _get_league_manager_for_mode(self, league_id: str, mode_type: str): The manager is retrieved from the league registry, which is populated during initialization. If the league or mode doesn't exist, returns None. """ + # One snapshot: the membership test and the lookup must see the same + # registry, or a rebind between them turns the check into a KeyError. + registry = self._league_registry + # Check if league exists in registry - if league_id not in self._league_registry: + if league_id not in registry: self.logger.warning(f"League {league_id} not found in registry") return None # Get managers dict for this league - managers = self._league_registry[league_id].get('managers', {}) + managers = registry[league_id].get('managers', {}) # Get the manager for this mode type manager = managers.get(mode_type) @@ -1343,8 +1368,10 @@ def on_config_change(self, new_config: Dict[str, Any]) -> None: thread.join(timeout=10.0) self._active_update_threads.clear() - # Clear stale runtime caches before rebuilding - self._league_registry.clear() + # Clear stale runtime caches before rebuilding. _league_registry is + # deliberately NOT cleared here: _initialize_league_registry() below + # rebuilds it into a local and swaps it in atomically, so the display + # thread never sees it empty. self._scroll_prepared.clear() self._scroll_active.clear() # Re-verify team codes against ESPN, so a corrected code is diff --git a/plugins/soccer-scoreboard/manifest.json b/plugins/soccer-scoreboard/manifest.json index 0af9ad62..fb0029c4 100644 --- a/plugins/soccer-scoreboard/manifest.json +++ b/plugins/soccer-scoreboard/manifest.json @@ -1,7 +1,7 @@ { "id": "soccer-scoreboard", "name": "Soccer Scoreboard", - "version": "2.12.6", + "version": "2.12.7", "author": "ChuckBuilds", "description": "Live, recent, and upcoming soccer games across multiple leagues including Premier League, La Liga, Bundesliga, Serie A, Ligue 1, MLS, Liga Portugal, Champions League, Europa League, and FIFA World Cup", "category": "sports", @@ -26,6 +26,12 @@ "soccer_upcoming" ], "versions": [ + { + "version": "2.12.7", + "released": "2026-08-22", + "ledmatrix_min_version": "2.0.0", + "notes": "Rebuild the league registry into a local and publish it with one atomic rebind instead of clearing the live dict and refilling it key by key. on_config_change runs on the core's config-watcher thread while the display path iterates the same registry, so saving config during a soccer frame could render a screen with no leagues, or raise \"dictionary changed size during iteration\" inside display(). Readers that consult the registry more than once (the league selection's iterate-then-sort, and the manager lookup's membership-test-then-index) now take a single snapshot, so a replacement landing mid-selection cannot turn a league collected from the old registry into a KeyError against the new one." + }, { "version": "2.12.2", "released": "2026-08-19", diff --git a/plugins/soccer-scoreboard/test_league_registry_atomic_swap.py b/plugins/soccer-scoreboard/test_league_registry_atomic_swap.py new file mode 100644 index 00000000..5e15c3ed --- /dev/null +++ b/plugins/soccer-scoreboard/test_league_registry_atomic_swap.py @@ -0,0 +1,157 @@ +#!/usr/bin/env python3 +""" +Regression test: rebuilding the league registry must not be visible mid-flight. + +``on_config_change`` runs on the core's ``ConfigService-Watcher`` thread, not +the render thread. It rebuilds the league registry, while the display path +iterates that same dict: + + _display_scroll_mode -> _get_enabled_leagues_for_mode + -> for league_id, league_data in self._league_registry.items() + +The rebuild used to clear the live dict and repopulate it key by key, so saving +config while a soccer frame was rendering could show a frame with no leagues, +or raise "dictionary changed size during iteration" inside display(). + +``_initialize_league_registry`` now builds into a local and publishes it with a +single attribute rebind, which is atomic. A reader sees either the whole old +registry or the whole new one. + +Run: /bin/python plugins/soccer-scoreboard/test_league_registry_atomic_swap.py +""" + +import sys +import types +from pathlib import Path + +plugin_dir = Path(__file__).parent +sys.path.insert(0, str(plugin_dir)) + + +def _stub_core_src(): + def mod(name, **attrs): + m = types.ModuleType(name) + for k, v in attrs.items(): + setattr(m, k, v) + sys.modules.setdefault(name, m) + return m + + mod("src") + mod("src.common") + mod("src.plugin_system") + mod("src.logo_downloader", LogoDownloader=object, download_missing_logo=lambda *a, **k: None) + mod("src.common.scroll_helper", ScrollHelper=object) + mod("src.plugin_system.base_plugin", BasePlugin=None, VegasDisplayMode=object) + mod("src.background_data_service", get_background_service=lambda *a, **k: None) + + +_stub_core_src() + +import logging # noqa: E402 +import inspect # noqa: E402 + +from manager import SoccerScoreboardPlugin # noqa: E402 + +results = [] + + +def check(case, passed): + results.append((case, passed)) + print(f" [{'pass' if passed else 'FAIL'}] {case}") + + +def make_plugin(): + p = SoccerScoreboardPlugin.__new__(SoccerScoreboardPlugin) + p.logger = logging.getLogger("test-soccer-registry-swap") + p.config = {"enabled": True, "custom_leagues": []} + p.display_manager = None + p.cache_manager = None + p.plugin_manager = None + p.league_enabled = {} + p.league_live_priority = {} + p._league_registry = {} + p.custom_league_map = {} + return p + + +plugin = make_plugin() +plugin._initialize_league_registry() +first = plugin._league_registry +check("registry is populated", len(first) > 0) + +# The old dict must survive the rebuild intact: a display thread that grabbed a +# reference before the swap keeps iterating a complete registry. +snapshot = plugin._league_registry +before_len = len(snapshot) +plugin._initialize_league_registry() + +check("rebuild rebinds rather than mutating in place", + plugin._league_registry is not snapshot) +check("the previously-published dict is left intact", + len(snapshot) == before_len) +check("new registry is complete", len(plugin._league_registry) == before_len) + +# Guard the mechanism itself, so a later refactor can't quietly reintroduce +# in-place mutation of the published dict. +src = inspect.getsource(SoccerScoreboardPlugin._initialize_league_registry) +check("no in-place writes to the published registry", + "self._league_registry[" not in src) +check("publishes with a single rebind", + src.count("self._league_registry = registry") == 1) + +occ_src = inspect.getsource(SoccerScoreboardPlugin.on_config_change) +check("on_config_change no longer clears the live registry", + "self._league_registry.clear()" not in occ_src) + +# --- a reader must survive a replacement landing mid-selection ------------ +# _get_enabled_leagues_for_mode iterates the registry, then sorts and logs +# using it again. If those later reads re-fetched self._league_registry, a +# config save that drops a custom league between the iteration and the sort +# would raise KeyError on the render thread. +swapped = make_plugin() +swapped._league_registry = { + "eng.1": {"enabled": True, "priority": 1, "is_custom": False, "managers": {}}, + "cus.1": {"enabled": True, "priority": 2, "is_custom": True, "managers": {}}, +} +swapped._get_league_config = lambda lid, data: {} + +original_items = swapped._league_registry.items + + +class _SwapOnIterate(dict): + """Rebinds the plugin's registry the moment the selection iterates it, + standing in for on_config_change landing on the watcher thread.""" + + def items(self): + # The replacement no longer has the custom league. + swapped._league_registry = { + "eng.1": {"enabled": True, "priority": 1, "is_custom": False, "managers": {}}, + } + return original_items() + + +swapped._league_registry = _SwapOnIterate(swapped._league_registry) +try: + selected = swapped._get_enabled_leagues_for_mode("live") + raised = None +except KeyError as exc: + selected, raised = None, exc + +check("a registry replacement mid-selection does not raise KeyError", raised is None) +check("the selection reflects the registry it started from", + selected is not None and "cus.1" in selected) + +# The membership test and lookup in _get_league_manager_for_mode must also +# agree with each other. +mgr_plugin = make_plugin() +mgr_plugin._league_registry = {"eng.1": {"enabled": True, "managers": {"live": "M"}}} +check("manager lookup returns from the same snapshot it tested", + mgr_plugin._get_league_manager_for_mode("eng.1", "live") == "M") + +print() +failed = [case for case, passed in results if not passed] +print(f"{len(results) - len(failed)}/{len(results)} passed") +if failed: + for case in failed: + print(f" FAILED: {case}") + sys.exit(1)