AsyncIO Race Condition Fix (#2641)v4.5.3

7 files changed, 764 insertions, 6 deletions

diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py
index 3fc7fad..9e16ee0 100644
--- a/redis/asyncio/client.py
+++ b/redis/asyncio/client.py
@@ -1385,10 +1385,16 @@ class Pipeline(Redis):  # lgtm [py/init-calls-subclass]
         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 conn.disconnect(nowait=True)
+            raise
         finally:
             await self.reset()
 
diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py
index 5a2dffd..569a076 100644
--- a/redis/asyncio/cluster.py
+++ b/redis/asyncio/cluster.py
@@ -1002,10 +1002,18 @@ class ClusterNode:
         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:
diff --git a/setup.py b/setup.py
index a0710d3..3003c59 100644
--- a/setup.py
+++ b/setup.py
@@ -8,7 +8,7 @@ setup(
     long_description_content_type="text/markdown",
     keywords=["Redis", "key-value store", "database"],
     license="MIT",
-    version="4.5.2",
+    version="4.5.3",
     packages=find_packages(
         include=[
             "redis",
diff --git a/tests/asynctests b/tests/asynctests
new file mode 100644
index 0000000..4f0fea9
--- /dev/null
+++ b/tests/asynctests
@@ -0,0 +1,285 @@
+test_response_callbacks
+test_case_insensitive_command_names
+test_command_on_invalid_key_type
+test_acl_cat_no_category
+test_acl_cat_with_category
+test_acl_deluser
+test_acl_genpass
+test_acl_getuser_setuser
+test_acl_list
+test_acl_log
+test_acl_setuser_categories_without_prefix_fails
+test_acl_setuser_commands_without_prefix_fails
+test_acl_setuser_add_passwords_and_nopass_fails
+test_acl_users
+test_acl_whoami
+test_client_list
+test_client_list_type
+test_client_id
+test_client_unblock
+test_client_getname
+test_client_setname
+test_client_kill
+test_client_kill_filter_invalid_params
+test_client_kill_filter_by_id
+test_client_kill_filter_by_addr
+test_client_list_after_client_setname
+test_client_pause
+test_config_get
+test_config_resetstat
+test_config_set
+test_dbsize
+test_echo
+test_info
+test_lastsave
+test_object
+test_ping
+test_slowlog_get
+test_slowlog_get_limit
+test_slowlog_length
+test_time
+test_never_decode_option
+test_empty_response_option
+test_append
+test_bitcount
+test_bitop_not_empty_string
+test_bitop_not
+test_bitop_not_in_place
+test_bitop_single_string
+test_bitop_string_operands
+test_bitpos
+test_bitpos_wrong_arguments
+test_decr
+test_decrby
+test_delete
+test_delete_with_multiple_keys
+test_delitem
+test_unlink
+test_unlink_with_multiple_keys
+test_dump_and_restore
+test_dump_and_restore_and_replace
+test_dump_and_restore_absttl
+test_exists
+test_exists_contains
+test_expire
+test_expireat_datetime
+test_expireat_no_key
+test_expireat_unixtime
+test_get_and_set
+test_get_set_bit
+test_getrange
+test_getset
+test_incr
+test_incrby
+test_incrbyfloat
+test_keys
+test_mget
+test_mset
+test_msetnx
+test_pexpire
+test_pexpireat_datetime
+test_pexpireat_no_key
+test_pexpireat_unixtime
+test_psetex
+test_psetex_timedelta
+test_pttl
+test_pttl_no_key
+test_randomkey
+test_rename
+test_renamenx
+test_set_nx
+test_set_xx
+test_set_px
+test_set_px_timedelta
+test_set_ex
+test_set_ex_timedelta
+test_set_multipleoptions
+test_set_keepttl
+test_setex
+test_setnx
+test_setrange
+test_strlen
+test_substr
+test_ttl
+test_ttl_nokey
+test_type
+test_blpop
+test_brpop
+test_brpoplpush
+test_brpoplpush_empty_string
+test_lindex
+test_linsert
+test_llen
+test_lpop
+test_lpush
+test_lpushx
+test_lrange
+test_lrem
+test_lset
+test_ltrim
+test_rpop
+test_rpoplpush
+test_rpush
+test_lpos
+test_rpushx
+test_scan
+test_scan_type
+test_scan_iter
+test_sscan
+test_sscan_iter
+test_hscan
+test_hscan_iter
+test_zscan
+test_zscan_iter
+test_sadd
+test_scard
+test_sdiff
+test_sdiffstore
+test_sinter
+test_sinterstore
+test_sismember
+test_smembers
+test_smove
+test_spop
+test_spop_multi_value
+test_srandmember
+test_srandmember_multi_value
+test_srem
+test_sunion
+test_sunionstore
+test_zadd
+test_zadd_nx
+test_zadd_xx
+test_zadd_ch
+test_zadd_incr
+test_zadd_incr_with_xx
+test_zcard
+test_zcount
+test_zincrby
+test_zlexcount
+test_zinterstore_sum
+test_zinterstore_max
+test_zinterstore_min
+test_zinterstore_with_weight
+test_zpopmax
+test_zpopmin
+test_bzpopmax
+test_bzpopmin
+test_zrange
+test_zrangebylex
+test_zrevrangebylex
+test_zrangebyscore
+test_zrank
+test_zrem
+test_zrem_multiple_keys
+test_zremrangebylex
+test_zremrangebyrank
+test_zremrangebyscore
+test_zrevrange
+test_zrevrangebyscore
+test_zrevrank
+test_zscore
+test_zunionstore_sum
+test_zunionstore_max
+test_zunionstore_min
+test_zunionstore_with_weight
+test_pfadd
+test_pfcount
+test_pfmerge
+test_hget_and_hset
+test_hset_with_multi_key_values
+test_hset_without_data
+test_hdel
+test_hexists
+test_hgetall
+test_hincrby
+test_hincrbyfloat
+test_hkeys
+test_hlen
+test_hmget
+test_hmset
+test_hsetnx
+test_hvals
+test_hstrlen
+test_sort_basic
+test_sort_limited
+test_sort_by
+test_sort_get
+test_sort_get_multi
+test_sort_get_groups_two
+test_sort_groups_string_get
+test_sort_groups_just_one_get
+test_sort_groups_no_get
+test_sort_groups_three_gets
+test_sort_desc
+test_sort_alpha
+test_sort_store
+test_sort_all_options
+test_sort_issue_924
+test_cluster_addslots
+test_cluster_count_failure_reports
+test_cluster_countkeysinslot
+test_cluster_delslots
+test_cluster_failover
+test_cluster_forget
+test_cluster_info
+test_cluster_keyslot
+test_cluster_meet
+test_cluster_nodes
+test_cluster_replicate
+test_cluster_reset
+test_cluster_saveconfig
+test_cluster_setslot
+test_cluster_slaves
+test_readwrite
+test_readonly_invalid_cluster_state
+test_readonly
+test_geoadd
+test_geoadd_invalid_params
+test_geodist
+test_geodist_units
+test_geodist_missing_one_member
+test_geodist_invalid_units
+test_geohash
+test_geopos
+test_geopos_no_value
+test_old_geopos_no_value
+test_georadius
+test_georadius_no_values
+test_georadius_units
+test_georadius_with
+test_georadius_count
+test_georadius_sort
+test_georadius_store
+test_georadius_store_dist
+test_georadiusmember
+test_xack
+test_xadd
+test_xclaim
+test_xclaim_trimmed
+test_xdel
+test_xgroup_create
+test_xgroup_create_mkstream
+test_xgroup_delconsumer
+test_xgroup_destroy
+test_xgroup_setid
+test_xinfo_consumers
+test_xinfo_stream
+test_xlen
+test_xpending
+test_xpending_range
+test_xrange
+test_xread
+test_xreadgroup
+test_xrevrange
+test_xtrim
+test_bitfield_operations
+test_bitfield_ro
+test_memory_stats
+test_memory_usage
+test_module_list
+test_binary_get_set
+test_binary_lists
+test_22_info
+test_large_responses
+test_floating_point_encoding
diff --git a/tests/synctests b/tests/synctests
new file mode 100644
index 0000000..b0de2d1
--- /dev/null
+++ b/tests/synctests
@@ -0,0 +1,421 @@
+test_response_callbacks
+test_case_insensitive_command_names
+test_auth
+test_command_on_invalid_key_type
+test_acl_cat_no_category
+test_acl_cat_with_category
+test_acl_dryrun
+test_acl_deluser
+test_acl_genpass
+test_acl_getuser_setuser
+test_acl_help
+test_acl_list
+test_acl_log
+test_acl_setuser_categories_without_prefix_fails
+test_acl_setuser_commands_without_prefix_fails
+test_acl_setuser_add_passwords_and_nopass_fails
+test_acl_users
+test_acl_whoami
+test_client_list
+test_client_info
+test_client_list_types_not_replica
+test_client_list_replica
+test_client_list_client_id
+test_client_id
+test_client_trackinginfo
+test_client_tracking
+test_client_unblock
+test_client_getname
+test_client_setname
+test_client_kill
+test_client_kill_filter_invalid_params
+test_client_kill_filter_by_id
+test_client_kill_filter_by_addr
+test_client_list_after_client_setname
+test_client_kill_filter_by_laddr
+test_client_kill_filter_by_user
+test_client_pause
+test_client_pause_all
+test_client_unpause
+test_client_no_evict
+test_client_reply
+test_client_getredir
+test_hello_notI_implemented
+test_config_get
+test_config_get_multi_params
+test_config_resetstat
+test_config_set
+test_config_set_multi_params
+test_failover
+test_dbsize
+test_echo
+test_info
+test_info_multi_sections
+test_lastsave
+test_lolwut
+test_reset
+test_object
+test_ping
+test_quit
+test_role
+test_select
+test_slowlog_get
+test_slowlog_get_limit
+test_slowlog_length
+test_time
+test_bgsave
+test_never_decode_option
+test_empty_response_option
+test_append
+test_bitcount
+test_bitcount_mode
+test_bitop_not_empty_string
+test_bitop_not
+test_bitop_not_in_place
+test_bitop_single_string
+test_bitop_string_operands
+test_bitpos
+test_bitpos_wrong_arguments
+test_bitpos_mode
+test_copy
+test_copy_and_replace
+test_copy_to_another_database
+test_decr
+test_decrby
+test_delete
+test_delete_with_multiple_keys
+test_delitem
+test_unlink
+test_unlink_with_multiple_keys
+test_lcs
+test_dump_and_restore
+test_dump_and_restore_and_replace
+test_dump_and_restore_absttl
+test_exists
+test_exists_contains
+test_expire
+test_expire_option_nx
+test_expire_option_xx
+test_expire_option_gt
+test_expire_option_lt
+test_expireat_datetime
+test_expireat_no_key
+test_expireat_unixtime
+test_expiretime
+test_expireat_option_nx
+test_expireat_option_xx
+test_expireat_option_gt
+test_expireat_option_lt
+test_get_and_set
+test_getdel
+test_getex
+test_getitem_and_setitem
+test_getitem_raises_keyerror_for_missing_key
+test_getitem_does_not_raise_keyerror_for_empty_string
+test_get_set_bit
+test_getrange
+test_getset
+test_incr
+test_incrby
+test_incrbyfloat
+test_keys
+test_mget
+test_lmove
+test_blmove
+test_mset
+test_msetnx
+test_pexpire
+test_pexpire_option_nx
+test_pexpire_option_xx
+test_pexpire_option_gt
+test_pexpire_option_lt
+test_pexpireat_datetime
+test_pexpireat_no_key
+test_pexpireat_unixtime
+test_pexpireat_option_nx
+test_pexpireat_option_xx
+test_pexpireat_option_gt
+test_pexpireat_option_lt
+test_pexpiretime
+test_psetex
+test_psetex_timedelta
+test_pttl
+test_pttl_no_key
+test_hrandfield
+test_randomkey
+test_rename
+test_renamenx
+test_set_nx
+test_set_xx
+test_set_px
+test_set_px_timedelta
+test_set_ex
+test_set_ex_str
+test_set_ex_timedelta
+test_set_exat_timedelta
+test_set_pxat_timedelta
+test_set_multipleoptions
+test_set_keepttl
+test_set_get
+test_setex
+test_setnx
+test_setrange
+test_stralgo_lcs
+test_stralgo_negative
+test_strlen
+test_substr
+test_ttl
+test_ttl_nokey
+test_type
+test_blpop
+test_brpop
+test_brpoplpush
+test_brpoplpush_empty_string
+test_blmpop
+test_lmpop
+test_lindex
+test_linsert
+test_llen
+test_lpop
+test_lpop_count
+test_lpush
+test_lpushx
+test_lpushx_with_list
+test_lrange
+test_lrem
+test_lset
+test_ltrim
+test_rpop
+test_rpop_count
+test_rpoplpush
+test_rpush
+test_lpos
+test_rpushx
+test_scan
+test_scan_type
+test_scan_iter
+test_sscan
+test_sscan_iter
+test_hscan
+test_hscan_iter
+test_zscan
+test_zscan_iter
+test_sadd
+test_scard
+test_sdiff
+test_sdiffstore
+test_sinter
+test_sintercard
+test_sinterstore
+test_sismember
+test_smembers
+test_smismember
+test_smove
+test_spop
+test_spop_multi_value
+test_srandmember
+test_srandmember_multi_value
+test_srem
+test_sunion
+test_sunionstore
+test_debug_segfault
+test_script_debug
+test_zadd
+test_zadd_nx
+test_zadd_xx
+test_zadd_ch
+test_zadd_incr
+test_zadd_incr_with_xx
+test_zadd_gt_lt
+test_zcard
+test_zcount
+test_zdiff
+test_zdiffstore
+test_zincrby
+test_zlexcount
+test_zinter
+test_zintercard
+test_zinterstore_sum
+test_zinterstore_max
+test_zinterstore_min
+test_zinterstore_with_weight
+test_zpopmax
+test_zpopmin
+test_zrandemember
+test_bzpopmax
+test_bzpopmin
+test_zmpop
+test_bzmpop
+test_zrange
+test_zrange_errors
+test_zrange_params
+test_zrangestore
+test_zrangebylex
+test_zrevrangebylex
+test_zrangebyscore
+test_zrank
+test_zrem
+test_zrem_multiple_keys
+test_zremrangebylex
+test_zremrangebyrank
+test_zremrangebyscore
+test_zrevrange
+test_zrevrangebyscore
+test_zrevrank
+test_zscore
+test_zunion
+test_zunionstore_sum
+test_zunionstore_max
+test_zunionstore_min
+test_zunionstore_with_weight
+test_zmscore
+test_pfadd
+test_pfcount
+test_pfmerge
+test_hget_and_hset
+test_hset_with_multi_key_values
+test_hset_with_key_values_passed_as_list
+test_hset_without_data
+test_hdel
+test_hexists
+test_hgetall
+test_hincrby
+test_hincrbyfloat
+test_hkeys
+test_hlen
+test_hmget
+test_hmset
+test_hsetnx
+test_hvals
+test_hstrlen
+test_sort_basic
+test_sort_limited
+test_sort_by
+test_sort_get
+test_sort_get_multi
+test_sort_get_groups_two
+test_sort_groups_string_get
+test_sort_groups_just_one_get
+test_sort_groups_no_get
+test_sort_groups_three_gets
+test_sort_desc
+test_sort_alpha
+test_sort_store
+test_sort_all_options
+test_sort_ro
+test_sort_issue_924
+test_cluster_addslots
+test_cluster_count_failure_reports
+test_cluster_countkeysinslot
+test_cluster_delslots
+test_cluster_failover
+test_cluster_forget
+test_cluster_info
+test_cluster_keyslot
+test_cluster_meet
+test_cluster_nodes
+test_cluster_replicate
+test_cluster_reset
+test_cluster_saveconfig
+test_cluster_setslot
+test_cluster_slaves
+test_readwrite
+test_readonly_invalid_cluster_state
+test_readonly
+test_geoadd
+test_geoadd_nx
+test_geoadd_xx
+test_geoadd_ch
+test_geoadd_invalid_params
+test_geodist
+test_geodist_units
+test_geodist_missing_one_member
+test_geodist_invalid_units
+test_geohash
+test_geopos
+test_geopos_no_value
+test_old_geopos_no_value
+test_geosearch
+test_geosearch_member
+test_geosearch_sort
+test_geosearch_with
+test_geosearch_negative
+test_geosearchstore
+test_geosearchstore_dist
+test_georadius
+test_georadius_no_values
+test_georadius_units
+test_georadius_with
+test_georadius_count
+test_georadius_sort
+test_georadius_store
+test_georadius_store_dist
+test_georadiusmember
+test_georadiusmember_count
+test_xack
+test_xadd
+test_xadd_nomkstream
+test_xadd_minlen_and_limit
+test_xadd_explicit_ms
+test_xautoclaim
+test_xautoclaim_negative
+test_xclaim
+test_xclaim_trimmed
+test_xdel
+test_xgroup_create
+test_xgroup_create_mkstream
+test_xgroup_create_entriesread
+test_xgroup_delconsumer
+test_xgroup_createconsumer
+test_xgroup_destroy
+test_xgroup_setid
+test_xinfo_consumers
+test_xinfo_stream
+test_xinfo_stream_full
+test_xlen
+test_xpending
+test_xpending_range
+test_xpending_range_idle
+test_xpending_range_negative
+test_xrange
+test_xread
+test_xreadgroup
+test_xrevrange
+test_xtrim
+test_xtrim_minlen_and_length_args
+test_bitfield_operations
+test
+test_bitfield_ro
+test_memory_help
+test_memory_doctor
+test_memory_malloc_stats
+test_memory_stats
+test_memory_usage
+test_latency_histogram_not_implemented
+test_latency_graph_not_implemented
+test_latency_doctor_not_implemented
+test_latency_history
+test_latency_latest
+test_latency_reset
+test_module_list
+test_command_count
+test_command_docs
+test_command_list
+test_command_getkeys
+test_command
+test_command_getkeysandflags
+test_module
+test_module_loadex
+test_restore
+test_restore_idletime
+test_restore_frequency
+test_replicaof
+test_shutdown
+test_shutdown_with_params
+test_sync
+test_psync
+test_binary_get_set
+test_binary_lists
+test_22_info
+test_large_responses
+test_floating_point_encoding
diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py
index 13e5e26..0857c05 100644
--- a/tests/test_asyncio/test_cluster.py
+++ b/tests/test_asyncio/test_cluster.py
@@ -340,6 +340,23 @@ class TestRedisClusterObj:
         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 e2d77fc..d3b6285 100644
--- a/tests/test_asyncio/test_connection.py
+++ b/tests/test_asyncio/test_connection.py
@@ -44,6 +44,27 @@ async def test_invalid_response(create_redis):
     await r.connection.disconnect()
 
 
+async def test_asynckills():
+
+    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"
+
+
 @pytest.mark.onlynoncluster
 async def test_single_connection():
     """Test that concurrent requests on a single client are synchronised."""