Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection task cancel doesn't clean up and hangs #2511

Closed
tobymao opened this issue Dec 14, 2022 · 2 comments
Closed

Connection task cancel doesn't clean up and hangs #2511

tobymao opened this issue Dec 14, 2022 · 2 comments
Labels
bug Bug

Comments

@tobymao
Copy link

tobymao commented Dec 14, 2022

Version: What redis-py and what redis version is the issue happening on?
v4.4.0

specifically this commit

9fe8366

Platform: What platform / version? (For example Python 3.5.1 on Windows 7 / Ubuntu 15.10 / Azure)
arch linux, python 3.10

Description: Description of your issue, stack traces from errors and code that reproduces the issue

The change in async def read_response BaseException -> Exception causes my unit tests to hang forever

Traceback (most recent call last):
  File "/usr/lib/python3.10/unittest/async_case.py", line 67, in _callTearDown
    self._callAsync(self.asyncTearDown)
  File "/usr/lib/python3.10/unittest/async_case.py", line 79, in _callAsync
    return self._asyncioTestLoop.run_until_complete(fut)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "/usr/lib/python3.10/unittest/async_case.py", line 101, in _asyncioLoopRunner
    ret = await awaitable
  File "/home/toby/dev/saq/tests/test_worker.py", line 64, in asyncTearDown
    await cleanup_queue(self.queue)
  File "/home/toby/dev/saq/tests/helpers.py", line 11, in cleanup_queue
    await queue.redis.flushdb()
  File "/home/toby/dev/redis-py/redis/asyncio/client.py", line 487, in execute_command
    return await conn.retry.call_with_retry(
  File "/home/toby/dev/redis-py/redis/asyncio/retry.py", line 62, in call_with_retry
    await fail(error)
  File "/home/toby/dev/redis-py/redis/asyncio/client.py", line 476, in _disconnect_raise
    raise error
  File "/home/toby/dev/redis-py/redis/asyncio/retry.py", line 59, in call_with_retry
    return await do()
  File "/home/toby/dev/redis-py/redis/asyncio/client.py", line 463, in _send_command_parse_response
    return await self.parse_response(conn, command_name, **options)
  File "/home/toby/dev/redis-py/redis/asyncio/client.py", line 505, in parse_response
    response = await connection.read_response()
  File "/home/toby/dev/redis-py/redis/asyncio/connection.py", line 948, in read_response
    response = await self._parser.read_response(
  File "/home/toby/dev/redis-py/redis/asyncio/connection.py", line 397, in read_response
    raw = await self._buffer.readline()
  File "/home/toby/dev/redis-py/redis/asyncio/connection.py", line 323, in readline
    await self._read_from_socket()
  File "/home/toby/dev/redis-py/redis/asyncio/connection.py", line 266, in _read_from_socket
    raise ConnectionError(SERVER_CLOSED_CONNECTION_ERROR)
redis.exceptions.ConnectionError: Connection closed by server.

I need to terminate redis to stop the process from hanging.

https://github.com/tobymao/saq/blob/master/tests/test_worker.py#L71

This is the unit test that fails. You can follow the SAQ install instructions to run that unit test.

Changing back to BaseException fixes the issue. My guess is because in the unit test, I cancel the task, and the error handling in redis-py doesn't disconnect when a task is cancelled anymore.

@tobymao tobymao changed the title https://github.com/redis/redis-py/issues/new/choose Connection task cancel doesn't clean up and hangs Dec 14, 2022
@dvora-h dvora-h added the bug Bug label Dec 14, 2022
@tobymao
Copy link
Author

tobymao commented Dec 14, 2022

after doing some more research, it looks like there was a lengthy discussion about it

#2103

as noted, my unit test shows that redis is now in a bad state which I don't see how this can be correct

original pull #2104

@dvora-h
Copy link
Collaborator

dvora-h commented Jul 16, 2023

Closing as I assume this issue was fixed with all the async and CancelledError fixes.

@tobymao Feel free to re-open it if this still happen to you.

@dvora-h dvora-h closed this as completed Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug
Projects
None yet
Development

No branches or pull requests

2 participants