5 ms·

dict.setdefault doesn’t solve the problem that he’s using the lock for (atomicity is not the problem).

mgraczyk 6y ago
Something like this should work, no? Only run the coroutine if you won the race. D = {} async def maybe_initialize(): global D this_setup = asyncio.ensure_future(one_time_setup()) actual_setup = D.setdefault('initializer', this_setup) if this_setup is not actual_setup: this_setup.cancel() await actual_setup return complete example: https://gist.github.com/mgraczyk/e251443bccfe54505e75b6526550c113 https://gist.github.com/mgraczyk/e251443bccfe54505e75b652655...
- sicromoft 6y ago
  I think something might be off in your mental model of python coroutines. They work via cooperative multitasking. Atomic operations aren't really necessary or relevant in such a system where you are in complete control of all yield points (i.e., calls to `await`). If you read the full blog post, the author solves the problem without setdefault, and without having to cancel any futures.
  - mgraczyk 6y ago
    I understand them quite well, thank you. You don't need setdefault if you only call the initializer from one thread. My code can be made thread safe (as long as you only use one event loop per thread). It's way easier with a single thread. You don't need a lock or atomic operations. initializer = None async def maybe_initialize(): global initializer if initializer is None: initializer = asyncio.ensure_future(one_time_setup()) await initializer return
    - jwilk 6y ago
      This is the same as the solution from the article, except that: - Chris forgot to declare the variable as global; - Chris used asyncio.create_task() instead of asyncio.ensure_future().
      - mgraczyk 6y ago
        Thanks for pointing that out, for some reason I had thought the last implementation also used a lock.
      - andreareina 6y ago
        Maybe it's me not grokking async, but the code seems right to me. `once` is a decorator, so `future` is "instantiated" once per decorated function, which is then accessed as nonlocal in the wrapper. So in the last code sample, every call to `one_time_setup()` is accessing the same `future`.
      - benji-york 6y ago
        Since D is not being rebound, the global declaration is unneeded.
      - jwilk 6y ago
        (Chris has added the global declaration since then.)
  - heavenlyblue 6y ago
    You have to make it clear you’re speaking of CPU atomics, because whatever atomic operations are trying to solve, for example mainly “read then write” operations - may still easily cause issues within asynchronous code.
- deleted 6y ago
  [deleted]
- benji-york 6y ago
  A small point: since you're not rebinding D, you don't need the "global" declaration.
vii 6y ago
exactly - The problem is that there are three logical states that need three different actions 1. if nothing done yet: start async computation 2. if async computation in progress: wait for it 3. return result as computaton done Atomicity in the sense that no other Python code can run during a setdefault, is not relevant. Waiting for something that is running in another loop actually isn't easy either; I'm not sure that Python really allows it - won't you get an error: The future belongs to a different loop than the one specified as the loop argument
- vii 6y ago
  This code creates that error by trying to wait in two threads in two loops. If the future is finished, it is fine to wait for it. Having multiple loops waiting for each other is pretty complex to debug. import asyncio import threading future = None def p(msg): print(f"{msg}: {threading.get_ident()}, event loop: {id(asyncio.get_running_loop())}") async def one_time_setup(): p('one_time_setup start') await asyncio.sleep(1) p('one_time_setup done') async def maybe_initialize(): global future if not future: future = asyncio.create_task(one_time_setup()) p('waiting start') await future p('waiting done') def worker(): new_loop = asyncio.new_event_loop() asyncio.set_event_loop(new_loop) new_loop.run_until_complete(maybe_initialize()) number_of_threads = 2 for _ in range(number_of_threads): threading.Thread(target=worker).start() this prints RuntimeError: Task <Task pending coro=<maybe_initialize() running at test.py:20> cb=[_run_until_complete_cb() at /Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py:158]> got Future <Task pending coro=<one_time_setup() running at test.py:11> cb=[<TaskWakeupMethWrapper object at 0x10cafdeb8>()]> attached to a different loop
- mgraczyk 6y ago
  You can do this in python using `Future` or `Task`. For example, with `asyncio.ensure_future`.

Something like this should work, no? Only run the coroutine if you won the race. D = {} async def maybe_initialize(): global D this_setup = asyncio.ensure_future(one_time_setup()) actual_setup = D.setdefault('initializer', this_setup) if this_setup is not actual_setup: this_setup.cancel() await actual_setup return complete example: https://gist.github.com/mgraczyk/e251443bccfe54505e75b6526550c113 https://gist.github.com/mgraczyk/e251443bccfe54505e75b652655...

sicromoft 6y ago
I think something might be off in your mental model of python coroutines. They work via cooperative multitasking. Atomic operations aren't really necessary or relevant in such a system where you are in complete control of all yield points (i.e., calls to `await`). If you read the full blog post, the author solves the problem without setdefault, and without having to cancel any futures.
- mgraczyk 6y ago
  I understand them quite well, thank you. You don't need setdefault if you only call the initializer from one thread. My code can be made thread safe (as long as you only use one event loop per thread). It's way easier with a single thread. You don't need a lock or atomic operations. initializer = None async def maybe_initialize(): global initializer if initializer is None: initializer = asyncio.ensure_future(one_time_setup()) await initializer return
  - jwilk 6y ago
    This is the same as the solution from the article, except that: - Chris forgot to declare the variable as global; - Chris used asyncio.create_task() instead of asyncio.ensure_future().
    - mgraczyk 6y ago
      Thanks for pointing that out, for some reason I had thought the last implementation also used a lock.
    - andreareina 6y ago
      Maybe it's me not grokking async, but the code seems right to me. `once` is a decorator, so `future` is "instantiated" once per decorated function, which is then accessed as nonlocal in the wrapper. So in the last code sample, every call to `one_time_setup()` is accessing the same `future`.
    - benji-york 6y ago
      Since D is not being rebound, the global declaration is unneeded.
    - jwilk 6y ago
      (Chris has added the global declaration since then.)
- heavenlyblue 6y ago
  You have to make it clear you’re speaking of CPU atomics, because whatever atomic operations are trying to solve, for example mainly “read then write” operations - may still easily cause issues within asynchronous code.
deleted 6y ago
[deleted]
benji-york 6y ago
A small point: since you're not rebinding D, you don't need the "global" declaration.

vii 6y ago

exactly - The problem is that there are three logical states that need three different actions 1. if nothing done yet: start async computation 2. if async computation in progress: wait for it 3. return result as computaton done Atomicity in the sense that no other Python code can run during a setdefault, is not relevant. Waiting for something that is running in another loop actually isn't easy either; I'm not sure that Python really allows it - won't you get an error: The future belongs to a different loop than the one specified as the loop argument

vii 6y ago
This code creates that error by trying to wait in two threads in two loops. If the future is finished, it is fine to wait for it. Having multiple loops waiting for each other is pretty complex to debug. import asyncio import threading future = None def p(msg): print(f"{msg}: {threading.get_ident()}, event loop: {id(asyncio.get_running_loop())}") async def one_time_setup(): p('one_time_setup start') await asyncio.sleep(1) p('one_time_setup done') async def maybe_initialize(): global future if not future: future = asyncio.create_task(one_time_setup()) p('waiting start') await future p('waiting done') def worker(): new_loop = asyncio.new_event_loop() asyncio.set_event_loop(new_loop) new_loop.run_until_complete(maybe_initialize()) number_of_threads = 2 for _ in range(number_of_threads): threading.Thread(target=worker).start() this prints RuntimeError: Task <Task pending coro=<maybe_initialize() running at test.py:20> cb=[_run_until_complete_cb() at /Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py:158]> got Future <Task pending coro=<one_time_setup() running at test.py:11> cb=[<TaskWakeupMethWrapper object at 0x10cafdeb8>()]> attached to a different loop
mgraczyk 6y ago
You can do this in python using `Future` or `Task`. For example, with `asyncio.ensure_future`.