Skip to content

Commit

Permalink
Fix an Injector.get() thread safety regression (#215)
Browse files Browse the repository at this point in the history
One of the thread safety tests has been failing on various PyPy
versions[1]:

    _____________ TestThreadSafety.test_singleton_scope_is_thread_safe _____________
    Traceback (most recent call last):
      File "/home/runner/work/injector/injector/injector_test.py", line 863, in test_singleton_scope_is_thread_safe
        assert a is b
    AssertionError: assert <injector_test.TestThreadSafety.setup.<locals>.XXX object at 0x0000000006b80d08> is <injector_test.TestThreadSafety.setup.<locals>.XXX object at 0x0000000006b810c0>

SingletonScope's get() was protected with a lock already but Injector.get()
wasn't and there are things in there that are totally likely to produce
invalid results if executed from multiple contexts concurrently.

Rather than try to nail down the exact issue that caused this, figure
out why it's been failing only on PyPy etc. I figured it's ok to go for
the big hammer here and protect the whole method with the lock. This way
we don't have to wonder about the details that can be *really* difficult
to figure out.

I don't think this is gonna make anything meaningfully slower –
ClassProvider and CallableProvider are already locked (indirecty,
through Injector.args_to_inject() being synchronized) and I expect
these to be involved in vast majority of the injection work.

[1] https://github.com/python-injector/injector/actions/runs/4156039132/jobs/7189544034
  • Loading branch information
jstasiak committed Feb 12, 2023
1 parent 90aa2cc commit 87826b3
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions injector/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,7 @@ def __init__(
def _log_prefix(self) -> str:
return '>' * (len(self._stack) + 1) + ' '

@synchronized(lock)
def get(self, interface: Type[T], scope: Union[ScopeDecorator, Type[Scope], None] = None) -> T:
"""Get an instance of the given interface.
Expand Down

0 comments on commit 87826b3

Please sign in to comment.