From f1f056c7277252dddb9dc5d32dd4d6465518b536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Mon, 13 Jul 2026 21:43:33 +0300 Subject: [PATCH 1/9] Add a plan --- docs/plans/1.md | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 docs/plans/1.md diff --git a/docs/plans/1.md b/docs/plans/1.md new file mode 100644 index 0000000..eca3487 --- /dev/null +++ b/docs/plans/1.md @@ -0,0 +1,47 @@ +# Вынести базовую реализацию SmartLock + +## Motivation +- Подготовить код к будущему семейству смартлоков: общая текущая реализация должна жить в базовом `AbstractSmartLock`, а публичный `SmartLock` должен стать конкретной тонкой оберткой. +- Сохранить поведение и публичный импорт `SmartLock` без изменений. +- Сделать перенос через `git mv`, чтобы по diff было легко проверить, что реализация не потерялась и была только переименована/расширена. + +## Summary +- Перед изменениями сохранить этот план в `docs/plans/1.md`. +- Перенести текущую реализацию `SmartLock` в `AbstractSmartLock` через `git mv`. +- Оставить публичный `SmartLock` тонким наследником без изменения поведения. +- Добавить один тест, фиксирующий, что `SmartLock` является потомком `AbstractSmartLock`. + +## Key Changes +- Выполнить `git mv locklib/locks/smart_lock/lock.py locklib/locks/smart_lock/abstract.py`. +- В `abstract.py` переименовать `SmartLock` в `AbstractSmartLock`, добавить импорт `ABC` из `abc` и объявить `class AbstractSmartLock(ABC):`; абстрактные методы и guard на прямое инстанцирование не добавлять. +- Создать новый `locklib/locks/smart_lock/lock.py` с минимальным кодом: + ```python + from locklib.locks.smart_lock.abstract import AbstractSmartLock + + + class SmartLock(AbstractSmartLock): + ... + ``` +- В `tests/units/locks/smart_lock/test_lock.py` импортировать `AbstractSmartLock` и добавить тест с докстрингом в стиле проекта: + ```python + def test_smart_lock_is_abstract_smart_lock_subclass(): + """SmartLock keeps AbstractSmartLock as its base implementation.""" + assert issubclass(SmartLock, AbstractSmartLock) + ``` +- Не менять `locklib/__init__.py`: `from locklib import SmartLock` должен работать как раньше. +- Не экспортировать `AbstractSmartLock` на верхнем уровне пакета. + +## Test Plan +- Запустить `ruff check locklib`. +- Запустить `ruff check tests`. +- Запустить `mypy locklib --strict`. +- Запустить `mypy tests`. +- Запустить coverage-команду из `AGENTS.md`: + ```bash + coverage run --source=locklib --omit="*tests*" -m pytest --cache-clear --assert=plain && coverage report -m --fail-under=100 + ``` + +## Assumptions +- `AbstractSmartLock` пока остается полностью рабочей реализацией и технически может быть инстанцирован напрямую, потому что у него нет абстрактных методов. +- Рассматривалась возможность написать тесты, которые проверят невозможность прямого инстанцирования `AbstractSmartLock`, но выяснилось, что ABC-классы без абстрактных методов не поднимают исключение при инстанцировании, так что решили пока такие тесты не добавлять. +- `SmartLock` должен сохранить весь текущий runtime API, сообщения исключений и поведение deadlock detection. From 9f01cb0d0e1bff17859b8ef29eab075d27e38595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Mon, 13 Jul 2026 21:45:17 +0300 Subject: [PATCH 2/9] Move all the logic from SmartLock to AbstractSmartLock --- locklib/locks/smart_lock/abstract.py | 68 ++++++++++++++++++++++++++++ locklib/locks/smart_lock/lock.py | 67 ++------------------------- 2 files changed, 71 insertions(+), 64 deletions(-) create mode 100644 locklib/locks/smart_lock/abstract.py diff --git a/locklib/locks/smart_lock/abstract.py b/locklib/locks/smart_lock/abstract.py new file mode 100644 index 0000000..c39f224 --- /dev/null +++ b/locklib/locks/smart_lock/abstract.py @@ -0,0 +1,68 @@ +try: + from threading import ( # type: ignore[attr-defined, unused-ignore] + Lock, + get_native_id, + ) +except ImportError: # pragma: no cover + from threading import Lock # get_native_id is available only since python 3.8 + from threading import get_ident as get_native_id + +from abc import ABC +from collections import deque +from types import TracebackType +from typing import Deque, Dict, Optional, Type + +from locklib.locks.smart_lock.graph import LocksGraph + +graph = LocksGraph() + + +class AbstractSmartLock(ABC): # noqa: B024 + def __init__(self, local_graph: LocksGraph = graph) -> None: + self.graph: LocksGraph = local_graph + self.lock: Lock = Lock() + self.deque: Deque[int] = deque() + self.local_locks: Dict[int, Lock] = {} + + def __enter__(self) -> None: + self.acquire() + + def __exit__(self, exception_type: Optional[Type[BaseException]], exception_value: Optional[BaseException], traceback: Optional[TracebackType]) -> None: + self.release() + + def acquire(self) -> None: + thread_id = get_native_id() + previous_element_lock = None + + with self.lock, self.graph.lock: + if not self.deque: + self.deque.appendleft(thread_id) + self.local_locks[thread_id] = Lock() + self.local_locks[thread_id].acquire() + else: + previous_element = self.deque[0] + self.graph.add_link(thread_id, previous_element) + self.deque.appendleft(thread_id) + self.local_locks[thread_id] = Lock() + self.local_locks[thread_id].acquire() + previous_element_lock = self.local_locks[previous_element] + + if previous_element_lock is not None: + previous_element_lock.acquire() + + def release(self) -> None: + thread_id = get_native_id() + + with self.lock, self.graph.lock: + if thread_id not in self.local_locks: + raise RuntimeError('Release unlocked lock.') + + self.deque.pop() + lock = self.local_locks[thread_id] + del self.local_locks[thread_id] + + if len(self.deque) != 0: + next_element = self.deque[-1] + self.graph.delete_link(next_element, thread_id) + + lock.release() diff --git a/locklib/locks/smart_lock/lock.py b/locklib/locks/smart_lock/lock.py index 3410f69..8b7078a 100644 --- a/locklib/locks/smart_lock/lock.py +++ b/locklib/locks/smart_lock/lock.py @@ -1,66 +1,5 @@ -try: - from threading import ( # type: ignore[attr-defined, unused-ignore] - Lock, - get_native_id, - ) -except ImportError: # pragma: no cover - from threading import Lock # get_native_id is available only since python 3.8 - from threading import get_ident as get_native_id +from locklib.locks.smart_lock.abstract import AbstractSmartLock -from collections import deque -from types import TracebackType -from typing import Deque, Dict, Optional, Type -from locklib.locks.smart_lock.graph import LocksGraph - -graph = LocksGraph() - -class SmartLock: - def __init__(self, local_graph: LocksGraph = graph) -> None: - self.graph: LocksGraph = local_graph - self.lock: Lock = Lock() - self.deque: Deque[int] = deque() - self.local_locks: Dict[int, Lock] = {} - - def __enter__(self) -> None: - self.acquire() - - def __exit__(self, exception_type: Optional[Type[BaseException]], exception_value: Optional[BaseException], traceback: Optional[TracebackType]) -> None: - self.release() - - def acquire(self) -> None: - thread_id = get_native_id() - previous_element_lock = None - - with self.lock, self.graph.lock: - if not self.deque: - self.deque.appendleft(thread_id) - self.local_locks[thread_id] = Lock() - self.local_locks[thread_id].acquire() - else: - previous_element = self.deque[0] - self.graph.add_link(thread_id, previous_element) - self.deque.appendleft(thread_id) - self.local_locks[thread_id] = Lock() - self.local_locks[thread_id].acquire() - previous_element_lock = self.local_locks[previous_element] - - if previous_element_lock is not None: - previous_element_lock.acquire() - - def release(self) -> None: - thread_id = get_native_id() - - with self.lock, self.graph.lock: - if thread_id not in self.local_locks: - raise RuntimeError('Release unlocked lock.') - - self.deque.pop() - lock = self.local_locks[thread_id] - del self.local_locks[thread_id] - - if len(self.deque) != 0: - next_element = self.deque[-1] - self.graph.delete_link(next_element, thread_id) - - lock.release() +class SmartLock(AbstractSmartLock): + ... From da1bfc2fb61e96a50baa5085f065978b98987909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Mon, 13 Jul 2026 21:51:03 +0300 Subject: [PATCH 3/9] Add test to verify SmartLock inherits from AbstractSmartLock --- tests/units/locks/smart_lock/test_lock.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/units/locks/smart_lock/test_lock.py b/tests/units/locks/smart_lock/test_lock.py index bfa8e39..16c7a03 100644 --- a/tests/units/locks/smart_lock/test_lock.py +++ b/tests/units/locks/smart_lock/test_lock.py @@ -6,6 +6,12 @@ from full_match import match from locklib import DeadLockError, SmartLock +from locklib.locks.smart_lock.abstract import AbstractSmartLock + + +def test_smart_lock_is_abstract_smart_lock_subclass(): + """SmartLock keeps AbstractSmartLock as its base implementation.""" + assert issubclass(SmartLock, AbstractSmartLock) def test_release_unlocked(): From 6b8378607a5ab0871aea1f10f23aa4f35a57ecb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Mon, 13 Jul 2026 21:51:35 +0300 Subject: [PATCH 4/9] Bump version to 0.0.24 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d4e59b8..62fb578 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = 'setuptools.build_meta' [project] name = 'locklib' -version = '0.0.23' +version = '0.0.24' authors = [ { name='Evgeniy Blinov', email='zheni-b@yandex.ru' }, ] From 2031880e76d66d925a9af21adc0aad88891ae045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Wed, 15 Jul 2026 12:41:08 +0300 Subject: [PATCH 5/9] Add a plan --- docs/plans/2.md | 181 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 docs/plans/2.md diff --git a/docs/plans/2.md b/docs/plans/2.md new file mode 100644 index 0000000..58ed26d --- /dev/null +++ b/docs/plans/2.md @@ -0,0 +1,181 @@ +# План по Issue #28: recursive acquire для `SmartLock` + +## Motivation + +- `SmartLock` заявлен как lock, который превращает deadlock в исключение, но self-recursive acquire сейчас остается обычным зависанием. +- Нужно закрыть этот пропуск с сохранением существующего поведения межпоточной deadlock detection. +- `Proposed solution` из issue задает совместимую модель: `SmartLock` ведет себя как нерекурсивный lock, а новый `SmartRLock` дает рекурсивную альтернативу. +- Подготовительный рефакторинг уже выполнен: общая текущая реализация живет в `AbstractSmartLock`, поэтому fix делается через расширение этого базового класса. + +## Summary + +- Источник: [GitHub Issue #28](https://github.com/mutating/locklib/issues/28). +- Перед имплементацией сохранить этот план в [docs/plans/2.md](/Users/pomponchik/Desktop/Projects/locklib/docs/plans/2.md). +- Подготовительный рефакторинг уже выполнен: текущая реализация перенесена в `AbstractSmartLock`, а `SmartLock` стал тонким наследником. +- Решение: расширить существующий `AbstractSmartLock`, оставить `SmartLock` нерекурсивным и добавить рекурсивный `SmartRLock`. + +## Public APIs / Interfaces / Types + +- Добавить публичный `SmartRLock` и экспортировать его из `locklib.__init__`. +- Сохранить текущую сигнатуру `SmartLock.__init__(local_graph: LocksGraph = graph)`. +- Для `SmartRLock` использовать такую же сигнатуру конструктора. +- Recursive deadlock у `SmartLock` поднимать как существующий `DeadLockError`. +- `AbstractSmartLock` оставить внутренним типом пакета `locklib.locks.smart_lock`. + +## Изменения реализации + +- Работать с уже существующим [abstract.py](/Users/pomponchik/Desktop/Projects/locklib/locklib/locks/smart_lock/abstract.py). +- В `AbstractSmartLock` добавить class-level флаг `recursive`. +- В `SmartLock` явно задать `recursive = False`. +- Добавить `locklib/locks/smart_lock/rlock.py` с `SmartRLock(AbstractSmartLock)` и `recursive = True`. +- Shared graph по умолчанию оставить общим для `SmartLock` и `SmartRLock`, чтобы mixed deadlocks между ними тоже детектировались. +- README расширить в разделе про `SmartLock`: добавить короткое описание `SmartRLock`, пример импорта `from locklib import SmartRLock`, пример вложенного `with lock, lock:` и пояснение, что количество `release()` должно соответствовать количеству `acquire()`. + +## Механизм recursion depth + +- Recursion depth хранить в `AbstractSmartLock`, например как `recursion_depths: Dict[int, int]`. +- Владельцем lock считать поток в правом краю очереди: `self.deque[-1]`. +- `local_locks` трактовать как состояние участия в очереди: там могут быть и владелец, и ожидающие потоки. +- На первом успешном acquire выставлять `recursion_depths[thread_id] = 1`. +- В `acquire()` перед обычной постановкой в очередь проверять self-recursion: `self.deque` заполнен и `self.deque[-1] == thread_id`. +- Для `SmartLock` при self-recursion сразу поднимать `DeadLockError`. +- Для `SmartRLock` при self-recursion увеличивать depth и возвращать управление с сохранением текущих `deque`, `local_locks` и graph. +- В `SmartRLock.release()` при depth больше `1` уменьшать счетчик и сохранять владение lock. +- При последнем `release()` выполнять обычное освобождение: `deque.pop()`, удалить `local_locks[thread_id]`, удалить `recursion_depths[thread_id]`, удалить graph edge для следующего ожидающего и release baton-lock. +- `LocksGraph` продолжает отвечать за wait-for отношения между потоками; recursion depth живет на уровне `AbstractSmartLock`. + +## План тестирования + +### Общие требования + +- Тесты разложить по файлам для `AbstractSmartLock`, `SmartLock` и `SmartRLock`. +- Сначала написать тесты и убедиться, что recursive acquire для текущего `SmartLock` воспроизводит баг без зависания всего тестового процесса. +- Для точных сообщений исключений использовать `full_match.match`, как в существующих тестах. +- Проверять не только факт исключения/успеха, но и восстановление состояния lock после сценария. +- Новые тесты должны иметь docstring в стиле существующих тестов проекта. + +### Реорганизация smart lock тестов + +- Создать `tests/units/locks/smart_lock/conftest.py` с фикстурой `smartlock_class`. +- `smartlock_class` параметризовать всеми текущими публичными наследниками `AbstractSmartLock`: `SmartLock` и `SmartRLock`. +- Создать файл универсальных тестов `tests/units/locks/smart_lock/test_abstract.py`. +- Создать файл тестов `SmartRLock`: `tests/units/locks/smart_lock/test_rlock.py`. +- Просмотреть существующие тесты в `tests/units/locks/smart_lock/test_lock.py` и разделить их на универсальные тесты поведения всех наследников `AbstractSmartLock` и тесты поведения конкретно `SmartLock`. +- Универсальные тесты перенести в `tests/units/locks/smart_lock/test_abstract.py`. +- Специфичные для `SmartLock` тесты оставить в `tests/units/locks/smart_lock/test_lock.py`. +- В универсальных тестах заменить прямое создание `SmartLock()` на `smartlock_class()`. +- В универсальных deadlock-тестах создавать все lock instances через `smartlock_class`, чтобы каждый наследник проходил один и тот же набор проверок. +- В `tests/units/locks/smart_lock/test_lock.py` оставить прямое использование `SmartLock` только в тестах, которые фиксируют именно нерекурсивное поведение `SmartLock`. +- `test_smart_lock_is_abstract_smart_lock_subclass` относится к специфичным для `SmartLock` тестам. + +### Универсальные тесты для наследников `AbstractSmartLock` + +1. Перенести и параметризовать `test_release_unlocked` + - Новое имя: `test_abstract_smart_lock_subclass_release_unlocked`. + - Фиксирует: каждый публичный наследник `AbstractSmartLock` поднимает `RuntimeError('Release unlocked lock.')` при `release()` без владения lock. + - Сценарий: тест принимает `smartlock_class`, создает lock и вызывает `release()`. + - Ожидание: точное сообщение исключения совпадает с существующим контрактом. + +2. Перенести и параметризовать `test_normal_using` + - Новое имя: `test_abstract_smart_lock_subclass_protects_shared_state`. + - Фиксирует: каждый публичный наследник `AbstractSmartLock` обеспечивает mutual exclusion под многопоточной нагрузкой. + - Сценарий: тест принимает `smartlock_class`, несколько потоков инкрементируют общий счетчик внутри `with lock:`. + - Ожидание: итоговый счетчик равен числу всех инкрементов. + +3. Перенести и параметризовать `test_raise_when_simple_deadlock` + - Новое имя: `test_abstract_smart_lock_subclass_detects_simple_deadlock`. + - Фиксирует: каждый публичный наследник `AbstractSmartLock` обнаруживает двухпоточный deadlock вместо зависания. + - Сценарий: два потока берут два lock instance в противоположном порядке. + - Ожидание: возникает один `DeadLockError`, unexpected exceptions отсутствуют, оба потока завершаются. + +4. Перенести и параметризовать `test_raise_when_not_so_simple_deadlock` + - Новое имя: `test_abstract_smart_lock_subclass_detects_three_lock_deadlock`. + - Фиксирует: каждый публичный наследник `AbstractSmartLock` обнаруживает циклическое ожидание из трех lock. + - Сценарий: три потока берут три lock instance в циклически сдвинутом порядке. + - Ожидание: возникает два `DeadLockError`, все потоки завершаются. + +5. Добавить `test_abstract_smart_lock_subclass_context_manager_blocks_other_threads_until_exit` + - Фиксирует: каждый публичный наследник `AbstractSmartLock` удерживает lock до выхода из context manager. + - Сценарий: тест принимает `smartlock_class`; основной поток входит в `with lock:`; worker thread пытается войти в тот же lock и после входа выставляет `entered_event`. + - Проверка ожидания: пока основной поток находится внутри `with`, вызвать `entered_event.wait(0.1)` и проверить, что он вернул `False`; после выхода из `with` сделать `thread.join()` и проверить, что `entered_event.is_set()` стал `True`. + +6. Добавить `test_abstract_smart_lock_subclass_release_from_non_owner_thread_raises` + - Фиксирует: каждый публичный наследник `AbstractSmartLock` разрешает `release()` только потоку-владельцу. + - Сценарий: основной поток делает `lock.acquire()`, worker thread вызывает `lock.release()` и сохраняет исключение. + - Ожидание: worker получает `RuntimeError('Release unlocked lock.')`; основной поток после этого успешно делает `release()`. + +### Тесты для `SmartLock` + +1. `test_smart_lock_is_abstract_smart_lock_subclass` + - Фиксирует: `SmartLock` остается наследником `AbstractSmartLock`. + +2. Добавить `test_smart_lock_raises_on_recursive_acquire_instead_of_hanging` + - Фиксирует: `SmartLock` завершает повторный `acquire()` тем же потоком через `DeadLockError` и сохраняет состояние после recovery. + - Сценарий: запускать весь self-recursive сценарий в daemon worker thread, потому что текущая реализация зависает на втором `acquire()`. + - Worker thread делает `lock.acquire()`, затем повторный `lock.acquire()`, ловит `DeadLockError`, делает `release()`, затем снова делает успешный `acquire()`, а следующий повторный `acquire()` снова получает `DeadLockError`. + - Проверка ожидания: после `thread.join(0.3)` поток должен быть завершен; иначе тест падает как воспроизведение исходного бага. + - Ожидание: первый и второй recursive acquire поднимают `DeadLockError`; acquire после release проходит успешно; финальный `release()` освобождает lock. + +3. Добавить `test_smart_lock_recursive_acquire_does_not_corrupt_state` + - Фиксирует: ошибка recursive acquire сохраняет рабочее состояние lock и graph. + - Сценарий: использовать отдельный `LocksGraph`, поймать `DeadLockError`, затем сделать обычный `release()`. + - Ожидание: graph пуст, lock можно снова `acquire()`/`release()`. + +4. Добавить `test_mixed_smart_lock_and_smart_rlock_detect_deadlock` + - Фиксирует: `SmartLock` и `SmartRLock` участвуют в одном wait-for graph. + - Параметризовать тест парами классов в двух порядках: `(SmartLock, SmartRLock)` и `(SmartRLock, SmartLock)`. + - Сценарий: два потока берут два lock instance в противоположном порядке, как в существующем simple deadlock тесте. + - Ожидание: возникает один `DeadLockError`, unexpected exceptions отсутствуют, оба потока завершаются. + +### Тесты для `SmartRLock` + +1. Добавить `test_smart_rlock_is_abstract_smart_lock_subclass` + - Фиксирует: `SmartRLock` является наследником `AbstractSmartLock`. + +2. Добавить `test_smart_rlock_allows_recursive_acquire_and_requires_matching_releases` + - Фиксирует: `SmartRLock` разрешает произвольную глубину рекурсивных acquire в проверяемом диапазоне и требует matching releases. + - Параметризовать тест значениями `number_of_acquires` от `0` до `10` включительно. + - Сценарий: создать `SmartRLock`, выполнить `acquire()` ровно `number_of_acquires` раз, затем выполнить `release()` ровно `number_of_acquires + 1` раз. + - Ожидание: первые `number_of_acquires` вызовов `release()` успешны, последний дополнительный `release()` дает `RuntimeError('Release unlocked lock.')`; для `number_of_acquires == 0` первый же `release()` дает это исключение. + +3. Добавить `test_smart_rlock_nested_context_manager_keeps_lock_until_outer_exit` + - Фиксирует: внутренний выход из рекурсивного context manager уменьшает depth и сохраняет владение `SmartRLock`. + - Сценарий: основной поток входит в `with lock:`, затем во вложенный `with lock:`; после выхода из внутреннего блока запускает worker thread, который пытается войти в тот же lock и выставить `entered_event`. + - Проверка ожидания: до выхода из внешнего `with lock:` `entered_event.wait(0.1)` возвращает `False`; после выхода из внешнего блока `thread.join()` завершается и `entered_event.is_set()` равен `True`. + +4. Добавить `test_smart_rlock_partial_release_does_not_unlock_for_other_threads` + - Фиксирует: `release()` при recursion depth больше `1` уменьшает счетчик и сохраняет владение lock. + - Сценарий: основной поток делает `lock.acquire()` два раза; worker пытается войти в `with lock:` и выставить `entered_event`; основной поток делает один `release()`. + - Проверка ожидания: после первого `release()` `entered_event.wait(0.1)` возвращает `False`; после второго `release()` worker завершается и `entered_event.is_set()` равен `True`. + +5. Добавить `test_smart_rlock_recursive_acquire_does_not_touch_graph` + - Фиксирует: рекурсивный acquire оставляет `LocksGraph` без новых wait-for edges. + - Сценарий: создать отдельный `LocksGraph`, передать его в `SmartRLock(local_graph=graph)`, выполнить `acquire()` два раза. + - Ожидание: `graph.links` пуст до обоих `release()`. + +6. Добавить `test_smart_rlock_recursion_depth_resets_after_full_release` + - Фиксирует: recursion depth очищается после полного освобождения lock и следующий цикл владения начинается с depth `1`. + - Сценарий: сделать `acquire()` три раза, затем `release()` три раза; после этого снова сделать один `acquire()` и один `release()`. + - Ожидание: следующий дополнительный `release()` дает `RuntimeError('Release unlocked lock.')`, а повторный цикл использует новый depth. + +### Документация и protocol/runtime тесты + +- В [tests/units/protocols/test_lock.py](/Users/pomponchik/Desktop/Projects/locklib/tests/units/protocols/test_lock.py) добавить `SmartRLock` как `LockProtocol`. +- В [tests/units/protocols/test_context_lock.py](/Users/pomponchik/Desktop/Projects/locklib/tests/units/protocols/test_context_lock.py) добавить `SmartRLock` как `ContextLockProtocol`. +- В [tests/documentation/test_readme.py](/Users/pomponchik/Desktop/Projects/locklib/tests/documentation/test_readme.py) добавить проверки под новые README-примеры: импорт `SmartRLock`, runtime membership в `LockProtocol`/`ContextLockProtocol`, рекурсивное использование `with lock, lock:`. + +## Проверка + +- `python3 -m pytest tests/units/locks/smart_lock` +- `python3 -m pytest tests/units/protocols tests/documentation/test_readme.py` +- `coverage run --branch --source=locklib --omit="*tests*" -m pytest --cache-clear --assert=plain && coverage report -m --fail-under=100` +- `ruff check locklib` +- `ruff check tests` +- `mypy locklib --strict` +- `mypy tests --check-untyped-defs` + +## Предположения + +- Фразу из issue “recursive acquire ... simply does nothing” трактуем как “рекурсивный acquire возвращает управление без блокировки и wait-for graph изменений”, а recursion depth все равно учитываем. +- `SmartRLock` является синхронным context lock. +- Версию пакета меняем только при отдельной release-подготовке. From 859bebf1cf31a862c1db663905dc0a5fcc096660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Wed, 15 Jul 2026 19:06:34 +0300 Subject: [PATCH 6/9] Add recursive SmartRLock and support for recursive locking in AbstractSmartLock --- locklib/__init__.py | 1 + locklib/locks/smart_lock/abstract.py | 21 +++++++++++++++++++-- locklib/locks/smart_lock/lock.py | 2 +- locklib/locks/smart_lock/rlock.py | 5 +++++ 4 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 locklib/locks/smart_lock/rlock.py diff --git a/locklib/__init__.py b/locklib/__init__.py index e629e28..9d32e28 100644 --- a/locklib/__init__.py +++ b/locklib/__init__.py @@ -10,6 +10,7 @@ ) from locklib.locks.empty.empty_lock import EmptyLock as EmptyLock from locklib.locks.smart_lock.lock import SmartLock as SmartLock +from locklib.locks.smart_lock.rlock import SmartRLock as SmartRLock from locklib.locks.tracer.tracer import ( LockTraceWrapper as LockTraceWrapper, ) diff --git a/locklib/locks/smart_lock/abstract.py b/locklib/locks/smart_lock/abstract.py index c39f224..d696405 100644 --- a/locklib/locks/smart_lock/abstract.py +++ b/locklib/locks/smart_lock/abstract.py @@ -10,19 +10,23 @@ from abc import ABC from collections import deque from types import TracebackType -from typing import Deque, Dict, Optional, Type +from typing import ClassVar, Deque, Dict, Optional, Type +from locklib.errors import DeadLockError from locklib.locks.smart_lock.graph import LocksGraph graph = LocksGraph() class AbstractSmartLock(ABC): # noqa: B024 + recursive: ClassVar[bool] = False + def __init__(self, local_graph: LocksGraph = graph) -> None: self.graph: LocksGraph = local_graph self.lock: Lock = Lock() self.deque: Deque[int] = deque() self.local_locks: Dict[int, Lock] = {} + self.recursion_depths: Dict[int, int] = {} def __enter__(self) -> None: self.acquire() @@ -35,16 +39,24 @@ def acquire(self) -> None: previous_element_lock = None with self.lock, self.graph.lock: + if self.deque and self.deque[-1] == thread_id: + if not self.recursive: + raise DeadLockError(f'A cycle between {thread_id}th and {thread_id}th threads has been detected.') + self.recursion_depths[thread_id] += 1 + return + if not self.deque: self.deque.appendleft(thread_id) self.local_locks[thread_id] = Lock() self.local_locks[thread_id].acquire() + self.recursion_depths[thread_id] = 1 else: previous_element = self.deque[0] self.graph.add_link(thread_id, previous_element) self.deque.appendleft(thread_id) self.local_locks[thread_id] = Lock() self.local_locks[thread_id].acquire() + self.recursion_depths[thread_id] = 1 previous_element_lock = self.local_locks[previous_element] if previous_element_lock is not None: @@ -54,12 +66,17 @@ def release(self) -> None: thread_id = get_native_id() with self.lock, self.graph.lock: - if thread_id not in self.local_locks: + if not self.deque or self.deque[-1] != thread_id: raise RuntimeError('Release unlocked lock.') + if self.recursive and self.recursion_depths[thread_id] > 1: + self.recursion_depths[thread_id] -= 1 + return + self.deque.pop() lock = self.local_locks[thread_id] del self.local_locks[thread_id] + del self.recursion_depths[thread_id] if len(self.deque) != 0: next_element = self.deque[-1] diff --git a/locklib/locks/smart_lock/lock.py b/locklib/locks/smart_lock/lock.py index 8b7078a..cbf8f57 100644 --- a/locklib/locks/smart_lock/lock.py +++ b/locklib/locks/smart_lock/lock.py @@ -2,4 +2,4 @@ class SmartLock(AbstractSmartLock): - ... + recursive = False diff --git a/locklib/locks/smart_lock/rlock.py b/locklib/locks/smart_lock/rlock.py new file mode 100644 index 0000000..fe80f48 --- /dev/null +++ b/locklib/locks/smart_lock/rlock.py @@ -0,0 +1,5 @@ +from locklib.locks.smart_lock.abstract import AbstractSmartLock + + +class SmartRLock(AbstractSmartLock): + recursive = True From 6ea2256b64f9f50cd6a641a527599aff19959906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Wed, 15 Jul 2026 19:06:51 +0300 Subject: [PATCH 7/9] Add SmartRLock to README and tests with recursive context examples --- README.md | 31 +++++++++++++++++++++++++++--- tests/documentation/test_readme.py | 27 +++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 00576ec..ddef519 100644 --- a/README.md +++ b/README.md @@ -70,13 +70,14 @@ from multiprocessing import Lock as MLock from threading import Lock as TLock, RLock as TRLock from asyncio import Lock as ALock -from locklib import SmartLock, LockProtocol +from locklib import SmartLock, SmartRLock, LockProtocol print(isinstance(MLock(), LockProtocol)) # True print(isinstance(TLock(), LockProtocol)) # True print(isinstance(TRLock(), LockProtocol)) # True print(isinstance(ALock(), LockProtocol)) # True print(isinstance(SmartLock(), LockProtocol)) # True +print(isinstance(SmartRLock(), LockProtocol)) # True ``` However, most idiomatic Python code uses locks as context managers. If your code does too, you can use one of the two protocols derived from the base `LockProtocol`: `ContextLockProtocol` or `AsyncContextLockProtocol`. Thus, the protocol hierarchy looks like this: @@ -89,18 +90,19 @@ LockProtocol `ContextLockProtocol` describes objects that satisfy `LockProtocol` and also implement the [context manager protocol](https://docs.python.org/3/library/stdtypes.html#typecontextmanager). Similarly,`AsyncContextLockProtocol` describes objects that satisfy `LockProtocol` and implement the [asynchronous context manager](https://docs.python.org/3/reference/datamodel.html#async-context-managers) protocol. -Almost all standard library locks, as well as `SmartLock`, satisfy `ContextLockProtocol`: +Almost all standard library locks, as well as `SmartLock` and `SmartRLock`, satisfy `ContextLockProtocol`: ```python from multiprocessing import Lock as MLock from threading import Lock as TLock, RLock as TRLock -from locklib import SmartLock, ContextLockProtocol +from locklib import SmartLock, SmartRLock, ContextLockProtocol print(isinstance(MLock(), ContextLockProtocol)) # True print(isinstance(TLock(), ContextLockProtocol)) # True print(isinstance(TRLock(), ContextLockProtocol)) # True print(isinstance(SmartLock(), ContextLockProtocol)) # True +print(isinstance(SmartRLock(), ContextLockProtocol)) # True ``` However, the [`asyncio.Lock`](https://docs.python.org/3/library/asyncio-sync.html#asyncio.Lock) belongs to a separate category and `AsyncContextLockProtocol` is needed to describe it: @@ -209,6 +211,29 @@ If you want to catch this exception, you can also import it from `locklib`: from locklib import DeadLockError ``` +`SmartLock` is deliberately not recursive: acquiring the same instance twice from the same thread raises `DeadLockError`. When recursive ownership is needed, use `SmartRLock` instead. It keeps the same deadlock detection for waits between threads, but lets the owning thread enter the same lock repeatedly: + +```python +from locklib import SmartRLock + +lock = SmartRLock() + +with lock, lock: + pass +``` + +If you use explicit `acquire()` and `release()` calls, every successful acquire must have a matching release: + +```python +from locklib import SmartRLock + +lock = SmartRLock() + +lock.acquire() +lock.acquire() +lock.release() +lock.release() +``` ## Test your locks diff --git a/tests/documentation/test_readme.py b/tests/documentation/test_readme.py index 129c3c7..21276ca 100644 --- a/tests/documentation/test_readme.py +++ b/tests/documentation/test_readme.py @@ -10,20 +10,22 @@ EmptyLock, LockProtocol, SmartLock, + SmartRLock, ) def test_lock_protocols_basic(): """ - The README lock examples satisfy LockProtocol at runtime. + The basic lock protocol example objects satisfy LockProtocol at runtime. - This checks multiprocessing.Lock, threading.Lock, threading.RLock, asyncio.Lock, and SmartLock by protocol membership, without exercising locking behavior. + This checks multiprocessing.Lock, threading.Lock, threading.RLock, asyncio.Lock, SmartLock, and SmartRLock by protocol membership, without exercising locking behavior. """ assert isinstance(MLock(), LockProtocol) assert isinstance(TLock(), LockProtocol) assert isinstance(TRLock(), LockProtocol) assert isinstance(ALock(), LockProtocol) assert isinstance(SmartLock(), LockProtocol) + assert isinstance(SmartRLock(), LockProtocol) def test_inheritance_order(): @@ -42,12 +44,31 @@ def test_almost_all_lock_are_context_locks(): """ ContextLockProtocol describes the listed synchronous locks as context-manager locks. - The README smoke test checks multiprocessing.Lock, threading.Lock, threading.RLock, and SmartLock by runtime protocol membership without exercising their locking behavior. + This checks multiprocessing.Lock, threading.Lock, threading.RLock, SmartLock, and SmartRLock by runtime protocol membership without exercising their locking behavior. """ assert isinstance(MLock(), ContextLockProtocol) assert isinstance(TLock(), ContextLockProtocol) assert isinstance(TRLock(), ContextLockProtocol) assert isinstance(SmartLock(), ContextLockProtocol) + assert isinstance(SmartRLock(), ContextLockProtocol) + + +def test_smart_rlock_nested_context_manager_readme_example(): + """The SmartRLock context-manager example can enter the same lock recursively.""" + lock = SmartRLock() + + with lock, lock: + pass + + +def test_smart_rlock_explicit_acquire_release_readme_example(): + """The SmartRLock explicit acquire example matches two acquires with two releases.""" + lock = SmartRLock() + + lock.acquire() + lock.acquire() + lock.release() + lock.release() def test_asyncio_lock_is_async_context_lock(): From 6ac1ac01ded1757a9554e2f712212cad0a2223af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Wed, 15 Jul 2026 19:07:52 +0300 Subject: [PATCH 8/9] Add smartlock_class fixture for testing SmartLock and SmartRLock --- tests/units/locks/smart_lock/conftest.py | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tests/units/locks/smart_lock/conftest.py diff --git a/tests/units/locks/smart_lock/conftest.py b/tests/units/locks/smart_lock/conftest.py new file mode 100644 index 0000000..90f5646 --- /dev/null +++ b/tests/units/locks/smart_lock/conftest.py @@ -0,0 +1,11 @@ +from typing import Type + +import pytest + +from locklib import SmartLock, SmartRLock +from locklib.locks.smart_lock.abstract import AbstractSmartLock + + +@pytest.fixture(params=[SmartLock, SmartRLock]) +def smartlock_class(request: pytest.FixtureRequest) -> Type[AbstractSmartLock]: + return request.param From d733449ccc2bde89a5497d40d78765ea38d69069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Wed, 15 Jul 2026 19:09:30 +0300 Subject: [PATCH 9/9] Add tests for SmartLock and SmartRLock deadlock detection, recursion, and context manager behavior --- tests/units/locks/smart_lock/test_abstract.py | 412 ++++++++++++++++++ tests/units/locks/smart_lock/test_lock.py | 329 ++++++-------- tests/units/locks/smart_lock/test_rlock.py | 174 ++++++++ tests/units/protocols/test_context_lock.py | 10 +- tests/units/protocols/test_lock.py | 4 +- 5 files changed, 742 insertions(+), 187 deletions(-) create mode 100644 tests/units/locks/smart_lock/test_abstract.py create mode 100644 tests/units/locks/smart_lock/test_rlock.py diff --git a/tests/units/locks/smart_lock/test_abstract.py b/tests/units/locks/smart_lock/test_abstract.py new file mode 100644 index 0000000..93e7a48 --- /dev/null +++ b/tests/units/locks/smart_lock/test_abstract.py @@ -0,0 +1,412 @@ +from threading import Event, Lock, Thread +from time import monotonic, sleep +from typing import List, Sequence, Type + +import pytest +from full_match import match + +from locklib.errors import DeadLockError +from locklib.locks.smart_lock.abstract import AbstractSmartLock +from locklib.locks.smart_lock.abstract import graph as shared_graph +from locklib.locks.smart_lock.graph import LocksGraph + + +def test_abstract_smart_lock_subclass_is_abstract_smart_lock_subclass(smartlock_class: Type[AbstractSmartLock]) -> None: + """Every public smart lock class inherits from AbstractSmartLock.""" + assert issubclass(smartlock_class, AbstractSmartLock) + + +def test_abstract_smart_lock_subclass_release_unlocked(smartlock_class: Type[AbstractSmartLock]) -> None: + """Every public AbstractSmartLock subclass rejects release without ownership.""" + lock = smartlock_class() + + with pytest.raises(RuntimeError, match=match('Release unlocked lock.')): + lock.release() + + +def test_abstract_smart_lock_subclass_protects_shared_state(smartlock_class: Type[AbstractSmartLock]) -> None: + """Every public AbstractSmartLock subclass serializes contended increments without losing updates.""" + number_of_threads = 5 + number_of_increments_per_thread = 1000 + + lock = smartlock_class(local_graph=LocksGraph()) + counter = 0 + errors_lock = Lock() + unexpected_errors: List[Exception] = [] + + def increment_counter() -> None: + nonlocal counter + + try: + for _ in range(number_of_increments_per_thread): + with lock: + counter_snapshot = counter + sleep(0) + counter = counter_snapshot + 1 + except Exception as error: # noqa: BLE001 + with errors_lock: + unexpected_errors.append(error) + + threads = [Thread(target=increment_counter, daemon=True) for _ in range(number_of_threads)] + + for thread in threads: + thread.start() + for thread in threads: + thread.join(2) + + assert all(not thread.is_alive() for thread in threads) + + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert counter == number_of_threads * number_of_increments_per_thread + + +@pytest.mark.timeout(10) +def test_abstract_smart_lock_subclass_detects_simple_deadlock(smartlock_class: Type[AbstractSmartLock]) -> None: # noqa: PLR0915 + """Every public AbstractSmartLock subclass breaks a two-thread deadlock with one DeadLockError and graph cleanup.""" + number_of_attempts = 50 + graph = LocksGraph() + lock_1 = smartlock_class(local_graph=graph) + lock_2 = smartlock_class(local_graph=graph) + + def acquire_locks( # noqa: PLR0913 + owned_lock: AbstractSmartLock, + requested_lock: AbstractSmartLock, + first_lock_acquired_event: Event, + request_second_lock_event: Event, + deadline: float, + result_lock: Lock, + deadlock_errors: List[DeadLockError], + unexpected_errors: List[Exception], + ) -> None: + try: + with owned_lock: + first_lock_acquired_event.set() + + if not request_second_lock_event.wait(max(deadline - monotonic(), 0.01)): + raise TimeoutError('Coordinator did not release second-lock attempt.') + + with requested_lock: + pass + except DeadLockError as error: + with result_lock: + deadlock_errors.append(error) + except Exception as error: # noqa: BLE001 + with result_lock: + unexpected_errors.append(error) + + for _ in range(number_of_attempts): + deadline = monotonic() + 2.5 + first_lock_acquired_events = [Event(), Event()] + request_second_lock_event = Event() + result_lock = Lock() + deadlock_errors: List[DeadLockError] = [] + unexpected_errors: List[Exception] = [] + + threads = [ + Thread( + target=acquire_locks, + args=(lock_1, lock_2, first_lock_acquired_events[0], request_second_lock_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + Thread( + target=acquire_locks, + args=(lock_2, lock_1, first_lock_acquired_events[1], request_second_lock_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + ] + + for thread in threads: + thread.start() + + all_threads_ready = False + try: + all_threads_ready = all(event.wait(max(deadline - monotonic(), 0.01)) for event in first_lock_acquired_events) + finally: + request_second_lock_event.set() + for thread in threads: + thread.join(1) + + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert all_threads_ready + assert all(not thread.is_alive() for thread in threads) + assert len(deadlock_errors) == 1 + assert all(not links for links in graph.links.values()) + + for lock in (lock_1, lock_2): + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + lock.acquire() + lock.release() + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + +@pytest.mark.timeout(10) +def test_abstract_smart_lock_subclass_detects_three_lock_deadlock(smartlock_class: Type[AbstractSmartLock]) -> None: + """Every public AbstractSmartLock subclass breaks a three-lock cyclic wait with two DeadLockErrors and graph cleanup.""" + number_of_attempts = 50 + graph = LocksGraph() + locks = [ + smartlock_class(local_graph=graph), + smartlock_class(local_graph=graph), + smartlock_class(local_graph=graph), + ] + + def acquire_locks( # noqa: PLR0913 + index: int, + ordered_locks: Sequence[AbstractSmartLock], + first_lock_acquired_events: Sequence[Event], + request_other_locks_event: Event, + deadline: float, + result_lock: Lock, + deadlock_errors: List[DeadLockError], + unexpected_errors: List[Exception], + ) -> None: + try: + with ordered_locks[0]: + first_lock_acquired_events[index].set() + + if not request_other_locks_event.wait(max(deadline - monotonic(), 0.01)): + raise TimeoutError('Coordinator did not release remaining lock attempts.') + + with ordered_locks[1], ordered_locks[2]: + pass + except DeadLockError as error: + with result_lock: + deadlock_errors.append(error) + except Exception as error: # noqa: BLE001 + with result_lock: + unexpected_errors.append(error) + + for _ in range(number_of_attempts): + deadline = monotonic() + 2.5 + result_lock = Lock() + first_lock_acquired_events = [Event(), Event(), Event()] + request_other_locks_event = Event() + deadlock_errors: List[DeadLockError] = [] + unexpected_errors: List[Exception] = [] + + threads = [ + Thread( + target=acquire_locks, + args=(0, (locks[0], locks[1], locks[2]), first_lock_acquired_events, request_other_locks_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + Thread( + target=acquire_locks, + args=(1, (locks[1], locks[2], locks[0]), first_lock_acquired_events, request_other_locks_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + Thread( + target=acquire_locks, + args=(2, (locks[2], locks[0], locks[1]), first_lock_acquired_events, request_other_locks_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + ] + + for thread in threads: + thread.start() + + all_threads_ready = False + try: + all_threads_ready = all(event.wait(max(deadline - monotonic(), 0.01)) for event in first_lock_acquired_events) + finally: + request_other_locks_event.set() + for thread in threads: + thread.join(1) + + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert all_threads_ready + assert all(not thread.is_alive() for thread in threads) + assert len(deadlock_errors) == 2 + assert all(not links for links in graph.links.values()) + + for lock in locks: + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + lock.acquire() + lock.release() + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + +@pytest.mark.timeout(10) +def test_abstract_smart_lock_subclass_default_instances_share_deadlock_graph(smartlock_class: Type[AbstractSmartLock]) -> None: # noqa: PLR0915 + """Default-constructed public AbstractSmartLock subclasses share the graph used for deadlock detection.""" + number_of_attempts = 50 + + def clear_shared_graph() -> None: + with shared_graph.lock: + shared_graph.links.clear() + + def acquire_locks( # noqa: PLR0913 + owned_lock: AbstractSmartLock, + requested_lock: AbstractSmartLock, + first_lock_acquired_event: Event, + request_second_lock_event: Event, + deadline: float, + result_lock: Lock, + deadlock_errors: List[DeadLockError], + unexpected_errors: List[Exception], + ) -> None: + owns_first_lock = False + owns_second_lock = False + + try: + owned_lock.acquire() + owns_first_lock = True + first_lock_acquired_event.set() + + if not request_second_lock_event.wait(max(deadline - monotonic(), 0.01)): + raise TimeoutError('Coordinator did not release second-lock attempt.') + + requested_lock.acquire() + owns_second_lock = True + except DeadLockError as error: + with result_lock: + deadlock_errors.append(error) + except Exception as error: # noqa: BLE001 + with result_lock: + unexpected_errors.append(error) + finally: + try: + if owns_second_lock: + requested_lock.release() + if owns_first_lock: + owned_lock.release() + except Exception as error: # noqa: BLE001 + with result_lock: + unexpected_errors.append(error) + + clear_shared_graph() + try: + for _ in range(number_of_attempts): + lock_1 = smartlock_class() + lock_2 = smartlock_class() + deadline = monotonic() + 2.5 + first_lock_acquired_events = [Event(), Event()] + request_second_lock_event = Event() + result_lock = Lock() + deadlock_errors: List[DeadLockError] = [] + unexpected_errors: List[Exception] = [] + + threads = [ + Thread( + target=acquire_locks, + args=(lock_1, lock_2, first_lock_acquired_events[0], request_second_lock_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + Thread( + target=acquire_locks, + args=(lock_2, lock_1, first_lock_acquired_events[1], request_second_lock_event, deadline, result_lock, deadlock_errors, unexpected_errors), + daemon=True, + ), + ] + + for thread in threads: + thread.start() + + all_threads_ready = False + try: + all_threads_ready = all(event.wait(max(deadline - monotonic(), 0.01)) for event in first_lock_acquired_events) + finally: + request_second_lock_event.set() + for thread in threads: + thread.join(1) + + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert all_threads_ready + assert all(not thread.is_alive() for thread in threads) + assert len(deadlock_errors) == 1 + + for lock in (lock_1, lock_2): + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + lock.acquire() + lock.release() + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + finally: + clear_shared_graph() + + +def test_abstract_smart_lock_subclass_context_manager_blocks_other_threads_until_exit(smartlock_class: Type[AbstractSmartLock]) -> None: + """Every public AbstractSmartLock subclass holds the lock until context-manager exit.""" + graph = LocksGraph() + lock = smartlock_class(local_graph=graph) + attempting_event = Event() + entered_event = Event() + unexpected_errors: List[Exception] = [] + + def enter_lock() -> None: + try: + attempting_event.set() + with lock: + entered_event.set() + except Exception as error: # noqa: BLE001 + unexpected_errors.append(error) + + try: + with lock: + thread = Thread(target=enter_lock, daemon=True) + thread.start() + + if not attempting_event.wait(1): + thread.join(1) + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + raise AssertionError(f'Worker did not attempt to enter lock. unexpected_errors={unexpected_errors!r}') + + assert not entered_event.wait(0.1) + finally: + thread.join(1) + + assert not thread.is_alive() + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert entered_event.is_set() + + +def test_abstract_smart_lock_subclass_release_from_non_owner_thread_raises(smartlock_class: Type[AbstractSmartLock]) -> None: + """Every public AbstractSmartLock subclass rejects release from a non-owner thread.""" + lock = smartlock_class() + release_error_messages: List[str] = [] + unexpected_errors: List[Exception] = [] + + def release_without_ownership() -> None: + try: + lock.release() + except RuntimeError as error: + release_error_messages.append(str(error)) + except Exception as error: # noqa: BLE001 + unexpected_errors.append(error) + + lock.acquire() + + thread = Thread(target=release_without_ownership, daemon=True) + try: + thread.start() + thread.join(1) + + assert not thread.is_alive() + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert release_error_messages == ['Release unlocked lock.'] + finally: + lock.release() diff --git a/tests/units/locks/smart_lock/test_lock.py b/tests/units/locks/smart_lock/test_lock.py index 16c7a03..dea5506 100644 --- a/tests/units/locks/smart_lock/test_lock.py +++ b/tests/units/locks/smart_lock/test_lock.py @@ -1,209 +1,168 @@ -from queue import Queue -from threading import Lock, Thread -from time import sleep +from threading import Event, Lock, Thread, get_native_id +from time import monotonic +from typing import List, Type import pytest from full_match import match -from locklib import DeadLockError, SmartLock +from locklib import DeadLockError, SmartLock, SmartRLock from locklib.locks.smart_lock.abstract import AbstractSmartLock +from locklib.locks.smart_lock.graph import LocksGraph -def test_smart_lock_is_abstract_smart_lock_subclass(): - """SmartLock keeps AbstractSmartLock as its base implementation.""" - assert issubclass(SmartLock, AbstractSmartLock) - - -def test_release_unlocked(): - """Releasing a fresh SmartLock raises RuntimeError with the exact unlocked-lock message.""" +@pytest.mark.timeout(5) +def test_smart_lock_raises_on_recursive_acquire_instead_of_hanging() -> None: + """SmartLock rejects recursive acquire without hanging, remains reusable, and rejects recursion again.""" lock = SmartLock() + unexpected_errors: List[Exception] = [] - with pytest.raises(RuntimeError, match=match('Release unlocked lock.')): - lock.release() - - -def test_normal_using(): - """ - A single SmartLock supports normal contended context-manager use. - - Several threads increment shared state under the lock, and the final count must include every increment. - """ - number_of_threads = 5 - number_of_attempts_per_thread = 100000 - - lock = SmartLock() - index = 0 + def verify_recursive_acquire_rejections() -> None: + try: + thread_id = get_native_id() + expected_message = f'A cycle between {thread_id}th and {thread_id}th threads has been detected.' - def function() -> None: - nonlocal index + for _ in range(2): + lock.acquire() - for _ in range(number_of_attempts_per_thread): - with lock: - index += 1 + with pytest.raises(DeadLockError, match=match(expected_message)): + lock.acquire() - threads = [Thread(target=function) for _ in range(number_of_threads)] + lock.release() + except Exception as error: # noqa: BLE001 + unexpected_errors.append(error) - for thread in threads: - thread.start() - for thread in threads: - thread.join() + thread = Thread(target=verify_recursive_acquire_rejections, daemon=True) + thread.start() + thread.join(2) - assert index == number_of_threads * number_of_attempts_per_thread + assert not thread.is_alive() + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] @pytest.mark.timeout(5) -def test_raise_when_simple_deadlock(): - """ - SmartLock detects a two-thread, two-lock deadlock instead of blocking. - - Each iteration runs opposite acquisition orders. Worker handlers collect unexpected exceptions so they fail the main test, and exactly one DeadLockError must be enough to let both threads finish. - """ - number_of_attempts = 50 - - lock_1 = SmartLock() - lock_2 = SmartLock() - - for _ in range(number_of_attempts): - flag = False - deadlock_errors = [] - unexpected_errors = [] - result_lock = Lock() - - def function_1(deadlock_errors=deadlock_errors, unexpected_errors=unexpected_errors, result_lock=result_lock): - nonlocal flag +def test_smart_lock_recursive_acquire_error_preserves_lock_and_graph_state() -> None: + """SmartLock recursive-acquire errors preserve reusable lock and graph state.""" + graph = LocksGraph() + lock = SmartLock(local_graph=graph) + unexpected_errors: List[Exception] = [] + + def exercise_recursive_acquire_recovery() -> None: + try: + thread_id = get_native_id() + expected_message = f'A cycle between {thread_id}th and {thread_id}th threads has been detected.' + + lock.acquire() + + with pytest.raises(DeadLockError, match=match(expected_message)): + lock.acquire() + + lock.release() + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + lock.acquire() + lock.release() + except Exception as error: # noqa: BLE001 + unexpected_errors.append(error) + + thread = Thread(target=exercise_recursive_acquire_recovery, daemon=True) + thread.start() + thread.join(2) + + assert not thread.is_alive() + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert not graph.links + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + +@pytest.mark.parametrize( + ('first_lock_class', 'second_lock_class'), + [ + (SmartLock, SmartRLock), + (SmartRLock, SmartLock), + ], +) +@pytest.mark.timeout(10) +def test_mixed_smart_lock_and_smart_rlock_detect_deadlock(first_lock_class: Type[AbstractSmartLock], second_lock_class: Type[AbstractSmartLock]) -> None: # noqa: PLR0915 + """SmartLock and SmartRLock share an explicit local graph and detect a two-lock deadlock in either order.""" + graph = LocksGraph() + lock_1 = first_lock_class(local_graph=graph) + lock_2 = second_lock_class(local_graph=graph) + deadline = monotonic() + 2.5 + first_lock_acquired_events = [Event(), Event()] + request_second_lock_event = Event() + result_lock = Lock() + deadlock_errors: List[DeadLockError] = [] + unexpected_errors: List[Exception] = [] + + def acquire_locks(owned_lock: AbstractSmartLock, requested_lock: AbstractSmartLock, first_lock_acquired_event: Event) -> None: + owns_first_lock = False + owns_second_lock = False + + try: + owned_lock.acquire() + owns_first_lock = True + first_lock_acquired_event.set() + + if not request_second_lock_event.wait(max(deadline - monotonic(), 0.01)): + raise TimeoutError('Coordinator did not release second-lock attempt.') + + requested_lock.acquire() + owns_second_lock = True + except DeadLockError as error: + with result_lock: + deadlock_errors.append(error) + except Exception as error: # noqa: BLE001 + with result_lock: + unexpected_errors.append(error) + finally: try: - while True: - with lock_1, lock_2: - if flag: - break - except DeadLockError as error: - with result_lock: - flag = True - deadlock_errors.append(error) - except Exception as error: # noqa: BLE001 - with result_lock: - flag = True - unexpected_errors.append(error) - - def function_2(deadlock_errors=deadlock_errors, unexpected_errors=unexpected_errors, result_lock=result_lock): - nonlocal flag - try: - while True: - with lock_2, lock_1: - if flag: - break - except DeadLockError as error: - with result_lock: - flag = True - deadlock_errors.append(error) + if owns_second_lock: + requested_lock.release() + if owns_first_lock: + owned_lock.release() except Exception as error: # noqa: BLE001 with result_lock: - flag = True unexpected_errors.append(error) - thread_1 = Thread(target=function_1) - thread_2 = Thread(target=function_2) - thread_1.start() - thread_2.start() - - thread_1.join() - thread_2.join() - - assert not unexpected_errors - assert len(deadlock_errors) == 1 + threads = [ + Thread(target=acquire_locks, args=(lock_1, lock_2, first_lock_acquired_events[0]), daemon=True), + Thread(target=acquire_locks, args=(lock_2, lock_1, first_lock_acquired_events[1]), daemon=True), + ] + for thread in threads: + thread.start() -@pytest.mark.timeout(5) -def test_raise_when_not_so_simple_deadlock(): # noqa: PLR0915 - """ - SmartLock reports a three-lock cyclic wait instead of hanging. - - Each attempt starts three threads with rotated lock order; two DeadLockError signals are enough to break the cycle and let all threads finish. - """ - number_of_attempts = 50 - - lock_1 = SmartLock() - lock_2 = SmartLock() - lock_3 = SmartLock() - - queue = Queue() - - for _ in range(number_of_attempts): - flag = False - cycles = 0 - lock = Lock() - def function_1(): - nonlocal flag - nonlocal cycles - try: - while True: - with lock_1: - sleep(0.0001) - with lock_2: - sleep(0.0001) - with lock_3: - if flag: - break - except DeadLockError: - with lock: # noqa: B023 - cycles += 1 - if cycles == 2: - flag = True - queue.put(True) - - def function_2(): - nonlocal flag - nonlocal cycles - try: - while True: - with lock_2: - sleep(0.0001) - with lock_3: - sleep(0.0001) - with lock_1: - if flag: - break - except DeadLockError: - with lock: # noqa: B023 - cycles += 1 - if cycles == 2: - flag = True - queue.put(True) - - def function_3(): - nonlocal flag - nonlocal cycles - try: - while True: - with lock_3: - sleep(0.0001) - with lock_1: - sleep(0.0001) - with lock_2: - if flag: - break - except DeadLockError: - with lock: # noqa: B023 - cycles += 1 - if cycles == 2: - flag = True - queue.put(True) - - thread_1 = Thread(target=function_1) - thread_2 = Thread(target=function_2) - thread_3 = Thread(target=function_3) - thread_1.start() - thread_2.start() - thread_3.start() - - thread_1.join() - thread_2.join() - thread_3.join() - - counter = 0 - - for _ in range(2): - queue.get() - counter += 1 - - assert counter == 2 + all_threads_ready = False + try: + all_threads_ready = all(event.wait(max(deadline - monotonic(), 0.01)) for event in first_lock_acquired_events) + finally: + request_second_lock_event.set() + for thread in threads: + thread.join(1) + + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert all_threads_ready + assert all(not thread.is_alive() for thread in threads) + assert len(deadlock_errors) == 1 + assert all(not links for links in graph.links.values()) + + for lock in (lock_1, lock_2): + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + lock.acquire() + lock.release() + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths diff --git a/tests/units/locks/smart_lock/test_rlock.py b/tests/units/locks/smart_lock/test_rlock.py new file mode 100644 index 0000000..32f98d8 --- /dev/null +++ b/tests/units/locks/smart_lock/test_rlock.py @@ -0,0 +1,174 @@ +from threading import Event, Thread, get_native_id +from typing import List + +import pytest +from full_match import match + +from locklib import SmartRLock +from locklib.locks.smart_lock.abstract import AbstractSmartLock +from locklib.locks.smart_lock.graph import LocksGraph + + +def test_smart_rlock_is_abstract_smart_lock_subclass() -> None: + """SmartRLock remains an AbstractSmartLock subclass in recursive mode.""" + assert issubclass(SmartRLock, AbstractSmartLock) + assert SmartRLock.recursive is True + + +@pytest.mark.parametrize('number_of_acquires', range(11)) +def test_smart_rlock_allows_recursive_acquire_and_requires_matching_releases(number_of_acquires: int) -> None: + """SmartRLock requires one release per acquire for depths 0 through 10, then raises unlocked-lock RuntimeError.""" + lock = SmartRLock() + + for _ in range(number_of_acquires): + lock.acquire() + + for _ in range(number_of_acquires): + lock.release() + + with pytest.raises(RuntimeError, match=match('Release unlocked lock.')): + lock.release() + + +def test_smart_rlock_nested_context_manager_keeps_lock_until_outer_exit() -> None: + """SmartRLock keeps ownership after leaving an inner recursive context manager.""" + graph = LocksGraph() + lock = SmartRLock(local_graph=graph) + attempting_event = Event() + entered_event = Event() + unexpected_errors: List[Exception] = [] + + def enter_lock() -> None: + try: + attempting_event.set() + with lock: + entered_event.set() + except Exception as error: # noqa: BLE001 + unexpected_errors.append(error) + + thread = Thread(target=enter_lock, daemon=True) + + try: + with lock: + with lock: + pass + + thread.start() + + if not attempting_event.wait(1): + thread.join(1) + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + raise AssertionError(f'Worker did not attempt to enter lock. unexpected_errors={unexpected_errors!r}') + + assert not entered_event.wait(0.1) + finally: + thread.join(1) + + assert not thread.is_alive() + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert entered_event.is_set() + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + +def test_smart_rlock_partial_release_does_not_unlock_for_other_threads() -> None: + """SmartRLock keeps ownership after a partial recursive release.""" + graph = LocksGraph() + lock = SmartRLock(local_graph=graph) + owner_thread_id = get_native_id() + + lock.acquire() + lock.acquire() + releases_left = 2 + attempting_event = Event() + entered_event = Event() + unexpected_errors: List[Exception] = [] + + def enter_lock() -> None: + try: + attempting_event.set() + with lock: + entered_event.set() + except Exception as error: # noqa: BLE001 + unexpected_errors.append(error) + + thread = Thread(target=enter_lock, daemon=True) + + try: + thread.start() + + if not attempting_event.wait(1): + thread.join(1) + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + raise AssertionError(f'Worker did not attempt to enter lock. unexpected_errors={unexpected_errors!r}') + + lock.release() + releases_left -= 1 + + with lock.lock: + assert lock.deque[-1] == owner_thread_id + assert lock.recursion_depths[owner_thread_id] == 1 + assert not entered_event.wait(0.1) + + lock.release() + releases_left -= 1 + finally: + try: + for _ in range(releases_left): + lock.release() + finally: + thread.join(1) + + assert not thread.is_alive() + if unexpected_errors: + raise AssertionError(f'Unexpected worker exceptions: {unexpected_errors!r}') from unexpected_errors[0] + assert entered_event.is_set() + + +def test_smart_rlock_recursive_acquire_does_not_touch_graph() -> None: + """SmartRLock recursive acquire leaves the wait-for graph unchanged.""" + graph = LocksGraph() + lock = SmartRLock(local_graph=graph) + + lock.acquire() + assert not graph.links + + lock.acquire() + + assert not graph.links + + lock.release() + lock.release() + assert not graph.links + + +def test_smart_rlock_recursion_depth_resets_after_full_release() -> None: + """SmartRLock clears recursive depth after fully releasing the lock.""" + lock = SmartRLock() + + for _ in range(3): + lock.acquire() + + for _ in range(3): + lock.release() + + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + lock.acquire() + lock.release() + + with lock.lock: + assert not lock.deque + assert not lock.local_locks + assert not lock.recursion_depths + + with pytest.raises(RuntimeError, match=match('Release unlocked lock.')): + lock.release() diff --git a/tests/units/protocols/test_context_lock.py b/tests/units/protocols/test_context_lock.py index b1f91b4..fa80a52 100644 --- a/tests/units/protocols/test_context_lock.py +++ b/tests/units/protocols/test_context_lock.py @@ -8,7 +8,13 @@ import pytest from full_match import match -from locklib import AsyncEmptyLock, ContextLockProtocol, EmptyLock, SmartLock +from locklib import ( + AsyncEmptyLock, + ContextLockProtocol, + EmptyLock, + SmartLock, + SmartRLock, +) @pytest.mark.parametrize( @@ -18,6 +24,7 @@ TLock(), TRLock(), SmartLock(), + SmartRLock(), EmptyLock(), ], ) @@ -95,4 +102,5 @@ def some_function(lock: ContextLockProtocol) -> ContextLockProtocol: some_function(TLock()) some_function(TRLock()) some_function(SmartLock()) + some_function(SmartRLock()) some_function(EmptyLock()) diff --git a/tests/units/protocols/test_lock.py b/tests/units/protocols/test_lock.py index 31a28e4..2495d9a 100644 --- a/tests/units/protocols/test_lock.py +++ b/tests/units/protocols/test_lock.py @@ -6,7 +6,7 @@ import pytest from full_match import match -from locklib import AsyncEmptyLock, EmptyLock, LockProtocol, SmartLock +from locklib import AsyncEmptyLock, EmptyLock, LockProtocol, SmartLock, SmartRLock @pytest.mark.parametrize( @@ -17,6 +17,7 @@ TRLock(), ALock(), SmartLock(), + SmartRLock(), EmptyLock(), AsyncEmptyLock(), ], @@ -71,5 +72,6 @@ def some_function(lock: LockProtocol) -> LockProtocol: some_function(TRLock()) some_function(ALock()) some_function(SmartLock()) + some_function(SmartRLock()) some_function(EmptyLock()) some_function(AsyncEmptyLock())