fix: Share one lock when defining scopes

injection/_core/common/threading.py

@@ -2,22 +2,21 @@
 
 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(
     int(getenv("PYTHON_INJECTION_THREADSAFE", "0"))
 )
 
 
-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()

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

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:

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)

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

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"),
     [