diff --git a/injection/_core/common/threading.py b/injection/_core/common/threading.py index 109e000..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( @@ -10,14 +10,13 @@ ) -def get_lock( +def lock_if_threadsafe( + lock: AbstractContextManager[Any] | None = None, 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 or Lock() 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 79632d0..1531914 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 Lock from types import EllipsisType, TracebackType from typing import ( TYPE_CHECKING, @@ -29,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, @@ -38,6 +39,8 @@ ScopeUndefinedError, ) +__thread_lock: Final[Lock] = Lock() + 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 = lock_if_threadsafe(__thread_lock, threadsafe) with lock: if get_first_scope(name, default=None): @@ -209,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 0f61e2d..5928d38 100644 --- a/tests/_core/common/test_threading.py +++ b/tests/_core/common/test_threading.py @@ -1,18 +1,25 @@ 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_lock_if_threadsafe_with_threadsafe_without_lock_returns_new_non_reentrant_lock() -> ( + None +): + lock = lock_if_threadsafe(threadsafe=True) + with lock: + assert lock.acquire(blocking=False) is False -def test_get_lock_without_threadsafe_returns_null_context() -> None: - assert isinstance(get_lock(threadsafe=False, reuse=RLock()), nullcontext) + assert lock_if_threadsafe(threadsafe=True) is not lock + + +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 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"), [