Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions docs/plans/5.md
Original file line number Diff line number Diff line change
@@ -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 не требует пользовательского текста.
4 changes: 3 additions & 1 deletion pristan/components/slot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
89 changes: 77 additions & 12 deletions tests/units/components/test_slot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
Loading