AsyncIO Race Condition Fix (#2641)

Author: chayim

redis/asyncio/client.py

@@ -1385,10 +1385,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 conn.disconnect(nowait=True)
+            raise
         finally:
             await self.reset()
 

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:

setup.py

@@ -8,7 +8,7 @@
     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",

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