mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 08:48:05 +00:00
fix(core): harden text-measurement caches; surface snapshot failures (#400)
* fix(core): harden text-measurement caches; surface snapshot failures Deep-dive findings, all three latent on every 24/7 install: - font_manager.metrics_cache and display_manager._text_width_cache were unbounded dicts keyed by (text, id(font)). Two problems: keys embed the measured TEXT, so ever-changing strings (a clock, a live score, a ticker) grow them without limit; and id()-keying without holding a reference means a garbage-collected font's id can be recycled by a DIFFERENT font, silently returning wrong widths/metrics (classic plugins create fonts per render, so this is reachable). Both caches are now LRU-bounded (1024) and pin the font in the entry so its id stays valid. metrics_cache also keyed on the text itself instead of hash(text), removing a collision path. - _write_snapshot_if_due logged failures at DEBUG — invisible at the default level. The snapshot's mtime is the web UI's display mirror AND its hardware-liveness proxy, so a quiet failure freezes the mirror and makes health checks lie (seen in the field: a stale root-owned /tmp file froze it for a day). Failures now WARN, rate- limited to once per 5 minutes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * test: sync mock cache manager signature with CacheManager.get test_circuit_breaker has been failing on main: plugin_health passes memory_ttl= to cache_manager.get(), and the conftest mock's signature was never updated — the same component/double drift class as the monitored_update bug (#392). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
Chuck
parent
6edd80d9f3
commit
9e3b5f366e
+35
-8
@@ -33,7 +33,8 @@ else:
|
||||
from contextlib import contextmanager
|
||||
from PIL import Image, ImageDraw, ImageFont
|
||||
import time
|
||||
from typing import Dict, Any, List, Optional
|
||||
from collections import OrderedDict
|
||||
from typing import Dict, Any, List, Optional, Tuple
|
||||
import logging
|
||||
import math
|
||||
import freetype
|
||||
@@ -180,14 +181,25 @@ class DisplayManager:
|
||||
# the logical image is blitted to the matrix unchanged.
|
||||
self._double_sided = None # dict {copies, axis, logical_width, logical_height} or None
|
||||
self._physical_image = None # full-chain buffer reused each frame when tiling
|
||||
# Text-width measurement cache: (text, id(font)) -> pixel_width
|
||||
# Text-width measurement cache: (text, id(font)) -> (width, font_ref)
|
||||
# Avoids re-measuring the same string+font on every display() call.
|
||||
# LRU-bounded: keys embed the TEXT, so changing strings (a clock, a
|
||||
# live score) would otherwise grow it forever on a 24/7 service.
|
||||
# Entries hold a strong reference to the font so its id() can't be
|
||||
# recycled by a different font object — an id-keyed cache without
|
||||
# the reference can return the WRONG width after garbage collection.
|
||||
# Cleared on _load_fonts() so stale entries don't survive a font reload.
|
||||
self._text_width_cache: Dict[tuple, int] = {}
|
||||
self._text_width_cache: "OrderedDict[tuple, Tuple[int, Any]]" = OrderedDict()
|
||||
self._TEXT_WIDTH_CACHE_MAX = 1024
|
||||
# Snapshot settings for web preview integration (service writes, web reads)
|
||||
self._snapshot_path = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path intentional; web UI reads same path
|
||||
self._snapshot_min_interval_sec = 0.2 # max ~5 fps
|
||||
self._last_snapshot_ts = 0.0
|
||||
# Snapshot failures are logged as warnings, rate-limited so a
|
||||
# persistent failure (e.g. an unwritable file) can't spam the log —
|
||||
# but is never silent: the snapshot's mtime doubles as the web UI's
|
||||
# hardware-liveness signal, so a quiet failure makes health checks lie.
|
||||
self._snapshot_fail_log_ts = 0.0
|
||||
|
||||
# Scrolling state tracking for graceful updates
|
||||
self._scrolling_state = {
|
||||
@@ -699,12 +711,15 @@ class DisplayManager:
|
||||
|
||||
Results are cached by (text, font identity) so plugins that measure
|
||||
the same string every frame (e.g. to centre a score) pay only one
|
||||
measurement per unique (text, font) pair.
|
||||
measurement per unique (text, font) pair. The entry keeps the font
|
||||
alive so its id() can't be recycled, and the cache is LRU-bounded so
|
||||
ever-changing text (clocks, tickers) can't grow it without limit.
|
||||
"""
|
||||
cache_key = (text, id(font))
|
||||
cached = self._text_width_cache.get(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
self._text_width_cache.move_to_end(cache_key)
|
||||
return cached[0]
|
||||
|
||||
try:
|
||||
if isinstance(font, freetype.Face):
|
||||
@@ -719,7 +734,9 @@ class DisplayManager:
|
||||
logger.error("Error getting text width: %s", e)
|
||||
return 0
|
||||
|
||||
self._text_width_cache[cache_key] = width
|
||||
self._text_width_cache[cache_key] = (width, font)
|
||||
while len(self._text_width_cache) > self._TEXT_WIDTH_CACHE_MAX:
|
||||
self._text_width_cache.popitem(last=False)
|
||||
return width
|
||||
|
||||
def get_font_height(self, font):
|
||||
@@ -1164,5 +1181,15 @@ class DisplayManager:
|
||||
pass
|
||||
self._last_snapshot_ts = now
|
||||
except Exception as e:
|
||||
# Snapshot failures should never break display; log at debug to avoid noise
|
||||
logger.debug(f"Snapshot write skipped: {e}")
|
||||
# Snapshot failures must never break display — but they must not
|
||||
# be silent either: the snapshot's mtime is the web UI's display
|
||||
# mirror AND its hardware-liveness proxy, so a quietly failing
|
||||
# write freezes the mirror and makes health checks lie (seen in
|
||||
# the field: a stale root-owned /tmp file froze it for a day).
|
||||
# Warn at most once per 5 minutes to avoid log spam.
|
||||
if (now - self._snapshot_fail_log_ts) > 300:
|
||||
self._snapshot_fail_log_ts = now
|
||||
logger.warning("Snapshot write failing (web preview/health "
|
||||
"mirror is stale): %s", e)
|
||||
else:
|
||||
logger.debug(f"Snapshot write skipped: {e}")
|
||||
Reference in New Issue
Block a user