From b4e068c7ffcec2cd438a5aacf890f387184c9604 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 10:29:38 +0200 Subject: [PATCH 1/6] guard --- redis/asyncio/client.py | 12 +++++++++--- redis/asyncio/cluster.py | 12 ++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py index 3d59016bb3..d1d60312e2 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -1349,10 +1349,16 @@ async def execute(self, raise_on_error: bool = True): conn = cast(Connection, conn) try: - return await conn.retry.call_with_retry( - lambda: execute(conn, stack, raise_on_error), - lambda error: self._disconnect_raise_reset(conn, error), + return await asyncio.shield( + conn.retry.call_with_retry( + lambda: execute(conn, stack, raise_on_error), + lambda error: self._disconnect_raise_reset(conn, error), + ) ) + except asyncio.CancelledError: + # not supposed to be possible, yet here we are + await connection.disconnect(nowait=True) + raise finally: await self.reset() diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 3fe3ebc47e..8dfb1cbdb8 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -879,10 +879,18 @@ async def execute_command(self, *args: Any, **kwargs: Any) -> Any: await connection.send_packed_command(connection.pack_command(*args), False) # Read response + return await asyncio.shield( + self._parse_and_release(connection, args[0], **kwargs) + ) + + async def _parse_and_release(self, connection, *args, **kwargs): try: - return await self.parse_response(connection, args[0], **kwargs) + return await self.parse_response(connection, *args, **kwargs) + except asyncio.CancelledError: + # should not be possible + await connection.disconnect(nowait=True) + raise finally: - # Release connection self._free.append(connection) async def execute_pipeline(self, commands: List["PipelineCommand"]) -> bool: From 74d08a27abeb463f4647da006878a121843acf77 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 10:49:29 +0200 Subject: [PATCH 2/6] fixing variable in cancel --- redis/asyncio/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py index d1d60312e2..5c0b546bf5 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -1357,7 +1357,7 @@ async def execute(self, raise_on_error: bool = True): ) except asyncio.CancelledError: # not supposed to be possible, yet here we are - await connection.disconnect(nowait=True) + await conn.disconnect(nowait=True) raise finally: await self.reset() From a054552e403c68e5e23ea2b898cac9e397ff6843 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 11:46:49 +0200 Subject: [PATCH 3/6] asyncio backport --- .github/workflows/integration.yaml | 14 +++++++------- setup.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 59f6c7d322..9636e0fde4 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -6,13 +6,13 @@ on: - 'docs/**' - '**/*.rst' - '**/*.md' - branches: - - master - - '[0-9].[0-9]' - pull_request: - branches: - - master - - '[0-9].[0-9]' +# branches: +# - master +# - '[0-9].[0-9]' +# pull_request: +# branches: +# - master +# - '[0-9].[0-9]' jobs: diff --git a/setup.py b/setup.py index 35c59f5452..3c66aea00b 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ long_description_content_type="text/markdown", keywords=["Redis", "key-value store", "database"], license="MIT", - version="4.3.5", + version="4.3.6", packages=find_packages( include=[ "redis", From 28c6b5707ebecf3e4f1ffb8bec735bbaf97b4b62 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 12:55:45 +0200 Subject: [PATCH 4/6] fail-fast false --- .github/workflows/integration.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 9636e0fde4..e3c2a7c49c 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -36,6 +36,7 @@ jobs: timeout-minutes: 30 strategy: max-parallel: 15 + fail-fast: false matrix: python-version: ['3.6', '3.7', '3.8', '3.9', '3.10', 'pypy-3.7'] test-type: ['standalone', 'cluster'] @@ -81,6 +82,7 @@ jobs: name: Install package from commit hash runs-on: ubuntu-latest strategy: + fail-fast: false matrix: python-version: ['3.6', '3.7', '3.8', '3.9', '3.10', 'pypy-3.7'] steps: From 045d1836fd200e5ffdfefd924b9bc27c3051dfbe Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 17:23:38 +0200 Subject: [PATCH 5/6] 20.04 --- .github/workflows/integration.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index e3c2a7c49c..87ebc2723c 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -6,13 +6,13 @@ on: - 'docs/**' - '**/*.rst' - '**/*.md' -# branches: -# - master -# - '[0-9].[0-9]' -# pull_request: -# branches: -# - master -# - '[0-9].[0-9]' + branches: + - master + - '[0-9].[0-9]' + pull_request: + branches: + - master + - '[0-9].[0-9]' jobs: @@ -32,7 +32,7 @@ jobs: invoke linters run-tests: - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 timeout-minutes: 30 strategy: max-parallel: 15 @@ -80,7 +80,7 @@ jobs: install_package_from_commit: name: Install package from commit hash - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 strategy: fail-fast: false matrix: From d5e19e7103d93049b78b876ff3c2ad36a4802578 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 17:41:12 +0200 Subject: [PATCH 6/6] and tests --- tests/test_asyncio/test_cluster.py | 17 +++++++++++++++++ tests/test_asyncio/test_connection.py | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index d6e01f79dc..2e44cdde3e 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -333,6 +333,23 @@ async def test_execute_command_node_flag_random(self, r: RedisCluster) -> None: called_count += 1 assert called_count == 1 + async def test_asynckills(self, r) -> None: + + await r.set("foo", "foo") + await r.set("bar", "bar") + + t = asyncio.create_task(r.get("foo")) + await asyncio.sleep(1) + t.cancel() + try: + await t + except asyncio.CancelledError: + pytest.fail("connection is left open with unread response") + + assert await r.get("bar") == b"bar" + assert await r.ping() + assert await r.get("foo") == b"foo" + async def test_execute_command_default_node(self, r: RedisCluster) -> None: """ Test command execution without node flag is being executed on the diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index f6259adbd2..c414ee05cc 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -28,6 +28,29 @@ async def test_invalid_response(create_redis): assert str(cm.value) == f"Protocol Error: {raw!r}" +@pytest.mark.onlynoncluster +async def test_asynckills(): + from redis.asyncio.client import Redis + + for b in [True, False]: + r = Redis(single_connection_client=b) + + await r.set("foo", "foo") + await r.set("bar", "bar") + + t = asyncio.create_task(r.get("foo")) + await asyncio.sleep(1) + t.cancel() + try: + await t + except asyncio.CancelledError: + pytest.fail("connection left open with unread response") + + assert await r.get("bar") == b"bar" + assert await r.ping() + assert await r.get("foo") == b"foo" + + @skip_if_server_version_lt("4.0.0") @pytest.mark.redismod @pytest.mark.onlynoncluster