From 1441e26fae5c34774bc443442f085a66ce970675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Mandart?= Date: Wed, 21 Oct 2026 14:50:03 +0200 Subject: [PATCH 1/4] fix: Share one lock when defining scopes Each define_scope call created its own lock, so checking whether a scope was already defined and binding it wasn't atomic across threads: two threads could define the same shared scope, the second overwriting the first. Co-Authored-By: Claude Opus 5.5 (1M context) --- injection/_core/scope.py | 5 ++++- tests/_core/test_scope.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/injection/_core/scope.py b/injection/_core/scope.py index 79632d0..020d972 100644 --- a/injection/_core/scope.py +++ b/injection/_core/scope.py @@ -16,6 +16,7 @@ from dataclasses import dataclass, field from enum import StrEnum from functools import partial +from threading import RLock from types import EllipsisType, TracebackType from typing import ( TYPE_CHECKING, @@ -38,6 +39,8 @@ ScopeUndefinedError, ) +__scope_lock: Final[RLock] = RLock() + class ScopeKind(StrEnum): CONTEXTUAL = "contextual" @@ -197,7 +200,7 @@ def _bind_scope( kind: ScopeKind | ScopeKindStr, threadsafe: bool | None, ) -> Iterator[ScopeFacade]: - lock = get_lock(threadsafe) + lock = get_lock(threadsafe, reuse=__scope_lock) with lock: if get_first_scope(name, default=None): diff --git a/tests/_core/test_scope.py b/tests/_core/test_scope.py index a3aba8e..b17059a 100644 --- a/tests/_core/test_scope.py +++ b/tests/_core/test_scope.py @@ -1,4 +1,8 @@ +import sys +from collections.abc import Iterator +from concurrent.futures import ThreadPoolExecutor from contextvars import Context +from threading import Barrier import pytest @@ -12,6 +16,27 @@ ) +@pytest.fixture +def fast_thread_switching() -> Iterator[None]: + interval = sys.getswitchinterval() + sys.setswitchinterval(1e-6) + yield + sys.setswitchinterval(interval) + + +def _define_shared_request_scope(barrier: Barrier) -> bool: + barrier.wait() + + try: + with define_scope("request", kind="shared", threadsafe=True): + barrier.wait() + except ScopeAlreadyDefinedError: + barrier.wait() + return False + + return True + + def _is_request_scope_defined() -> bool: return get_first_scope("request", default=None) is not None @@ -22,6 +47,20 @@ def test_define_scope_with_defined_scope_raises_scope_already_defined_error() -> pass +@pytest.mark.usefixtures("fast_thread_switching") +def test_define_scope_with_concurrent_definitions_raises_scope_already_defined_error() -> ( + None +): + with ThreadPoolExecutor(2) as executor: + for _ in range(5000): + barrier = Barrier(2) + futures = [ + executor.submit(_define_shared_request_scope, barrier) for _ in range(2) + ] + + assert sorted(future.result() for future in futures) == [False, True] + + @pytest.mark.parametrize( ("kind", "expected"), [ From a7ec562ae30d0b748f28c15f6d237e813d667f24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Mandart?= Date: Wed, 21 Oct 2026 14:50:30 +0200 Subject: [PATCH 2/4] refactor: Replace get_lock with lock_if_threadsafe Every caller now passes an existing lock, so the function no longer creates one: it returns the given lock when threadsafe, otherwise a null context. InjectMetadata receives its lock explicitly. Co-Authored-By: Claude Opus 5.5 (1M context) --- injection/_core/common/threading.py | 11 +++-------- injection/_core/module.py | 13 ++++++------- injection/_core/scope.py | 4 ++-- tests/_core/common/test_threading.py | 14 +++++--------- tests/_core/test_module.py | 3 ++- 5 files changed, 18 insertions(+), 27 deletions(-) diff --git a/injection/_core/common/threading.py b/injection/_core/common/threading.py index 109e000..1ec94a7 100644 --- a/injection/_core/common/threading.py +++ b/injection/_core/common/threading.py @@ -10,14 +10,9 @@ ) -def get_lock( +def lock_if_threadsafe( + lock: RLock, threadsafe: bool | None = None, - *, - reuse: RLock | None = None, ) -> AbstractContextManager[Any]: threadsafe = _PYTHON_INJECTION_THREADSAFE if threadsafe is None else threadsafe - - if not threadsafe: - return nullcontext() - - return reuse or RLock() + return lock if threadsafe else nullcontext() diff --git a/injection/_core/module.py b/injection/_core/module.py index f326fdc..7d72e22 100644 --- a/injection/_core/module.py +++ b/injection/_core/module.py @@ -45,7 +45,7 @@ from injection._core.common.event import Event, EventChannel, EventListener from injection._core.common.invertible import Invertible, SimpleInvertible from injection._core.common.lazy import Lazy -from injection._core.common.threading import get_lock +from injection._core.common.threading import lock_if_threadsafe from injection._core.common.type import ( InputType, TypeInfo, @@ -356,7 +356,7 @@ def create_metadata[**P, T]( /, threadsafe: bool | None = None, ) -> InjectMetadata[P, T]: - return InjectMetadata(wrapped, threadsafe, lock=self.__lock).listen(self) + return InjectMetadata(wrapped, self.__lock, threadsafe).listen(self) if TYPE_CHECKING: # pragma: no cover @@ -403,7 +403,7 @@ async def afind_instance[T]( *, threadsafe: bool | None = None, ) -> T: - with get_lock(threadsafe, reuse=self.__lock): + with lock_if_threadsafe(self.__lock, threadsafe): injectable = self[cls] return await injectable.aget_instance() @@ -413,7 +413,7 @@ def find_instance[T]( *, threadsafe: bool | None = None, ) -> T: - with get_lock(threadsafe, reuse=self.__lock): + with lock_if_threadsafe(self.__lock, threadsafe): injectable = self[cls] return injectable.get_instance() @@ -840,13 +840,12 @@ class InjectMetadata[**P, T](Caller[P, T], EventListener): def __init__( self, wrapped: Callable[P, T], + lock: RLock, /, threadsafe: bool | None = None, - *, - lock: RLock | None = None, ) -> None: self.__dependencies = Dependencies.empty() - self.__lock = get_lock(threadsafe, reuse=lock) + self.__lock = lock_if_threadsafe(lock, threadsafe) self.__owner = None self.__tasks = deque() self.__wrapped = wrapped diff --git a/injection/_core/scope.py b/injection/_core/scope.py index 020d972..203ac2b 100644 --- a/injection/_core/scope.py +++ b/injection/_core/scope.py @@ -30,7 +30,7 @@ runtime_checkable, ) -from injection._core.common.threading import get_lock +from injection._core.common.threading import lock_if_threadsafe from injection._core.slots import SlotKey from injection.exceptions import ( InjectionError, @@ -200,7 +200,7 @@ def _bind_scope( kind: ScopeKind | ScopeKindStr, threadsafe: bool | None, ) -> Iterator[ScopeFacade]: - lock = get_lock(threadsafe, reuse=__scope_lock) + lock = lock_if_threadsafe(__scope_lock, threadsafe) with lock: if get_first_scope(name, default=None): diff --git a/tests/_core/common/test_threading.py b/tests/_core/common/test_threading.py index 0f61e2d..58958d9 100644 --- a/tests/_core/common/test_threading.py +++ b/tests/_core/common/test_threading.py @@ -1,18 +1,14 @@ from contextlib import nullcontext from threading import RLock -from injection._core.common.threading import get_lock +from injection._core.common.threading import lock_if_threadsafe -def test_get_lock_with_threadsafe_and_reuse_returns_reused_lock() -> None: +def test_lock_if_threadsafe_with_threadsafe_returns_lock() -> None: lock = RLock() - assert get_lock(threadsafe=True, reuse=lock) is lock + assert lock_if_threadsafe(lock, threadsafe=True) is lock -def test_get_lock_with_threadsafe_without_reuse_returns_new_lock() -> None: - assert get_lock(threadsafe=True) is not get_lock(threadsafe=True) - - -def test_get_lock_without_threadsafe_returns_null_context() -> None: - assert isinstance(get_lock(threadsafe=False, reuse=RLock()), nullcontext) +def test_lock_if_threadsafe_without_threadsafe_returns_null_context() -> None: + assert isinstance(lock_if_threadsafe(RLock(), threadsafe=False), nullcontext) diff --git a/tests/_core/test_module.py b/tests/_core/test_module.py index 9a92420..7a76d60 100644 --- a/tests/_core/test_module.py +++ b/tests/_core/test_module.py @@ -3,6 +3,7 @@ from collections.abc import AsyncIterator, Callable, Iterator from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass +from threading import RLock from typing import Annotated, Any, Optional, Union import anyio @@ -1231,6 +1232,6 @@ def test_str_with_success_returns_message(self, module: Module) -> None: class TestInjectMetadata: def test_on_event_without_module_event_returns_none(self) -> None: - metadata = InjectMetadata(A) + metadata = InjectMetadata(A, RLock()) assert metadata.on_event(_Event()) is None From f54711c169c2e3970855859e90b086e40ff429b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Mandart?= Date: Wed, 21 Oct 2026 15:01:22 +0200 Subject: [PATCH 3/4] refactor: Give each user scope its own lock The scope definition lock is renamed to __thread_lock and only protects the binding of scopes; slot writes use a lock of their own again. lock_if_threadsafe now creates a lock only when none is given and the call is threadsafe. Co-Authored-By: Claude Opus 5.5 (1M context) --- injection/_core/common/threading.py | 8 ++++++-- injection/_core/scope.py | 6 +++--- tests/_core/common/test_threading.py | 8 ++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/injection/_core/common/threading.py b/injection/_core/common/threading.py index 1ec94a7..e332bc8 100644 --- a/injection/_core/common/threading.py +++ b/injection/_core/common/threading.py @@ -11,8 +11,12 @@ def lock_if_threadsafe( - lock: RLock, + lock: RLock | None = None, threadsafe: bool | None = None, ) -> AbstractContextManager[Any]: threadsafe = _PYTHON_INJECTION_THREADSAFE if threadsafe is None else threadsafe - return lock if threadsafe else nullcontext() + + if not threadsafe: + return nullcontext() + + return lock or RLock() diff --git a/injection/_core/scope.py b/injection/_core/scope.py index 203ac2b..d52822a 100644 --- a/injection/_core/scope.py +++ b/injection/_core/scope.py @@ -39,7 +39,7 @@ ScopeUndefinedError, ) -__scope_lock: Final[RLock] = RLock() +__thread_lock: Final[RLock] = RLock() class ScopeKind(StrEnum): @@ -200,7 +200,7 @@ def _bind_scope( kind: ScopeKind | ScopeKindStr, threadsafe: bool | None, ) -> Iterator[ScopeFacade]: - lock = lock_if_threadsafe(__scope_lock, threadsafe) + lock = lock_if_threadsafe(__thread_lock, threadsafe) with lock: if get_first_scope(name, default=None): @@ -212,7 +212,7 @@ def _bind_scope( stack.enter_context(__scope_resolvers[kind][name].bind(scope)) try: - yield _UserScope(scope, lock) + yield _UserScope(scope, lock_if_threadsafe(threadsafe=threadsafe)) finally: with lock: diff --git a/tests/_core/common/test_threading.py b/tests/_core/common/test_threading.py index 58958d9..e86dd57 100644 --- a/tests/_core/common/test_threading.py +++ b/tests/_core/common/test_threading.py @@ -10,5 +10,13 @@ def test_lock_if_threadsafe_with_threadsafe_returns_lock() -> None: assert lock_if_threadsafe(lock, threadsafe=True) is lock +def test_lock_if_threadsafe_with_threadsafe_without_lock_returns_new_lock() -> None: + first = lock_if_threadsafe(threadsafe=True) + second = lock_if_threadsafe(threadsafe=True) + + assert first is not second + assert not isinstance(first, nullcontext) + + def test_lock_if_threadsafe_without_threadsafe_returns_null_context() -> None: assert isinstance(lock_if_threadsafe(RLock(), threadsafe=False), nullcontext) From 92f62e9af5bc1276936115864900c1353a47ffb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Mandart?= Date: Wed, 21 Oct 2026 15:07:29 +0200 Subject: [PATCH 4/4] refactor: Use non-reentrant locks for scopes No user code runs while the scope locks are held, so they never need to be reacquired by the same thread. The module lock stays reentrant, since recipes can resolve dependencies while it is held. Co-Authored-By: Claude Opus 5.5 (1M context) --- injection/_core/common/threading.py | 6 +++--- injection/_core/scope.py | 4 ++-- tests/_core/common/test_threading.py | 13 ++++++++----- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/injection/_core/common/threading.py b/injection/_core/common/threading.py index e332bc8..06739df 100644 --- a/injection/_core/common/threading.py +++ b/injection/_core/common/threading.py @@ -2,7 +2,7 @@ from contextlib import AbstractContextManager, nullcontext from os import getenv -from threading import RLock +from threading import Lock from typing import Any, Final _PYTHON_INJECTION_THREADSAFE: Final[bool] = bool( @@ -11,7 +11,7 @@ def lock_if_threadsafe( - lock: RLock | None = None, + lock: AbstractContextManager[Any] | None = None, threadsafe: bool | None = None, ) -> AbstractContextManager[Any]: threadsafe = _PYTHON_INJECTION_THREADSAFE if threadsafe is None else threadsafe @@ -19,4 +19,4 @@ def lock_if_threadsafe( if not threadsafe: return nullcontext() - return lock or RLock() + return lock or Lock() diff --git a/injection/_core/scope.py b/injection/_core/scope.py index d52822a..1531914 100644 --- a/injection/_core/scope.py +++ b/injection/_core/scope.py @@ -16,7 +16,7 @@ from dataclasses import dataclass, field from enum import StrEnum from functools import partial -from threading import RLock +from threading import Lock from types import EllipsisType, TracebackType from typing import ( TYPE_CHECKING, @@ -39,7 +39,7 @@ ScopeUndefinedError, ) -__thread_lock: Final[RLock] = RLock() +__thread_lock: Final[Lock] = Lock() class ScopeKind(StrEnum): diff --git a/tests/_core/common/test_threading.py b/tests/_core/common/test_threading.py index e86dd57..5928d38 100644 --- a/tests/_core/common/test_threading.py +++ b/tests/_core/common/test_threading.py @@ -10,12 +10,15 @@ def test_lock_if_threadsafe_with_threadsafe_returns_lock() -> None: assert lock_if_threadsafe(lock, threadsafe=True) is lock -def test_lock_if_threadsafe_with_threadsafe_without_lock_returns_new_lock() -> None: - first = lock_if_threadsafe(threadsafe=True) - second = lock_if_threadsafe(threadsafe=True) +def test_lock_if_threadsafe_with_threadsafe_without_lock_returns_new_non_reentrant_lock() -> ( + None +): + lock = lock_if_threadsafe(threadsafe=True) - assert first is not second - assert not isinstance(first, nullcontext) + with lock: + assert lock.acquire(blocking=False) is False + + assert lock_if_threadsafe(threadsafe=True) is not lock def test_lock_if_threadsafe_without_threadsafe_returns_null_context() -> None: