> 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(…) ``` In the case of Django <https://github.com/django/django/blob/77aa74cb70dd85497dbade6bc0f394aa41e88c94/django/forms/renderers.py#L19>, the “something that’s not present yet” is the Django settings: ``` @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 29 Apr 2020, at 23:04, Raymond Hettinger <raymond.hettin...@gmail.com> > wrote: > > > >> On Apr 29, 2020, at 11:15 AM, Tom Forbes <t...@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 > > > > > > > > > > >
_______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-le...@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/V6ISSAG6NNMB5SZSGOCUHBBJOOG3Y3WH/ Code of Conduct: http://python.org/psf/codeofconduct/