mirror of https://mirror.osredm.com/root/redis.git
do not call handleClientsBlockedOnKeys inside yielding command (#12459)
Fix the assertion when a busy script (timeout) signal ready keys (like LPUSH),
and then an arbitrary client's `allow-busy` command steps into `handleClientsBlockedOnKeys`
try wake up clients blocked on keys (like BLPOP).
Reproduction process:
1. start a redis with aof
`./redis-server --appendonly yes`
2. exec blpop
`127.0.0.1:6379> blpop a 0`
3. use another client call a busy script and this script push the blocked key
`127.0.0.1:6379> eval "redis.call('lpush','a','b') while(1) do end" 0`
4. user a new client call an allow-busy command like auth
`127.0.0.1:6379> auth a`
BTW, this issue also break the atomicity of script.
This bug has been around for many years, the old versions only have the
atomic problem, only 7.0/7.2 has the assertion problem.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 8226f39fb2
)
This commit is contained in:
parent
37599fe75a
commit
4d67bb6afa
|
@ -4019,7 +4019,7 @@ int processCommand(client *c) {
|
|||
} else {
|
||||
call(c,CMD_CALL_FULL);
|
||||
c->woff = server.master_repl_offset;
|
||||
if (listLength(server.ready_keys))
|
||||
if (listLength(server.ready_keys) && !isInsideYieldingLongCommand())
|
||||
handleClientsBlockedOnKeys();
|
||||
}
|
||||
|
||||
|
|
|
@ -1102,6 +1102,53 @@ start_server {tags {"scripting"}} {
|
|||
r ping
|
||||
} {PONG}
|
||||
|
||||
test {Timedout scripts and unblocked command} {
|
||||
# make sure a command that's allowed during BUSY doesn't trigger an unblocked command
|
||||
|
||||
# enable AOF to also expose an assertion if the bug would happen
|
||||
r flushall
|
||||
r config set appendonly yes
|
||||
|
||||
# create clients, and set one to block waiting for key 'x'
|
||||
set rd [redis_deferring_client]
|
||||
set rd2 [redis_deferring_client]
|
||||
set r3 [redis_client]
|
||||
$rd2 blpop x 0
|
||||
wait_for_blocked_clients_count 1
|
||||
|
||||
# hack: allow the script to use client list command so that we can control when it aborts
|
||||
r DEBUG set-disable-deny-scripts 1
|
||||
r config set lua-time-limit 10
|
||||
run_script_on_connection $rd {
|
||||
local clients
|
||||
redis.call('lpush',KEYS[1],'y');
|
||||
while true do
|
||||
clients = redis.call('client','list')
|
||||
if string.find(clients, 'abortscript') ~= nil then break end
|
||||
end
|
||||
redis.call('lpush',KEYS[1],'z');
|
||||
return clients
|
||||
} 1 x
|
||||
|
||||
# wait for the script to be busy
|
||||
after 200
|
||||
catch {r ping} e
|
||||
assert_match {BUSY*} $e
|
||||
|
||||
# run cause the script to abort, and run a command that could have processed
|
||||
# unblocked clients (due to a bug)
|
||||
$r3 hello 2 setname abortscript
|
||||
|
||||
# make sure the script completed before the pop was processed
|
||||
assert_equal [$rd2 read] {x z}
|
||||
assert_match {*abortscript*} [$rd read]
|
||||
|
||||
$rd close
|
||||
$rd2 close
|
||||
$r3 close
|
||||
r DEBUG set-disable-deny-scripts 0
|
||||
} {OK} {external:skip needs:debug}
|
||||
|
||||
test {Timedout scripts that modified data can't be killed by SCRIPT KILL} {
|
||||
set rd [redis_deferring_client]
|
||||
r config set lua-time-limit 10
|
||||
|
|
Loading…
Reference in New Issue