Files
LEDMatrix/test/test_plugin_loader_reload_isolation.py
T
ChuckandClaude Opus 5.5 d18e4d3c9d fix(plugins): sub-package reload, symlinked dev plugins, BaseException in update(), config callbacks outside the lock (#741)
* fix(plugins): drop a plugin's package modules when it unloads

A plugin that keeps helpers in a package (providers/feed.py, imported as
`from providers.feed import ...`) leaves dotted entries in sys.modules.
PluginLoader only tracked bare names: `providers` was namespaced and
dropped on unload, `providers.feed` stayed. A reload after a store update
imported a fresh `providers`, then got the old `feed` back from the module
cache, so the new manager.py ran against the old helpers until the display
restarted. A load that failed part-way left them behind the same way.
Elections (providers/), flights (enrichment/) and olympics (data/,
renderers/) ship packages.

The loader now records the dotted modules whose file (or, for a namespace
package, every __path__ entry) lies inside the plugin directory. They keep
their names while the plugin runs, as before, and unregister_plugin_modules()
drops them, only while sys.modules still holds that plugin's module. The
failed-load cleanup in load_module() drops them too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(plugins): remove a symlinked dev plugin as a link

PluginStoreManager._safe_remove_directory, behind uninstall and behind
discarding the set-aside copy after an install or update, handed a
symlinked dev plugin (scripts/dev/dev_plugin_setup.sh) to shutil.rmtree,
which refuses a symlink. The chmod fallback then walked through the link
and set every directory and file in the linked checkout to 0700, and the
sudo stage refused the resolved path as outside the plugins directory. The
removal failed, the link stayed, and the developer's checkout lost its
group/other permissions. A dangling link read as already removed, because
exists() follows it, and was left behind.

A symlink is now unlinked before any other stage runs, and before the
exists() check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(plugins): load a dev plugin linked in under a different name

contained_plugin_dir(), the containment check before a plugin's
dependencies are installed, resolved the plugin directory and looked for
the resolved folder's name among the plugins directory's entries. A dev
plugin symlinked in under its id by a name its checkout does not share --
`dev_plugin_setup.sh link-github foo <url>` clones ledmatrix-foo, the
repository naming convention, and links it as plugins/foo -- has no such
entry, so install_dependencies() returned False and the load failed with
"Dependency installation failed", even with no requirements.txt.

When the path sits directly in the plugins directory, the entry it names
(the link) is looked up first; anything else is resolved and matched by
name as before. The answer is still always rebuilt from a name os.scandir()
returned for the plugins directory, so a path outside it is still refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(plugins): release a plugin whose update() raises a BaseException

On the async update worker, the wrapped update() finished its bookkeeping
(_finish: release the plugin lock, drop the pending slot, state back to
ENABLED) only for an Exception. asyncio.CancelledError and SystemExit
derive from BaseException, so one raised from update() skipped _finish:
the plugin kept its lock and stayed RUNNING for the life of the process,
never rescheduled, with every display() skipped as busy. PluginExecutor
caught only Exception as well, so its thread died with the call never
marked complete and an immediate failure was logged and recorded as a
timeout.

_target_update now runs _finish for any BaseException and re-raises it,
and the executor's thread stores it like any other exception, so it is
reported as the operation's failure (PluginError) on both the async and
the synchronous path. _finish and _record_update_failure take a
BaseException.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(config): notify config subscribers outside the service lock

ConfigService._load_config ran every subscriber while holding _lock. The
display's per-plugin subscriber calls PluginManager.apply_config_change,
which waits up to PLUGIN_LOCK_TIMEOUT (5 s) for a plugin busy in update().
A save that enables or disables a plugin also flags a reconcile, which the
render thread runs: its get_config(), and the unsubscribe() of a plugin it
disables, both take _lock, so the panel froze behind every slow callback,
up to 5 s per busy plugin.

The config is now swapped under _lock and the subscribers are called after
it is released, from a copy of the subscriber lists. A separate
_notify_lock is held across a whole reload (read, swap, notify), so one
reload's notifications still finish before the next one's start. Each
callback is checked against the live lists just before it runs, and
unsubscribe() waits only for a call of that same callback already in
progress (unless it is that callback's own thread), so a callback it
removed is not running and will not run once it returns, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 22:18:42 -04:00

141 lines
5.4 KiB
Python

"""A reloaded plugin imports its own bare-name modules, not another plugin's.
Scoreboard plugins each ship a ``sports.py`` and import it by bare name. Each
plugin's directory goes on sys.path when it loads, and a bare import resolves
to the first directory that has the file. The loader used to add a directory
only if it was missing, so after alpha, then beta, loaded, re-enabling alpha
from the web UI left beta's directory in front: alpha's ``from sports import
...`` got beta's copy. On a Pi, re-enabling UFC with hockey running failed
with "cannot import name '_status_is_final' from 'sports'".
"""
import sys
import pytest
from src.plugin_system.plugin_loader import PluginLoader
def _write_plugin(root, name):
d = root / name
d.mkdir(parents=True)
(d / "sports.py").write_text(f"WHO = {name!r}\n", encoding="utf-8")
(d / "manager.py").write_text("from sports import WHO\n", encoding="utf-8")
return d
@pytest.fixture
def plugins(tmp_path):
before_path = list(sys.path)
before_modules = set(sys.modules)
dirs = {name: _write_plugin(tmp_path, name) for name in ("alpha", "beta")}
yield dirs
sys.path[:] = before_path
for key in set(sys.modules) - before_modules:
sys.modules.pop(key, None)
def _unload(loader, plugin_id):
# What PluginManager.unload_plugin does to the module entries.
sys.modules.pop(f"plugin_{plugin_id}", None)
loader.unregister_plugin_modules(plugin_id)
def test_each_plugin_gets_its_own_bare_module(plugins):
loader = PluginLoader()
assert loader.load_module("alpha", plugins["alpha"], "manager.py").WHO == "alpha"
assert loader.load_module("beta", plugins["beta"], "manager.py").WHO == "beta"
def test_a_reloaded_plugin_still_gets_its_own_bare_module(plugins):
loader = PluginLoader()
loader.load_module("alpha", plugins["alpha"], "manager.py")
loader.load_module("beta", plugins["beta"], "manager.py")
_unload(loader, "alpha")
reloaded = loader.load_module("alpha", plugins["alpha"], "manager.py")
assert reloaded.WHO == "alpha"
assert sys.path.index(str(plugins["alpha"])) < sys.path.index(str(plugins["beta"]))
assert sys.path.count(str(plugins["alpha"])) == 1
# -- sub-packages ------------------------------------------------------------
#
# A plugin that keeps helpers in a package (``providers/feed.py``, imported as
# ``from providers.feed import ...``) leaves dotted entries in sys.modules.
# Only the bare ``providers`` used to be tracked, so ``providers.feed`` outlived
# the plugin: a reload after a store update re-ran the new manager.py against
# the old feed.py, until the display restarted. Elections (providers/),
# flights (enrichment/) and olympics (data/, renderers/) ship packages.
@pytest.fixture
def package_plugin(tmp_path):
before_path = list(sys.path)
before_modules = set(sys.modules)
plugin_dir = tmp_path / "pkgdemo"
(plugin_dir / "providers").mkdir(parents=True)
(plugin_dir / "providers" / "__init__.py").write_text("", encoding="utf-8")
(plugin_dir / "providers" / "feed.py").write_text("VERSION = 'v1'\n", encoding="utf-8")
(plugin_dir / "manager.py").write_text(
"from providers.feed import VERSION\n", encoding="utf-8")
yield plugin_dir
sys.path[:] = before_path
for key in set(sys.modules) - before_modules:
sys.modules.pop(key, None)
def test_a_reloaded_plugin_runs_its_updated_subpackage_module(package_plugin):
loader = PluginLoader()
assert loader.load_module("pkgdemo", package_plugin, "manager.py").VERSION == "v1"
_unload(loader, "pkgdemo")
# The store update: a different size, so no cached bytecode can match.
(package_plugin / "providers" / "feed.py").write_text(
"VERSION = 'v2 from the update'\n", encoding="utf-8")
reloaded = loader.load_module("pkgdemo", package_plugin, "manager.py")
assert reloaded.VERSION == "v2 from the update"
def test_unload_drops_the_plugins_subpackage_modules(package_plugin):
loader = PluginLoader()
loader.load_module("pkgdemo", package_plugin, "manager.py")
# Still importable while the plugin runs, as before.
assert "providers.feed" in sys.modules
_unload(loader, "pkgdemo")
assert not [k for k in sys.modules if k.startswith("providers")]
def test_a_failed_load_leaves_no_subpackage_module_behind(package_plugin):
(package_plugin / "manager.py").write_text(
"from providers.feed import VERSION\nraise RuntimeError('broken')\n",
encoding="utf-8")
loader = PluginLoader()
with pytest.raises(RuntimeError):
loader.load_module("pkgdemo", package_plugin, "manager.py")
assert not [k for k in sys.modules if k.startswith("providers")]
def test_unload_leaves_packages_from_outside_the_plugin_alone(package_plugin, tmp_path):
# A library the plugin imports is not the plugin's to drop.
lib_root = tmp_path / "site"
(lib_root / "extlib").mkdir(parents=True)
(lib_root / "extlib" / "__init__.py").write_text("", encoding="utf-8")
(lib_root / "extlib" / "sub.py").write_text("X = 1\n", encoding="utf-8")
sys.path.append(str(lib_root))
(package_plugin / "manager.py").write_text(
"import extlib.sub\nfrom providers.feed import VERSION\n", encoding="utf-8")
loader = PluginLoader()
loader.load_module("pkgdemo", package_plugin, "manager.py")
_unload(loader, "pkgdemo")
assert "extlib.sub" in sys.modules
assert "extlib" in sys.modules