From 9ae4f5c73d2e2cd80cebd80850e9e7423821ae3f Mon Sep 17 00:00:00 2001
From: "Meir Shpilraien (Spielrein)" <meir@redislabs.com>
Date: Wed, 17 Mar 2021 18:52:11 +0200
Subject: [PATCH] Fix script kill to work also on scripts that use pcall
 (#8661)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

pcall function runs another LUA function in protected mode, this means
that any error will be caught by this function and will not stop the LUA
execution. The script kill mechanism uses error to stop the running script.
Scripts that uses pcall can catch the error raise by the script kill mechanism,
this will cause a script like this to be unkillable:

local f = function()
        while 1 do
                redis.call('ping')
        end
end
while 1 do
        pcall(f)
end

The fix is, when we want to kill the script, we set the hook function to be invoked 
after each line. This will promise that the execution will get another
error before it is able to enter the pcall function again.
---
 src/scripting.c          |  8 ++++++++
 tests/unit/scripting.tcl | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/scripting.c b/src/scripting.c
index 345feda2f..4276190ce 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -1453,6 +1453,14 @@ void luaMaskCountHook(lua_State *lua, lua_Debug *ar) {
     if (server.lua_timedout) processEventsWhileBlocked();
     if (server.lua_kill) {
         serverLog(LL_WARNING,"Lua script killed by user with SCRIPT KILL.");
+
+        /*
+         * Set the hook to invoke all the time so the user
+         * will not be able to catch the error with pcall and invoke
+         * pcall again which will prevent the script from ever been killed
+         */
+        lua_sethook(lua, luaMaskCountHook, LUA_MASKLINE, 0);
+
         lua_pushstring(lua,"Script killed by user with SCRIPT KILL...");
         lua_error(lua);
     }
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index c44ec74f5..92ca05bdc 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -612,6 +612,34 @@ start_server {tags {"scripting"}} {
         assert_equal [r ping] "PONG"
     }
 
+    test {Timedout read-only scripts can be killed by SCRIPT KILL even when use pcall} {
+        set rd [redis_deferring_client]
+        r config set lua-time-limit 10
+        $rd eval {local f = function() while 1 do redis.call('ping') end end while 1 do pcall(f) end} 0
+        
+        wait_for_condition 50 100 {
+            [catch {r ping} e] == 1
+        } else {
+            fail "Can't wait for script to start running"
+        }
+        catch {r ping} e
+        assert_match {BUSY*} $e
+
+        r script kill
+
+        wait_for_condition 50 100 {
+            [catch {r ping} e] == 0
+        } else {
+            fail "Can't wait for script to be killed"
+        }
+        assert_equal [r ping] "PONG"
+
+        catch {$rd read} res
+        $rd close
+
+        assert_match {*killed by user*} $res        
+    }
+
     test {Timedout script link is still usable after Lua returns} {
         r config set lua-time-limit 10
         r eval {for i=1,100000 do redis.call('ping') end return 'ok'} 0