Krukov / cashews

Cache with async power
MIT License
406 stars 25 forks source link

thunder_protection with multiple event loops breaks #221

Closed robert-schmidtke closed 4 months ago

robert-schmidtke commented 5 months ago

Hi, I am using cashews within the "main" event loop, as well as within worker threads with their own event loops.

The "main" event loop is from a FastAPI application (Uvicorn, uvloop), whereas the other loops I have to create manually and run them in dedicated threads. This is so I can use the async-only cashews library to cache method calls within a synchronous 3rd party library which I have hooked into.

The snippet below distills the setup and also shows an error I am facing when doing so. I believe the reason is that the thunder_protection caches tasks regardless of what loop they're running in, which means the currently running loop when doing await tasks[_key] might be different from the one that put it there. When using protected=False, the issue does not appear.

I have fixed this locally using context local tasks caching instead of global task caching, and will open a PR shortly that illustrates this.

import asyncio
from threading import Thread

from cashews import cache

cache.setup("mem://")

@cache(ttl="10m", key="foo")  # protected=False would fix the issue
async def foo(arg: str):
    print(arg)

async def main():
    def start_event_loop(loop):
        asyncio.set_event_loop(loop)
        loop.run_forever()

    loop = asyncio.new_event_loop()
    thread = Thread(target=start_event_loop, args=(loop,), daemon=True)
    thread.start()

    future = asyncio.run_coroutine_threadsafe(foo("threadsafe"), loop)
    await foo("mainthread")
    future.result()

if __name__ == "__main__":
    asyncio.run(main())

# prints mainthread
Traceback (most recent call last):
  File "/home/rschmidtke/scratch/ts-aws-data-hub/asynciotest.py", line 26, in <module>
    asyncio.run(amain())
  File "/home/rschmidtke/miniforge3/envstestenvlib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/home/rschmidtke/miniforge3/envstestenvlib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "/home/rschmidtke/scratch/ts-aws-data-hub/asynciotest.py", line 23, in amain
    f1.result()
  File "/home/rschmidtke/miniforge3/envstestenvlib/python3.10/concurrent/futures/_base.py", line 451, in result
    return self.__get_result()
  File "/home/rschmidtke/miniforge3/envstestenvlib/python3.10/concurrent/futures/_base.py", line 403, in __get_result
    raise self._exception
  File "/home/rschmidtke/miniforge3/envstestenvlib/python3.10/site-packages/cashews/wrapper/decorators.py", line 69, in _call
    return await thunder_protection(decorator)(*args, **kwargs)
  File "/home/rschmidtke/miniforge3/envstestenvlib/python3.10/site-packages/cashews/decorators/locked.py", line 98, in _wrapper
    return await tasks[_key]
RuntimeError: Task <Task pending name='Task-4' coro=<foo() running at /home/rschmidtke/miniforge3/envstestenvlib/python3.10/site-packages/cashews/wrapper/decorators.py:69> cb=[_chain_future.<locals>._call_set_state() at /home/rschmidtke/miniforge3/envstestenvlib/python3.10/asyncio/futures.py:392]> got Future <Task pending name='Task-2' coro=<foo() running at /home/rschmidtke/miniforge3/envstestenvlib/python3.10/site-packages/cashews/decorators/cache/simple.py:57> cb=[thunder_protection.<locals>._decor.<locals>.done_callback('foo')() at /home/rschmidtke/miniforge3/envstestenvlib/python3.10/site-packages/cashews/decorators/locked.py:91, Task.task_wakeup()]> attached to a different loop
robert-schmidtke commented 5 months ago

I actually noted that a straightforward fix for this would be to use dedicated Cache instances, rather than reusing the default cache.

Krukov commented 4 months ago

Hello,

Nice case but unfortunately any application that use a shared state , like tasks lists or connections pool, will have problems with multiple loops ( like problem with redis that you mentioned in PR ) So probably if you can fix it with separated/dedicated Cache instance that would be a better solution.

robert-schmidtke commented 4 months ago

Closing as the workaround is more versatile anyway and available today already.