Merge pull request #29 from mutating/develop

0.0.23

Author: pomponchik

docs/plans/5.md

@@ -0,0 +1,98 @@
+# План по Issue #28: не выполнять bodies под `Slot.lock`
+
+## Summary
+
+- Источник: [GitHub Issue #28](https://github.com/mutating/pristan/issues/28).
+- Перед имплементацией сохранить этот план в [docs/plans/5.md](/Users/pomponchik/Desktop/Projects/symplug/docs/plans/5.md).
+- По TDD: сначала добавить/изменить тесты, запустить целевой pytest и убедиться, что проверки нового поведения падают на текущей реализации, затем менять код.
+- Проблема подтверждена в текущей реализации: [Slot.__call__](/Users/pomponchik/Desktop/Projects/symplug/pristan/components/slot.py:109) вызывает `self.backed_caller(...)` внутри `with self.lock`.
+- Исправление: держать `Slot.lock` только на `_load_entrypoints()` и создании snapshot-backed caller, а plugin/default body выполнять после release lock.
+- Entry point loading deadlocks не трогать: `_load_entrypoints()` остается под `Slot.lock`.
+
+## Public APIs / Interfaces / Types
+
+- Публичные сигнатуры API, типы возврата, тексты исключений и контракт lazy loading не менять.
+- `self.backed_caller` не удалять: он остается частью текущей внутренней модели и используется `__bool__`.
+- Новое поведение: один вызов слота видит один стабильный список плагинов; плагин, зарегистрированный во время dispatch, виден только со следующего вызова.
+
+## Изменения реализации
+
+- В `Slot.__call__` заменить dispatch через live `self.backed_caller` внутри lock на локальный snapshot-backed `CallerWithPlugins`:
+  ```python
+  with self.lock:
+      self._load_entrypoints()
+      backed_caller = CallerWithPlugins(self.caller, list(self.plugins.plugins))
+
+  return backed_caller(*args, **kwargs)
+  ```
+- Локальный `backed_caller` создавать внутри `with self.lock`, чтобы snapshot списка плагинов фиксировался под блокировкой.
+- Вызов `backed_caller(...)` выполнять после release lock, чтобы plugin/default body не исполнялись в registry critical section.
+- Snapshot должен быть новым списком ссылок на текущие `Plugin`-объекты из `self.plugins.plugins`; сами `Plugin`-объекты не копировать и не пересоздавать.
+
+## План тестирования
+
+### Общие требования
+
+- Новые тесты добавить в [tests/units/components/test_slot.py](/Users/pomponchik/Desktop/Projects/symplug/tests/units/components/test_slot.py).
+- Каждый новый или измененный тест должен иметь docstring в стиле существующих тестов проекта: начинаться с одной фразы с общим смыслом теста; при сложной семантике или фиксации поведения, явно не описанного в README, можно добавить один или несколько абзацев с уточнением, какое поведение фиксируется и как именно тест это делает.
+- Использовать уже подключенный `LockTraceWrapper`; не импортировать `RLock`.
+- В lock-boundary тестах подменять `_load_entrypoints()` на функцию без собственной блокировки, которая делает `slot.lock.notify('load')`.
+- Для snapshot использовать traced list, чей `__iter__` делает `slot.lock.notify('snapshot')`.
+- Не подменять `slot.backed_caller` и не monkeypatch-ить `slot_module.CallerWithPlugins`: тесты должны идти через реальный `Slot.__call__` и проверять observable contract, а не конкретный конструктор.
+- Проверять не только `was_event_locked(...)`, но и точный порядок trace: `acquire -> load -> snapshot -> release -> body`.
+- Не добавлять thread-based deadlock test в unit suite: он зависит от scheduling; deterministic trace/snapshot tests фиксируют контракт точнее.
+
+### Новые и изменяемые тесты
+
+1. Заменить `test_call_is_protected_by_slot_lock`
+   - Новое имя: `test_call_snapshots_registered_plugins_under_slot_lock_but_runs_plugins_after_release`.
+   - Фиксирует: для слота с зарегистрированными плагинами `_load_entrypoints()` и создание snapshot списка плагинов происходят под `Slot.lock`, а plugin body выполняется после release.
+   - Сценарий: создать `Slot` с list-return body, зарегистрировать plugin, обернуть `slot.lock`, заменить `slot.plugins.plugins` на traced list, `_load_entrypoints()` на `load`.
+   - Plugin body делает `slot.lock.notify('plugin-body')` и возвращает `'plugin'`.
+   - Ожидание: `slot() == ['plugin']`, `load` и `snapshot` под lock, `plugin-body` не под lock, trace строго `acquire/load/snapshot/release/plugin-body`.
+
+2. Добавить `test_call_snapshots_empty_plugins_under_lock_but_runs_fallback_after_release`
+   - Фиксирует: для пустого слота без плагинов fallback body считается пользовательским кодом и тоже не выполняется под `Slot.lock`.
+   - Сценарий: default body делает `slot.lock.notify('fallback-body')` и возвращает `['fallback']`.
+   - `slot.plugins.plugins` заменить на пустой traced list, `_load_entrypoints()` на `load`.
+   - Ожидание: `slot() == ['fallback']`, `load` и `snapshot` под lock, `fallback-body` не под lock, trace строго `acquire/load/snapshot/release/fallback-body`.
+
+3. Добавить `test_plugins_registered_during_dispatch_are_called_on_later_calls_only`
+   - Фиксирует: `Slot.__call__` dispatch-ит по стабильному snapshot, а не по live list.
+   - Сценарий: первый plugin `registrar` при первом вызове регистрирует plugin `late`, затем возвращает `'registrar'`.
+   - Ожидание: первый `slot()` возвращает `['registrar']`; второй `slot()` возвращает `['registrar', 'late']`.
+   - В тесте не использовать sleeps, threads или timeouts.
+
+4. Сохранить `test_bool_is_protected_by_slot_lock` без изменения поведения
+   - `__bool__` не является частью issue и продолжает проверять `bool(self.backed_caller)` под lock.
+   - При необходимости обновить только соседние assertions/import ordering после удаления старого `test_call_is_protected_by_slot_lock`.
+
+5. Существующие `.one`, `__iter__`, `__getitem__`, `__delitem__`, `__contains__`, `__len__`, `keys`, `_pop_plugins`, `_load_entrypoints`, `_add_plugin` тесты не расширять
+   - Они относятся к предыдущему плану потокобезопасности и уже фиксируют registry operations under lock.
+   - Issue #28 меняет только границу `Slot.__call__`.
+
+## Проверка полноты
+
+- `Slot.__call__` больше не вызывает plugin/default body под `Slot.lock`.
+- Отдельно покрыты оба пути dispatch: слот с зарегистрированными плагинами и пустой слот с fallback body.
+- Lazy loading остается под `Slot.lock`.
+- Snapshot-backed `CallerWithPlugins` создается под `Slot.lock`.
+- Dispatch идет через локальный caller со snapshot, поэтому не видит плагины, добавленные во время текущего вызова.
+- Entry point loading остается out of scope.
+
+## Проверка
+
+- Запустить из активированного виртуального окружения:
+  - `pytest tests/units/components/test_slot.py`
+  - `coverage run --source=pristan --omit="*tests*" -m pytest --cache-clear --assert=plain && coverage report -m --fail-under=100`
+  - `coverage run --branch --source=pristan --omit="*tests*" -m pytest --cache-clear --assert=plain && coverage report -m --fail-under=100`
+  - `ruff check pristan`
+  - `ruff check tests`
+  - `mypy --strict pristan`
+  - `mypy tests --exclude tests/typing`
+
+## Предположения
+
+- Snapshot копирует только контейнер списка: он содержит ссылки на те же `Plugin`-объекты, поэтому состояние `run_once` и прочие object-level semantics сохраняются.
+- Локальный `backed_caller` должен создаваться внутри lock, но вызываться только после release.
+- README и публичную документацию не менять, если новый контракт полностью покрыт тестами и issue не требует пользовательского текста.

pristan/components/slot.py

@@ -109,7 +109,9 @@ def __init__(self, slot_function: SlotFunction[SlotParameters, SlotResult[Plugin
     def __call__(self, *args: SlotParameters.args, **kwargs: SlotParameters.kwargs) -> SlotResult[PluginResult]:
         with self.lock:
             self._load_entrypoints()
-            return self.backed_caller(*args, **kwargs)
+            backed_caller = CallerWithPlugins(self.caller, list(self.plugins.plugins))
+
+        return backed_caller(*args, **kwargs)
 
     def __bool__(self) -> bool:
         with self.lock:

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "pristan"
-version = "0.0.22"
+version = "0.0.23"
 authors = [{ name = "Evgeniy Blinov", email = "zheni-b@yandex.ru" }]
 description = "Function-based plugin system with respect to typing"
 readme = "README.md"

tests/units/components/test_slot.py

@@ -27,34 +27,99 @@ def test_set_max_less_than_zero():
         Slot(lambda x: x, signature='.', slot_name='slot_name', max=-1, type_check=False, entrypoint_group='pristan', unique=False)
 
 
-def test_call_is_protected_by_slot_lock():
-    """Slot calls keep lazy loading and dispatch under the slot lock."""
-    def empty_body():
-        pass
+def test_call_snapshots_registered_plugins_under_slot_lock_but_runs_plugins_after_release():
+    """Slot loads entry points and snapshots registered plugins under its lock, then runs plugin bodies after release."""
+    def empty_body() -> List[str]:
+        return []
 
     slot = Slot(empty_body, signature=None, slot_name=None, max=None, type_check=True, entrypoint_group='pristan', unique=False)
+
+    @slot.plugin
+    def plugin():
+        slot.lock.notify('plugin-body')
+        return 'plugin'
+
     slot.lock = LockTraceWrapper(slot.lock)
 
-    class BackedCaller:
-        def __call__(self):
-            slot.lock.notify('call')
-            return 'result'
+    class PluginsList(list):
+        def __iter__(self):
+            slot.lock.notify('snapshot')
+            yield from super().__iter__()
 
     slot._load_entrypoints = lambda: slot.lock.notify('load')  # type: ignore[method-assign]
-    slot.backed_caller = BackedCaller()  # type: ignore[assignment]
+    slot.plugins.plugins = PluginsList(slot.plugins.plugins)
+
+    assert slot() == ['plugin']
+
+    assert slot.lock.was_event_locked('load')
+    assert slot.lock.was_event_locked('snapshot')
+    assert not slot.lock.was_event_locked('plugin-body', raise_exception=False)
+    assert [(event.type.value, event.identifier) for event in slot.lock.trace] == [
+        ('acquire', None),
+        ('action', 'load'),
+        ('action', 'snapshot'),
+        ('release', None),
+        ('action', 'plugin-body'),
+    ]
+
 
-    assert slot() == 'result'
+def test_call_snapshots_empty_plugins_under_lock_but_runs_fallback_after_release():
+    """Slot loads entry points and snapshots an empty plugin list under its lock, then runs the fallback body after release."""
+    def fallback_body() -> List[str]:
+        slot.lock.notify('fallback-body')
+        return ['fallback']
+
+    slot = Slot(fallback_body, signature=None, slot_name=None, max=None, type_check=True, entrypoint_group='pristan', unique=False)
+    slot.lock = LockTraceWrapper(slot.lock)
+
+    class PluginsList(list):
+        def __iter__(self):
+            slot.lock.notify('snapshot')
+            yield from super().__iter__()
+
+    slot._load_entrypoints = lambda: slot.lock.notify('load')  # type: ignore[method-assign]
+    slot.plugins.plugins = PluginsList()
+
+    assert slot() == ['fallback']
 
     assert slot.lock.was_event_locked('load')
-    assert slot.lock.was_event_locked('call')
+    assert slot.lock.was_event_locked('snapshot')
+    assert not slot.lock.was_event_locked('fallback-body', raise_exception=False)
     assert [(event.type.value, event.identifier) for event in slot.lock.trace] == [
         ('acquire', None),
         ('action', 'load'),
-        ('action', 'call'),
+        ('action', 'snapshot'),
         ('release', None),
+        ('action', 'fallback-body'),
     ]
 
 
+def test_plugins_registered_during_dispatch_are_called_on_later_calls_only():
+    """Slot uses a stable plugin snapshot, so plugins registered during a call run only on later calls."""
+    def empty_body() -> List[str]:
+        return []
+
+    slot = Slot(empty_body, signature=None, slot_name=None, max=None, type_check=True, entrypoint_group='pristan', unique=False)
+    slot._load_entrypoints = lambda: None  # type: ignore[method-assign]
+    late_was_registered = False
+
+    @slot.plugin
+    def registrar():
+        nonlocal late_was_registered
+
+        if not late_was_registered:
+            late_was_registered = True
+
+            @slot.plugin
+            def late():
+                return 'late'
+
+        return 'registrar'
+
+    assert slot() == ['registrar']
+    assert slot() == ['registrar', 'late']
+
+
 def test_bool_is_protected_by_slot_lock():
     """Slot truth-value checks keep lazy loading and backed-caller inspection under the slot lock."""
     def empty_body():