From 097c4a65367d9d7c9cdfffe42c2103f3e64937fd Mon Sep 17 00:00:00 2001
From: "zhaozhao.zz" <zhaozhao.zz@alibaba-inc.com>
Date: Tue, 15 Jan 2019 16:58:29 +0800
Subject: [PATCH 1/2] scripting: flag lua_client as CLIENT_MULTI after
redis.replicate_commands() immediately
To avoid nested MULTI/EXEC, we check the lua_caller's flag,
if we are in the MULTI context we flag the lua_client as
CLIENT_MULTI, but it's not enough we shoud flag lua_client
as CLIENT_MULTI after redis.replicate_commands() immediately
or the first write command after redis.replicate_commands()
cannot know it's in an transaction, I know the missing CLIENT_MULTI
doesn't have any effect now, but it's a real bug and we should fix
it, in case someday we allow some dangerous command like BLPOP.
---
src/scripting.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/scripting.c b/src/scripting.c
index 7cf21f408..76885a02b 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -703,6 +703,9 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
{
execCommandPropagateMulti(server.lua_caller);
server.lua_multi_emitted = 1;
+ /* Now we are in the MULTI context, the lua_client should be
+ * flag as CLIENT_MULTI. */
+ c->flags |= CLIENT_MULTI;
}
/* Run the command */
From 73841e8c495ec4917a8cdee3cbef6fb83433f018 Mon Sep 17 00:00:00 2001
From: "zhaozhao.zz" <zhaozhao.zz@alibaba-inc.com>
Date: Tue, 15 Jan 2019 20:08:11 +0800
Subject: [PATCH 2/2] scripting: a little refactor about EVAL and redis.call()
Change server.lua_client's flag in a more explicit way.
---
src/scripting.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/scripting.c b/src/scripting.c
index 76885a02b..d7b1fdd4a 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -485,13 +485,6 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
static size_t cached_objects_len[LUA_CMD_OBJCACHE_SIZE];
static int inuse = 0; /* Recursive calls detection. */
- /* Reflect MULTI state */
- if (server.lua_multi_emitted || (server.lua_caller->flags & CLIENT_MULTI)) {
- c->flags |= CLIENT_MULTI;
- } else {
- c->flags &= ~CLIENT_MULTI;
- }
-
/* By using Lua debug hooks it is possible to trigger a recursive call
* to luaRedisGenericCommand(), which normally should never happen.
* To make this function reentrant is futile and makes it slower, but
@@ -1432,6 +1425,22 @@ void luaMaskCountHook(lua_State *lua, lua_Debug *ar) {
}
}
+void prepareLuaClient(void) {
+ /* Select the right DB in the context of the Lua client */
+ selectDb(server.lua_client,server.lua_caller->db->id);
+ server.lua_client->resp = 2; /* Default is RESP2, scripts can change it. */
+
+ /* If we are in MULTI context, flag Lua client as CLIENT_MULTI. */
+ if (server.lua_caller->flags & CLIENT_MULTI) {
+ server.lua_client->flags |= CLIENT_MULTI;
+ }
+}
+
+void resetLuaClient(void) {
+ /* After the script done, remove the MULTI state. */
+ server.lua_client->flags &= ~CLIENT_MULTI;
+}
+
void evalGenericCommand(client *c, int evalsha) {
lua_State *lua = server.lua;
char funcname[43];
@@ -1520,10 +1529,6 @@ void evalGenericCommand(client *c, int evalsha) {
luaSetGlobalArray(lua,"KEYS",c->argv+3,numkeys);
luaSetGlobalArray(lua,"ARGV",c->argv+3+numkeys,c->argc-3-numkeys);
- /* Set the Lua client database and protocol. */
- selectDb(server.lua_client,c->db->id);
- server.lua_client->resp = 2; /* Default is RESP2, scripts can change it. */
-
/* Set a hook in order to be able to stop the script execution if it
* is running for too much time.
* We set the hook only if the time limit is enabled as the hook will
@@ -1543,11 +1548,15 @@ void evalGenericCommand(client *c, int evalsha) {
delhook = 1;
}
+ prepareLuaClient();
+
/* At this point whether this script was never seen before or if it was
* already defined, we can call it. We have zero arguments and expect
* a single return value. */
err = lua_pcall(lua,0,1,-2);
+ resetLuaClient();
+
/* Perform some cleanup that we need to do both on error and success. */
if (delhook) lua_sethook(lua,NULL,0,0); /* Disable hook */
if (server.lua_timedout) {