From baee5650295e72fbcfe1f69ca7fa60f64cb5f740 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 10 Jan 2013 10:46:05 +0100 Subject: [PATCH] Multiple fixes for EVAL (issue #872). 1) The event handler was no restored after a timeout condition if the command was eventually executed with success. 2) The command was not converted to EVAL in case of errors in the middle of the execution. 3) Terrible duplication of code without any apparent reason. --- src/scripting.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/scripting.c b/src/scripting.c index d614f42a1..46301eb11 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -787,7 +787,7 @@ void evalGenericCommand(redisClient *c, int evalsha) { lua_State *lua = server.lua; char funcname[43]; long long numkeys; - int delhook = 0; + int delhook = 0, err; /* We want the same PRNG sequence at every call so that our PRNG is * not affected by external state. */ @@ -869,30 +869,31 @@ void evalGenericCommand(redisClient *c, int evalsha) { /* At this point whatever 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. */ - if (lua_pcall(lua,0,1,0)) { - if (delhook) lua_sethook(lua,luaMaskCountHook,0,0); /* Disable hook */ - if (server.lua_timedout) { - server.lua_timedout = 0; - /* Restore the readable handler that was unregistered when the - * script timeout was detected. */ - aeCreateFileEvent(server.el,c->fd,AE_READABLE, - readQueryFromClient,c); - } - server.lua_caller = NULL; - selectDb(c,server.lua_client->db->id); /* set DB ID from Lua client */ - addReplyErrorFormat(c,"Error running script (call to %s): %s\n", - funcname, lua_tostring(lua,-1)); - lua_pop(lua,1); - lua_gc(lua,LUA_GCCOLLECT,0); - return; - } + err = lua_pcall(lua,0,1,0); + + /* Perform some cleanup that we need to do both on error and success. */ if (delhook) lua_sethook(lua,luaMaskCountHook,0,0); /* Disable hook */ - server.lua_timedout = 0; + if (server.lua_timedout) { + server.lua_timedout = 0; + /* Restore the readable handler that was unregistered when the + * script timeout was detected. */ + aeCreateFileEvent(server.el,c->fd,AE_READABLE, + readQueryFromClient,c); + } server.lua_caller = NULL; selectDb(c,server.lua_client->db->id); /* set DB ID from Lua client */ - luaReplyToRedisReply(c,lua); lua_gc(lua,LUA_GCSTEP,1); + if (err) { + addReplyErrorFormat(c,"Error running script (call to %s): %s\n", + funcname, lua_tostring(lua,-1)); + lua_pop(lua,1); /* Consume the Lua reply. */ + } else { + /* On success convert the Lua return value into Redis protocol, and + * send it to * the client. */ + luaReplyToRedisReply(c,lua); + } + /* If we have slaves attached we want to replicate this command as * EVAL instead of EVALSHA. We do this also in the AOF as currently there * is no easy way to propagate a command in a different way in the AOF