RuntimeWarning: coroutine 'Redis.close' was never awaited

[brianmaissy]

I'm using aioredis in a pytest fixture, (using pytest-asyncio), and just upgraded to 2.0.0.

At the end of my test run, I'm getting the following warning:

/home/brian/repos/medigator/venv/lib/python3.8/site-packages/aioredis/client.py:1047: RuntimeWarning: coroutine 'Redis.close' was never awaited
  pass
Object allocated at (most recent call last):
  File "/home/brian/repos/medigator/venv/lib/python3.8/site-packages/aioredis/client.py", lineno 1045
    loop.run_until_complete(self.close())

I'm not sure exactly why it's happening, maybe it has something to do with the way pytest-asyncio is managing the loop, but it isn't really critical (I'm not saying aioredis is doing anything wrong).

I was wondering if it would be possible to add the if conn check that close() does to __del__() also, such that if I make sure to manually clean it up beforehand, the close() coroutine won't get created at __del__ time.

Happy to open a PR if it would help.

[abrookins]

I'm seeing this too and haven't fixed it yet. I'll take a closer look!

[krieghan]

I've been seeing something similar in 2.0.0, as well, when I try to close the client:

File "/usr/local/lib/python3.8/asyncio/base_events.py", line 602, in run_until_complete
self.run_forever()
File "/usr/local/lib/python3.8/asyncio/base_events.py", line 569, in run_forever
self._run_once()
File "/usr/local/lib/python3.8/asyncio/base_events.py", line 1850, in _run_once
handle._run()
File "/usr/local/lib/python3.8/asyncio/events.py", line 81, in _run
self._context.run(self._callback, *self._args)
File "lib/python3.8/site-packages/aioredis/connection.py", line 627, in del
coro = self.disconnect()
self._ready.clear()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
./usr/local/lib/python3.8/asyncio/base_events.py:640: RuntimeWarning: coroutine 'Connection.disconnect' was never awaited

[Andrew-Chen-Wang]

Try asyncio.run_coroutine_threadsafe https://docs.python.org/3/library/asyncio-task.html#asyncio.run_coroutine_threadsafe Not sure if the coroutine is even put into queue for execution (you'll prob need to monitor the connections in redis-cli).

[krieghan]

Try asyncio.run_coroutine_threadsafe https://docs.python.org/3/library/asyncio-task.html#asyncio.run_coroutine_threadsafe Not sure if the coroutine is even put into queue for execution (you'll prob need to monitor the connections in redis-cli).

Following up on this, I'm afraid using run_coroutine_threadsafe did not, in itself, fix or change the issue. I'll continue looking at it to see if I can figure out specifically what's going on.

[symisc]

Any updates on this issue?

[jar3b]

@symisc,

Any updates on this issue?

Not an update but I use non-optimal workaround with aiohttp in my code (aioredis==2.0.0):

async def redis_init(app):
    app['rc'] = aioredis.from_url("redis://127.0.0.1")
    del type(app['rc']).__del__

async def redis_close(app):
    await app['rc'].close()
    
def run():
   app = web.Application()
   app.on_startup.append(redis_init)
   app.on_cleanup.append(redis_close)

This code cleanups Redis connection on exit but skips __del__ call which produces this never awaited warining. The __del__ method is useless if you using aiohttp because .close() was actually never called.

[Andrew-Chen-Wang]

Again, at least in django, we manually close any cache backend manuallly in a post-request hook. I'm +1 on just removing the __del__ method entirely and have the user manually call a disconnect method.

[eshfield]

I have the same problem.
So far I am using the workaround suggested by jar3b but looking forward to clear solution.

[Andrew-Chen-Wang]

@DemX86 please check master as we've merged a potential solution.

[eshfield]

@DemX86 please check master as we've merged a potential solution.

That's great, thanks a lot!
That master with the solution commit will be released soon with a new package version, right?

[Andrew-Chen-Wang]

Soon is iffy at the moment. @seandstewart thoughts on just releasing current version as patch? Incremental patching would help more people to slowly migrate.

[seandstewart]

Soon is iffy at the moment. @seandstewart thoughts on just releasing current version as patch? Incremental patching would help more people to slowly migrate.

Yeah I think that's a good idea. The migration to the latest redis client has numerous big changes so we'll want to have a separate release cycle and not block patches to the current version. We just need to make sure these changes make it into the new version as well.

[brianmaissy]

So this should be resolved by #1227?

[Andrew-Chen-Wang]

Yes. Closing for now as this is merged in v2.0.1. If it appears again, we'll reopen this issue.