Risks: Any reentrancy or recursion will result in deadlock.
Reentrancy is a great point that I didn’t consider. I would say that as the intended use case for this is returning lazy singletons of some kind then reentrant calls would be a bug that would end with a recursion error - which is infinitely better and more debuggable than a deadlock. So yes, this should be a `RLock` instead.
Limitations: No instrumentation. No ability to reset or clear. Won't work across multiple processes.
I get the feeling that you might be about to make the point that this is somewhat re-inventing the `lru_cache` wheel, and I honestly agree with you. Given that the `lru_cache()` overhead with no arguments is so small, would you accept a PR that defaults `maxsize` to `None` if the wrapped function accepts no arguments and a change to the documentation showing that you could use `lru_cache` as a replacement for `call_once`?
Or, we could add the ability to clear and add stats pretty easily. Ugly implementation:
```
def once(func):
sentinel = object()
cache = sentinel
lock = Lock()
hit_count = 0
@functools.wraps(func)
def _wrapper():
nonlocal cache, lock, sentinel
if cache is sentinel:
with lock:
if cache is sentinel:
cache = func()
else:
hit_count += 1
else:
hit_count += 1
return cache
def clear():
cache = sentinel
def stats():
return hit_count
_wrapper.clear = clear
_wrapper.stats = stats
return _wrapper
```
It would be nice to look at some compelling use cases. Off hand, I can't think of time when I would have used this decorator. Also, I have a nagging worry that holding a non-reentrant lock across an arbitrary user defined function call is recipe for deadlocks. That's why during code reviews we typically check every single use of Lock() to see if it should have been an RLock(), especially in big systems where GC, __del__, or weakref callbacks can trigger running any code at just about any time.
There are two specific use cases that I’ve seen:
1. You want a module-level singleton that creates an object that depends on something that’s not present yet. In it’s simplest form that might involve circular dependencies:
```
@call_once()
def create_client():
# some_module imports the outer module
from some_module import something
something(…)
```
```
@functools.lru_cache()
def get_default_renderer():
renderer_class = import_string(settings.FORM_RENDERER)
return renderer_class()
```
We don’t want to needlessly re-create the default form renderer, and we cannot create this instance at the module level.
2. You ant to create a singleton instance that is lazily created, can be easily mocked. For example, a `redis` client:
```
@call_once()
def get_redis_connection():
return redis.Redis(host='localhost', port=6379, db=0)
```
You don’t want to create this at the module level as the class might well start creating connections to Redis when it’s instantiated. On top of that other modules can do `from redis import get_redis_connection`, rather than `from redis import redis_connection`, which is easier and more consistent to mock.
On Apr 29, 2020, at 11:15 AM, Tom Forbes <tom@tomforb.es> wrote:
What exactly would the issue be with this:
```
import functools
from threading import Lock
def once(func):
sentinel = object()
cache = sentinel
lock = Lock()
@functools.wraps(func)
def _wrapper():
nonlocal cache, lock, sentinel
if cache is sentinel:
with lock:
if cache is sentinel:
cache = func()
return cache
return _wrapper
```
This recipe is the best variant so far and gives us something concrete to talk about :-)
Benefits: Guarantees the wrapped function is not called more than once.
Restrictions: Only works with zero argument functions.
Risks: Any reentrancy or recursion will result in deadlock.
Limitations: No instrumentation. No ability to reset or clear. Won't work across multiple processes.
It would be nice to look at some compelling use cases. Off hand, I can't think of time when I would have used this decorator. Also, I have a nagging worry that holding a non-reentrant lock across an arbitrary user defined function call is recipe for deadlocks. That's why during code reviews we typically check every single use of Lock() to see if it should have been an RLock(), especially in big systems where GC, __del__, or weakref callbacks can trigger running any code at just about any time.
Raymond