From 6f50d943a80e922bd5fb4de5206b210c6fd94b38 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 10:29:38 +0200 Subject: [PATCH 1/5] 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 e56fd022fc..f208e0ef92 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -1374,10 +1374,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 5a2dffdd1d..569a0765f8 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -1002,10 +1002,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 86136e07a5ae4ea12c84393f725311565eda5916 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 10:49:29 +0200 Subject: [PATCH 2/5] 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 f208e0ef92..0bd21d3fab 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -1382,7 +1382,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 fddbac4e229638c98439e3eaa1e1f734c4f0f0df Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 11:44:54 +0200 Subject: [PATCH 3/5] 443 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 16a9156641..4314d776b8 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.4.2", + version="4.4.3", packages=find_packages( include=[ "redis", From 5884a4f257d893062a5bd4b503cd91cbe090cd08 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 11:45:46 +0200 Subject: [PATCH 4/5] for cron disable --- .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 8d38cd45c7..891dfe4566 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -6,15 +6,15 @@ on: - 'docs/**' - '**/*.rst' - '**/*.md' - branches: - - master - - '[0-9].[0-9]' - pull_request: - branches: - - master - - '[0-9].[0-9]' - schedule: - - cron: '0 1 * * *' # nightly build +# branches: +# - master +# - '[0-9].[0-9]' +# pull_request: +# branches: +# - master +# - '[0-9].[0-9]' +# schedule: +# - cron: '0 1 * * *' # nightly build permissions: contents: read # to fetch code (actions/checkout) From ad76776ff36652c5eabd9ada3ca8aa415caf2aa7 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Wed, 22 Mar 2023 16:55:57 +0200 Subject: [PATCH 5/5] tests --- .github/workflows/integration.yaml | 14 +++++++------- tests/test_asyncio/test_cluster.py | 17 +++++++++++++++++ tests/test_asyncio/test_connection.py | 22 ++++++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 891dfe4566..5b534006a8 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]' # schedule: # - cron: '0 1 * * *' # nightly build diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 13e5e26ae3..0857c056c2 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -340,6 +340,23 @@ async def test_from_url(self, request: FixtureRequest) -> None: rc = RedisCluster.from_url("rediss://localhost:16379") assert rc.connection_kwargs["connection_class"] is SSLConnection + 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_max_connections( self, create_redis: Callable[..., RedisCluster] ) -> None: diff --git a/tests/test_asyncio/test_connection.py b/tests/test_asyncio/test_connection.py index bf59dbe6b0..ccd6cdb67a 100644 --- a/tests/test_asyncio/test_connection.py +++ b/tests/test_asyncio/test_connection.py @@ -41,6 +41,28 @@ async def test_invalid_response(create_redis): await r.connection.disconnect() +@pytest.mark.onlynoncluster +async def test_asynckills(create_redis): + + for b in [True, False]: + r = await create_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