mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
fix(plugins): one hung plugin no longer stops every plugin from updating (#677)
The update worker no longer blocks forever on a plugin whose display() never returns. It waits at most PLUGIN_LOCK_TIMEOUT (5s) for a plugin's lock, then skips that plugin's update (a report-only "busy skip" in health) and keeps updating every other plugin. display() frames are timed (slow calls logged and counted; calls past the executor timeout recorded as hangs), a hung update() is recorded, and on_config_change() now runs under the plugin lock or is deferred to the worker. The plugin-facing API is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1029,17 +1029,28 @@ class DisplayController:
|
||||
accepts_display_mode: Whether display() takes ``display_mode``.
|
||||
force_clear: Passed through to display().
|
||||
|
||||
Each call is timed (two monotonic reads) and handed to
|
||||
PluginManager.note_display_duration, which logs and records slow
|
||||
calls and counts one that ran past the executor's timeout as a hang.
|
||||
|
||||
Returns:
|
||||
display()'s result, or True when the frame was skipped because
|
||||
the plugin's update() holds its lock (the panel keeps the last
|
||||
frame; that is not a failure).
|
||||
"""
|
||||
with self._display_lock_or_skip(getattr(plugin, 'plugin_id', None)) as can_display:
|
||||
plugin_id = getattr(plugin, 'plugin_id', None)
|
||||
with self._display_lock_or_skip(plugin_id) as can_display:
|
||||
if not can_display:
|
||||
return True
|
||||
if accepts_display_mode:
|
||||
return plugin.display(display_mode=mode, force_clear=force_clear)
|
||||
return plugin.display(force_clear=force_clear)
|
||||
started = time.monotonic()
|
||||
try:
|
||||
if accepts_display_mode:
|
||||
return plugin.display(display_mode=mode, force_clear=force_clear)
|
||||
return plugin.display(force_clear=force_clear)
|
||||
finally:
|
||||
note = getattr(self.plugin_manager, 'note_display_duration', None)
|
||||
if note is not None and plugin_id:
|
||||
note(plugin_id, time.monotonic() - started)
|
||||
|
||||
def _health_tracker(self):
|
||||
"""The plugin circuit breaker, or None when it is not enabled."""
|
||||
@@ -2498,6 +2509,7 @@ class DisplayController:
|
||||
pm = self.plugin_manager
|
||||
display_lock = pm.get_plugin_lock(plugin_id) if pm else None
|
||||
can_display = display_lock is None or display_lock.acquire(blocking=False)
|
||||
display_hung = False
|
||||
|
||||
if display_lock is None:
|
||||
# Only when plugin loading failed part-way.
|
||||
@@ -2517,7 +2529,7 @@ class DisplayController:
|
||||
# thread actually finishes it, rather than
|
||||
# here when this dispatch merely returns.
|
||||
release_guard = threading.Lock()
|
||||
released = {'done': False}
|
||||
released = {'done': False, 'started': False}
|
||||
|
||||
def _release_display_lock():
|
||||
with release_guard:
|
||||
@@ -2528,6 +2540,7 @@ class DisplayController:
|
||||
|
||||
if _accepts_display_mode:
|
||||
def _display_target(display_mode=None, force_clear=False):
|
||||
released['started'] = True
|
||||
try:
|
||||
return manager_to_display.display(
|
||||
display_mode=display_mode, force_clear=force_clear)
|
||||
@@ -2535,11 +2548,13 @@ class DisplayController:
|
||||
_release_display_lock()
|
||||
else:
|
||||
def _display_target(force_clear=False):
|
||||
released['started'] = True
|
||||
try:
|
||||
return manager_to_display.display(force_clear=force_clear)
|
||||
finally:
|
||||
_release_display_lock()
|
||||
|
||||
dispatch_start = time.monotonic()
|
||||
try:
|
||||
result = pm.plugin_executor.execute_display(
|
||||
types.SimpleNamespace(display=_display_target),
|
||||
@@ -2562,6 +2577,18 @@ class DisplayController:
|
||||
_release_display_lock()
|
||||
raise
|
||||
|
||||
dispatch_seconds = time.monotonic() - dispatch_start
|
||||
if released['started'] and not released['done']:
|
||||
# The executor gave up waiting and display()
|
||||
# is still running on its thread, holding
|
||||
# the lock. A hang, not a success: recorded
|
||||
# so repeats open the circuit breaker, and
|
||||
# the update worker's bounded wait skips it.
|
||||
display_hung = True
|
||||
pm.record_display_hang(plugin_id, dispatch_seconds)
|
||||
else:
|
||||
pm.note_display_duration(plugin_id, dispatch_seconds)
|
||||
|
||||
logger.debug(f"display() returned: {result} (type: {type(result)})")
|
||||
if isinstance(result, bool):
|
||||
display_result = result
|
||||
@@ -2575,7 +2602,7 @@ class DisplayController:
|
||||
# be lost when display() finally does run.
|
||||
if can_display:
|
||||
health_tracker = self._health_tracker()
|
||||
if health_tracker is not None:
|
||||
if health_tracker is not None and not display_hung:
|
||||
health_tracker.record_success(plugin_id)
|
||||
self.force_change = False
|
||||
except Exception as exc: # pylint: disable=broad-except
|
||||
@@ -3274,8 +3301,18 @@ class DisplayController:
|
||||
# says disabled, and on_config_change would switch
|
||||
# the instance off mid-session.
|
||||
new_config = {**new_config, 'enabled': True}
|
||||
_plugin.on_config_change(new_config)
|
||||
logger.debug("Plugin %s notified of config change", _pid)
|
||||
# Runs on ConfigService's watcher thread. Under the
|
||||
# plugin's lock, so it cannot interleave with update()
|
||||
# on the worker or display() on the render thread; a
|
||||
# lock held past the bound defers it to the worker.
|
||||
apply = getattr(self.plugin_manager, 'apply_config_change', None)
|
||||
if callable(apply):
|
||||
applied = apply(_pid, new_config, plugin_instance=_plugin)
|
||||
else:
|
||||
_plugin.on_config_change(new_config)
|
||||
applied = True
|
||||
logger.debug("Plugin %s notified of config change%s", _pid,
|
||||
"" if applied else " (deferred: plugin busy)")
|
||||
except Exception as e:
|
||||
logger.error("Error in plugin %s config change handler: %s", _pid, e, exc_info=True)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user