gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

do not call handleClientsBlockedOnKeys inside yielding command (#12459)

Browse Source 
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>
This commit is contained in:
zhaozhao.zz 2023-08-05 14:52:03 +08:00 committed by GitHub
parent da9c2804a5
commit 8226f39fb2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 48 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/server.c
View File

@ -4158,7 +4158,7 @@ int processCommand(client *c) {
int flags = CMD_CALL_FULL;
if (client_reprocessing_command) flags |= CMD_CALL_REPROCESSING;
call(c,flags);
if (listLength(server.ready_keys))
if (listLength(server.ready_keys) && !isInsideYieldingLongCommand())
handleClientsBlockedOnKeys();
}

47
tests/unit/scripting.tcl
View File

@ -1119,6 +1119,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